)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f2fe782853aaeca78f7fa49da9786094620ed27c","unresolved":true,"context_lines":[{"line_number":22,"context_line":"iterator (see the RelatedChange)."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Some GetOrHeadHandler unit tests are relocated to a dedicated TestCase"},{"line_number":25,"context_line":"class for GetOrHEadHandler."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"This refactoring patch makes no intentional behavioral changes, apart"},{"line_number":28,"context_line":"from the text of some error log messages."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"53cbfa69_e017dcc2","line":25,"updated":"2023-10-02 17:50:57.000000000","message":"s/HEad/Head","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c4f0c5cd079cb43837d6d724b6d648efa1d555a5","unresolved":false,"context_lines":[{"line_number":22,"context_line":"iterator (see the RelatedChange)."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"Some GetOrHeadHandler unit tests are relocated to a dedicated TestCase"},{"line_number":25,"context_line":"class for GetOrHEadHandler."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"This refactoring patch makes no intentional behavioral changes, apart"},{"line_number":28,"context_line":"from the text of some error log messages."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"eb154a8b_fb9d2d52","line":25,"in_reply_to":"53cbfa69_e017dcc2","updated":"2023-10-06 16:57:42.000000000","message":"Done","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"8018c6fc5478396d6b8a1cd4fe6b00fd940347e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"fa8b778f_a4ef150c","updated":"2023-08-16 14:42:09.000000000","message":"recheck\n\nprobetests failed due to resetswift not cleaning up (unrelated)","commit_id":"d068a553d6a3b42bb0bfff3b4a78c3d01b49eb73"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"acda13b18d4834bbae2b68b50bdb36575d3220c5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"6ea961dc_be6a1f12","updated":"2023-09-19 10:52:24.000000000","message":"Moved the new tests to a separate parent patch https://review.opendev.org/c/openstack/swift/+/895802/1?usp\u003drelated-change, no other changes.","commit_id":"e3d3f8cf339ab063648a8ddfb432a8d6c5ccb2c0"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"268a8f5de699dc7cf726cacc40d68d971194cb5e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"7b126d9b_8ee5aed8","updated":"2023-10-03 15:23:05.000000000","message":"I\u0027ve tried to fix the StopIteration business in https://review.opendev.org/c/openstack/swift/+/897236","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f2fe782853aaeca78f7fa49da9786094620ed27c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"fbaa9bfd_a3bf29bb","updated":"2023-10-02 17:50:57.000000000","message":"this change doesn\u0027t really make sense to me; it looks like it\u0027s trying to add a shim to carry forward a bug in the replicated multi-part retry path that hides a backend error - a bug that we fixed in EC in the referenced related change.\n\nI appreciate that we can\u0027t boil the ocean, but i think moving `_get_next_response_part` into base, while each concrete implementation has it\u0027s own `_iter_parts_from_response` is a little strange; maybe if we strightend out THAT difference it would be clear the replicated path doesn\u0027t need a special case for translating backend errors into \"finished normally\" StopIteration\n\nthe app/node_timeout/resource_type\u003d\u0027object\u0027/x-newest-special-case is a smell","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c75f9d6656a0b25ce7239f490ac3206df624707d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"15eec28d_3fe1b61f","updated":"2023-10-03 20:26:08.000000000","message":"I think https://review.opendev.org/c/openstack/swift/+/897236 is a big improvement - please squash it all in.  We add better tests for the behaviors we care about and reorganize and fix this code at the same time.","commit_id":"cb8a0b497b330fa68ec1d9859813acb00ba75753"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"a30bc68dd42db8f3710cb1de2520cf364b97ebc8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"d663d11b_b824b3e6","updated":"2023-10-03 20:00:45.000000000","message":"I\u0027m marking this WIP just because there\u0027s some follow-ups that may want to be squashed in","commit_id":"cb8a0b497b330fa68ec1d9859813acb00ba75753"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"1fb235e88afed2db41d029b037a5cc2b6dad1e5d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"a3e6d20e_59f140fe","updated":"2024-02-22 02:30:24.000000000","message":"good refactoring patch to reuse the same function. All look good to me, except I don\u0027t understand why the extra node_timeout calculation was needed too.","commit_id":"85d7aadbbfc3c64ee4e3dde410e99a87ecace195"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"1a29a3eaf077aff5b7c73ce9a5e22bcea1d595f5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"c23b9eb8_cf58136a","updated":"2024-02-23 01:32:17.000000000","message":"LGTM.","commit_id":"ffbb155effa19c672f26892d32d7f5db10221bb7"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"ac405202f6e85b214e5f610d4ccfe0a8c4e399d8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"8fd0ed5e_6a2ff016","updated":"2024-02-26 03:14:46.000000000","message":"This is looking good, I like the refactor and it is a parent of another patch that we want to land.\n\nJust have a question about the movement of one bit of code. Maybe I just missed something?","commit_id":"ffbb155effa19c672f26892d32d7f5db10221bb7"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"b2a2e131f9093b8ba168bfb4876d470b08ccd766","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":25,"id":"e61b13c3_6973f952","updated":"2024-02-27 00:52:37.000000000","message":"Adding a +A because of Jianjian\u0027s +1 previously.","commit_id":"8061dfb1c38de338f7eee29b0819c26825e31a3a"}],"swift/proxy/controllers/base.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0fa448242cae1d2b2b16df6f2170c03c6d4e4936","unresolved":true,"context_lines":[{"line_number":1164,"context_line":"                if not self._replace_source("},{"line_number":1165,"context_line":"                        \u0027Trying to read next part of %s multi-part GET \u0027"},{"line_number":1166,"context_line":"                        \u0027(retrying)\u0027 % self.resource_type):"},{"line_number":1167,"context_line":"                    raise"},{"line_number":1168,"context_line":""},{"line_number":1169,"context_line":"    def fast_forward(self, num_bytes):"},{"line_number":1170,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"92269c56_8cef3d24","line":1167,"updated":"2023-08-09 17:24:06.000000000","message":"see GetOrHeadHandler for replicated policy specialisation of what is raised.\n\nMore generally, I wonder if raising a new Exception type (\"NoMoreSources\" - that\u0027s the reason we\u0027re bailing out) would be clearer than ChunkReadTimeout  (that\u0027s the reason the last source failed)??","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"86411821381385aef077d8b04f7bd7d48e470f3e","unresolved":false,"context_lines":[{"line_number":1164,"context_line":"                if not self._replace_source("},{"line_number":1165,"context_line":"                        \u0027Trying to read next part of %s multi-part GET \u0027"},{"line_number":1166,"context_line":"                        \u0027(retrying)\u0027 % self.resource_type):"},{"line_number":1167,"context_line":"                    raise"},{"line_number":1168,"context_line":""},{"line_number":1169,"context_line":"    def fast_forward(self, num_bytes):"},{"line_number":1170,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"63313bc9_859f13d7","line":1167,"in_reply_to":"92269c56_8cef3d24","updated":"2024-02-07 11:12:42.000000000","message":"Done","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0fa448242cae1d2b2b16df6f2170c03c6d4e4936","unresolved":true,"context_lines":[{"line_number":1317,"context_line":"        try:"},{"line_number":1318,"context_line":"            return super(GetOrHeadHandler, self)._get_next_response_part()"},{"line_number":1319,"context_line":"        except ChunkReadTimeout:"},{"line_number":1320,"context_line":"            raise StopIteration()"},{"line_number":1321,"context_line":""},{"line_number":1322,"context_line":"    def _iter_bytes_from_response_part(self, part_file, nbytes):"},{"line_number":1323,"context_line":"        # yield chunks of bytes from a single response part; if an error"}],"source_content_type":"text/x-python","patch_set":1,"id":"52ae8fbe_9cb23fe8","line":1320,"updated":"2023-08-09 17:24:06.000000000","message":"I\u0027m not tremendously happy about this shim, but AFAICT it is necessary - or we stop re-raising the ChunkReadTimeout that is caught in `_iter_parts_from_response` line 1399","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f2fe782853aaeca78f7fa49da9786094620ed27c","unresolved":true,"context_lines":[{"line_number":1317,"context_line":"        try:"},{"line_number":1318,"context_line":"            return super(GetOrHeadHandler, self)._get_next_response_part()"},{"line_number":1319,"context_line":"        except ChunkReadTimeout:"},{"line_number":1320,"context_line":"            raise StopIteration()"},{"line_number":1321,"context_line":""},{"line_number":1322,"context_line":"    def _iter_bytes_from_response_part(self, part_file, nbytes):"},{"line_number":1323,"context_line":"        # yield chunks of bytes from a single response part; if an error"}],"source_content_type":"text/x-python","patch_set":1,"id":"ab5aead4_22d3134f","line":1320,"in_reply_to":"52ae8fbe_9cb23fe8","updated":"2023-10-02 17:50:57.000000000","message":"what\u0027s on line 1399?  A ChunkWriteTimeout?","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"268a8f5de699dc7cf726cacc40d68d971194cb5e","unresolved":true,"context_lines":[{"line_number":1317,"context_line":"        try:"},{"line_number":1318,"context_line":"            return super(GetOrHeadHandler, self)._get_next_response_part()"},{"line_number":1319,"context_line":"        except ChunkReadTimeout:"},{"line_number":1320,"context_line":"            raise StopIteration()"},{"line_number":1321,"context_line":""},{"line_number":1322,"context_line":"    def _iter_bytes_from_response_part(self, part_file, nbytes):"},{"line_number":1323,"context_line":"        # yield chunks of bytes from a single response part; if an error"}],"source_content_type":"text/x-python","patch_set":1,"id":"da60c18e_82a3690a","line":1320,"in_reply_to":"ab5aead4_22d3134f","updated":"2023-10-03 15:23:05.000000000","message":"If a ChunkReadTimeout is raised here then it is re-raised at line 1397 in this patchset, and then AFAICT not caught. Raising StopIteration here somehow avoids spewing a traceback.\n\nI think I was wondering if we could raise ChunkReadTimeout here and then avoid the traceback NOT re-raising it at line 1397.","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c75f9d6656a0b25ce7239f490ac3206df624707d","unresolved":true,"context_lines":[{"line_number":1317,"context_line":"        try:"},{"line_number":1318,"context_line":"            return super(GetOrHeadHandler, self)._get_next_response_part()"},{"line_number":1319,"context_line":"        except ChunkReadTimeout:"},{"line_number":1320,"context_line":"            raise StopIteration()"},{"line_number":1321,"context_line":""},{"line_number":1322,"context_line":"    def _iter_bytes_from_response_part(self, part_file, nbytes):"},{"line_number":1323,"context_line":"        # yield chunks of bytes from a single response part; if an error"}],"source_content_type":"text/x-python","patch_set":1,"id":"efd4088c_6ffd187a","line":1320,"in_reply_to":"da60c18e_82a3690a","updated":"2023-10-03 20:26:08.000000000","message":"yeah, I think the downstream effects of not translating ChunkReadTimeout to StopIteration are better dealt with downstream - this just looks wrong/confusing.","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c4f0c5cd079cb43837d6d724b6d648efa1d555a5","unresolved":false,"context_lines":[{"line_number":1317,"context_line":"        try:"},{"line_number":1318,"context_line":"            return super(GetOrHeadHandler, self)._get_next_response_part()"},{"line_number":1319,"context_line":"        except ChunkReadTimeout:"},{"line_number":1320,"context_line":"            raise StopIteration()"},{"line_number":1321,"context_line":""},{"line_number":1322,"context_line":"    def _iter_bytes_from_response_part(self, part_file, nbytes):"},{"line_number":1323,"context_line":"        # yield chunks of bytes from a single response part; if an error"}],"source_content_type":"text/x-python","patch_set":1,"id":"e5d44934_5087d870","line":1320,"in_reply_to":"efd4088c_6ffd187a","updated":"2023-10-06 16:57:42.000000000","message":"now topic of pre-patch","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0fa448242cae1d2b2b16df6f2170c03c6d4e4936","unresolved":true,"context_lines":[{"line_number":1446,"context_line":"            return False"},{"line_number":1447,"context_line":"        node_timeout \u003d self.app.node_timeout"},{"line_number":1448,"context_line":"        if self.server_type \u003d\u003d \u0027Object\u0027 and not self.newest:"},{"line_number":1449,"context_line":"            node_timeout \u003d self.app.recoverable_node_timeout"},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"        req_headers \u003d dict(self.backend_headers)"},{"line_number":1452,"context_line":"        ip, port \u003d get_ip_port(node, req_headers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"6f4414ef_7942c1db","line":1449,"updated":"2023-08-09 17:24:06.000000000","message":"I don\u0027t yet understand the significance of newest w.r.t. the choice of timeout - can anyone help me? It\u0027s frustrating that we cannot just use self.node_timeout in this method.","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f2fe782853aaeca78f7fa49da9786094620ed27c","unresolved":true,"context_lines":[{"line_number":1446,"context_line":"            return False"},{"line_number":1447,"context_line":"        node_timeout \u003d self.app.node_timeout"},{"line_number":1448,"context_line":"        if self.server_type \u003d\u003d \u0027Object\u0027 and not self.newest:"},{"line_number":1449,"context_line":"            node_timeout \u003d self.app.recoverable_node_timeout"},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"        req_headers \u003d dict(self.backend_headers)"},{"line_number":1452,"context_line":"        ip, port \u003d get_ip_port(node, req_headers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3c3c119c_3f487ea7","line":1449,"in_reply_to":"1f1098e9_fac1f977","updated":"2023-10-02 17:50:57.000000000","message":"maybe it was defensive when we were adding concurrent GETs and we were trying to be lazy and not think to hard about \"x-newest\"","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"268a8f5de699dc7cf726cacc40d68d971194cb5e","unresolved":true,"context_lines":[{"line_number":1446,"context_line":"            return False"},{"line_number":1447,"context_line":"        node_timeout \u003d self.app.node_timeout"},{"line_number":1448,"context_line":"        if self.server_type \u003d\u003d \u0027Object\u0027 and not self.newest:"},{"line_number":1449,"context_line":"            node_timeout \u003d self.app.recoverable_node_timeout"},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"        req_headers \u003d dict(self.backend_headers)"},{"line_number":1452,"context_line":"        ip, port \u003d get_ip_port(node, req_headers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7ec8f823_a4085eb4","line":1449,"in_reply_to":"3c3c119c_3f487ea7","updated":"2023-10-03 15:23:05.000000000","message":"Makes sense that for an x-newest request we\u0027d try harder to get a response from every node.\n\nGiven that we won\u0027t try to resume with x-newest, should we also wait a bit longer when reading in _iter_bytes_from_response_part?","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"eae56536dd743793ccc2ed0e46aa06ca58234b8c","unresolved":true,"context_lines":[{"line_number":1446,"context_line":"            return False"},{"line_number":1447,"context_line":"        node_timeout \u003d self.app.node_timeout"},{"line_number":1448,"context_line":"        if self.server_type \u003d\u003d \u0027Object\u0027 and not self.newest:"},{"line_number":1449,"context_line":"            node_timeout \u003d self.app.recoverable_node_timeout"},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"        req_headers \u003d dict(self.backend_headers)"},{"line_number":1452,"context_line":"        ip, port \u003d get_ip_port(node, req_headers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f1098e9_fac1f977","line":1449,"in_reply_to":"6413b745_bc142c96","updated":"2023-09-21 13:24:38.000000000","message":"Note that recoverable_node_timeout is always used in _iter_bytes_from_response_part (x-newest or not). Which I\u0027m not sure makes sense but with x-newest we won\u0027t try to resume a GET, so ought to wait longer once we\u0027re reading the body??\n\nI like the idea of moving this logic to __init__ but will wait to see if actually we need to straighten out that anomaly w.r.t. _iter_bytes_from_response_part","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"bfbc38387c25c18c1c23bf438d338858704eb527","unresolved":true,"context_lines":[{"line_number":1446,"context_line":"            return False"},{"line_number":1447,"context_line":"        node_timeout \u003d self.app.node_timeout"},{"line_number":1448,"context_line":"        if self.server_type \u003d\u003d \u0027Object\u0027 and not self.newest:"},{"line_number":1449,"context_line":"            node_timeout \u003d self.app.recoverable_node_timeout"},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"        req_headers \u003d dict(self.backend_headers)"},{"line_number":1452,"context_line":"        ip, port \u003d get_ip_port(node, req_headers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"6413b745_bc142c96","line":1449,"in_reply_to":"6f4414ef_7942c1db","updated":"2023-09-19 16:38:40.000000000","message":"I\u0027m not sure, but if I had to guess, since we know `X-Newest` requests are extra-expensive, we ought to have them be fairly expensive for the client, too, by keeping it serial. Prevent some force-multiplication?\n\nMaybe we could add the newest check to `GetOrHeadHandler.__init__`, too?","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c75f9d6656a0b25ce7239f490ac3206df624707d","unresolved":true,"context_lines":[{"line_number":1446,"context_line":"            return False"},{"line_number":1447,"context_line":"        node_timeout \u003d self.app.node_timeout"},{"line_number":1448,"context_line":"        if self.server_type \u003d\u003d \u0027Object\u0027 and not self.newest:"},{"line_number":1449,"context_line":"            node_timeout \u003d self.app.recoverable_node_timeout"},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"        req_headers \u003d dict(self.backend_headers)"},{"line_number":1452,"context_line":"        ip, port \u003d get_ip_port(node, req_headers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"97336400_72df1fd8","line":1449,"in_reply_to":"7ec8f823_a4085eb4","updated":"2023-10-03 20:26:08.000000000","message":"won\u0027t try to resume with x-newest!?  I think the different timeouts is mostly only confusing\n\n```\nsudo grep timeout /etc/swift/proxy-server.conf \nclient_timeout \u003d 60.0\nkeepalive_timeout \u003d 10.0\nnode_timeout \u003d 16.0\nrecoverable_node_timeout \u003d 16.0\nconn_timeout \u003d 2.0\npost_quorum_timeout \u003d 0.1\nconcurrency_timeout \u003d 0.5\nconcurrency_timeout \u003d 0.5\nconcurrency_timeout \u003d 0.5\nconcurrency_timeout \u003d 0.5\nconcurrency_timeout \u003d 0.5\nconcurrency_timeout \u003d 0.5\nconcurrency_timeout \u003d 0.5\nconcurrency_timeout \u003d 0.5\ncache_timeout \u003d 300\n```","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"86411821381385aef077d8b04f7bd7d48e470f3e","unresolved":true,"context_lines":[{"line_number":1446,"context_line":"            return False"},{"line_number":1447,"context_line":"        node_timeout \u003d self.app.node_timeout"},{"line_number":1448,"context_line":"        if self.server_type \u003d\u003d \u0027Object\u0027 and not self.newest:"},{"line_number":1449,"context_line":"            node_timeout \u003d self.app.recoverable_node_timeout"},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"        req_headers \u003d dict(self.backend_headers)"},{"line_number":1452,"context_line":"        ip, port \u003d get_ip_port(node, req_headers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"a521d285_38ae50d3","line":1449,"in_reply_to":"97336400_72df1fd8","updated":"2024-02-07 11:12:42.000000000","message":"I think I\u0027ve convinced myself that this \u0027special case\u0027 should be more generally applied when x-newest is sent. i.e. if x-newest then do not use recoverable_node_timeout, because the request is not recoverable; wait longer before giving up entirely.\n\nFor now, I have kept the change separate https://review.opendev.org/c/openstack/swift/+/908287/1?usp\u003drelated-change","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"1fb235e88afed2db41d029b037a5cc2b6dad1e5d","unresolved":true,"context_lines":[{"line_number":1446,"context_line":"            return False"},{"line_number":1447,"context_line":"        node_timeout \u003d self.app.node_timeout"},{"line_number":1448,"context_line":"        if self.server_type \u003d\u003d \u0027Object\u0027 and not self.newest:"},{"line_number":1449,"context_line":"            node_timeout \u003d self.app.recoverable_node_timeout"},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"        req_headers \u003d dict(self.backend_headers)"},{"line_number":1452,"context_line":"        ip, port \u003d get_ip_port(node, req_headers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"cde5bb01_76708788","line":1449,"in_reply_to":"a521d285_38ae50d3","updated":"2024-02-22 02:30:24.000000000","message":"I am also baffled by the calculation of ``node_timeout`` here.","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"1a29a3eaf077aff5b7c73ce9a5e22bcea1d595f5","unresolved":true,"context_lines":[{"line_number":1446,"context_line":"            return False"},{"line_number":1447,"context_line":"        node_timeout \u003d self.app.node_timeout"},{"line_number":1448,"context_line":"        if self.server_type \u003d\u003d \u0027Object\u0027 and not self.newest:"},{"line_number":1449,"context_line":"            node_timeout \u003d self.app.recoverable_node_timeout"},{"line_number":1450,"context_line":""},{"line_number":1451,"context_line":"        req_headers \u003d dict(self.backend_headers)"},{"line_number":1452,"context_line":"        ip, port \u003d get_ip_port(node, req_headers)"}],"source_content_type":"text/x-python","patch_set":1,"id":"632aa3e8_5d978f13","line":1449,"in_reply_to":"cde5bb01_76708788","updated":"2024-02-23 01:32:17.000000000","message":"okay, this is kept as same as before. the new fix is in https://review.opendev.org/c/openstack/swift/+/908287.","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"bfbc38387c25c18c1c23bf438d338858704eb527","unresolved":true,"context_lines":[{"line_number":1114,"context_line":"        self.logger \u003d logger or app.logger"},{"line_number":1115,"context_line":"        self.bytes_used_from_backend \u003d 0"},{"line_number":1116,"context_line":"        self.source \u003d None"},{"line_number":1117,"context_line":"        if policy is not None and policy.policy_type \u003d\u003d EC_POLICY:"},{"line_number":1118,"context_line":"            self.resource_type \u003d \u0027EC fragment\u0027"},{"line_number":1119,"context_line":"        else:"},{"line_number":1120,"context_line":"            self.resource_type \u003d \u0027object\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"f4abb624_0e00d108","line":1117,"range":{"start_line":1117,"start_character":11,"end_line":1117,"end_character":29},"updated":"2023-09-19 16:38:40.000000000","message":"This seems like a smell -- when would `policy` be `None`? ... A/C requests, maybe? Certainly, the base handler\u0027s `GETorHEAD_base` uses `GetOrHeadHandler`, which is-a `GetterBase`...\n\nSo, are we setting `resource_type \u003d \u0027object\u0027` even for A/C requests?","commit_id":"d068a553d6a3b42bb0bfff3b4a78c3d01b49eb73"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c4f0c5cd079cb43837d6d724b6d648efa1d555a5","unresolved":false,"context_lines":[{"line_number":1114,"context_line":"        self.logger \u003d logger or app.logger"},{"line_number":1115,"context_line":"        self.bytes_used_from_backend \u003d 0"},{"line_number":1116,"context_line":"        self.source \u003d None"},{"line_number":1117,"context_line":"        if policy is not None and policy.policy_type \u003d\u003d EC_POLICY:"},{"line_number":1118,"context_line":"            self.resource_type \u003d \u0027EC fragment\u0027"},{"line_number":1119,"context_line":"        else:"},{"line_number":1120,"context_line":"            self.resource_type \u003d \u0027object\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"cef199fe_49973d16","line":1117,"range":{"start_line":1117,"start_character":11,"end_line":1117,"end_character":29},"in_reply_to":"34737a58_6355aea4","updated":"2023-10-06 16:57:42.000000000","message":"Done","commit_id":"d068a553d6a3b42bb0bfff3b4a78c3d01b49eb73"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"eae56536dd743793ccc2ed0e46aa06ca58234b8c","unresolved":true,"context_lines":[{"line_number":1114,"context_line":"        self.logger \u003d logger or app.logger"},{"line_number":1115,"context_line":"        self.bytes_used_from_backend \u003d 0"},{"line_number":1116,"context_line":"        self.source \u003d None"},{"line_number":1117,"context_line":"        if policy is not None and policy.policy_type \u003d\u003d EC_POLICY:"},{"line_number":1118,"context_line":"            self.resource_type \u003d \u0027EC fragment\u0027"},{"line_number":1119,"context_line":"        else:"},{"line_number":1120,"context_line":"            self.resource_type \u003d \u0027object\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"34737a58_6355aea4","line":1117,"range":{"start_line":1117,"start_character":11,"end_line":1117,"end_character":29},"in_reply_to":"f4abb624_0e00d108","updated":"2023-09-21 13:24:38.000000000","message":"1. I think it would be better to pass resource_type rather than this kludge.\n\n2. It is the existing behaviour that it\u0027s always \u0027object\u0027 for replicated, event container and account, but it\u0027s moot: the var is only used for logging messages that only occur for object server type.\n\nSee https://review.opendev.org/c/openstack/swift/+/896098 follow-up to maybe make this more palatable?","commit_id":"d068a553d6a3b42bb0bfff3b4a78c3d01b49eb73"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"38d87ecfe56176e1ebe65e16862e89c26b9a6307","unresolved":true,"context_lines":[{"line_number":1137,"context_line":"            self.source.close()"},{"line_number":1138,"context_line":"        return self._find_source()"},{"line_number":1139,"context_line":""},{"line_number":1140,"context_line":"    def _get_next_response_part(self):"},{"line_number":1141,"context_line":"        # return the next part of the response body; there may only be one part"},{"line_number":1142,"context_line":"        # unless it\u0027s a multipart/byteranges response"},{"line_number":1143,"context_line":"        while True:"}],"source_content_type":"text/x-python","patch_set":3,"id":"563636e3_f4e54475","line":1140,"updated":"2023-09-19 00:34:13.000000000","message":"should we remove the prefix and rename it to \"get_next_response_part\"? since child classes will call this method.","commit_id":"d068a553d6a3b42bb0bfff3b4a78c3d01b49eb73"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7f3287aef40ac69bf788d09ea9e0c50c5af19aaf","unresolved":false,"context_lines":[{"line_number":1137,"context_line":"            self.source.close()"},{"line_number":1138,"context_line":"        return self._find_source()"},{"line_number":1139,"context_line":""},{"line_number":1140,"context_line":"    def _get_next_response_part(self):"},{"line_number":1141,"context_line":"        # return the next part of the response body; there may only be one part"},{"line_number":1142,"context_line":"        # unless it\u0027s a multipart/byteranges response"},{"line_number":1143,"context_line":"        while True:"}],"source_content_type":"text/x-python","patch_set":3,"id":"24a36b08_314bce59","line":1140,"in_reply_to":"44d34ce9_8edf845f","updated":"2023-09-19 09:31:20.000000000","message":"I think of the single underscore prefix convention being equivalent to \"protected\", and therefore \"available\" to subclasses (of course, everything in Python is available, so I mean available-by-convention).\n\nRemoving the \u0027_\u0027 would suggest that the method is public, which is not the intent here. So I\u0027d prefer to keep the \u0027_\u0027. However, I acknowledge that the Swift codebase is not consistent in use of leading \u0027_\u0027: I think we\u0027re pretty good (not perfect) at not calling \u0027_foo()\u0027 from outside of (sub)classes, but we\u0027re not so good at ensuring that all \u0027protected\u0027 methods do have \u0027_\u0027.","commit_id":"d068a553d6a3b42bb0bfff3b4a78c3d01b49eb73"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"a9f041208d826101d88b44a5e6ea569bf700b590","unresolved":true,"context_lines":[{"line_number":1137,"context_line":"            self.source.close()"},{"line_number":1138,"context_line":"        return self._find_source()"},{"line_number":1139,"context_line":""},{"line_number":1140,"context_line":"    def _get_next_response_part(self):"},{"line_number":1141,"context_line":"        # return the next part of the response body; there may only be one part"},{"line_number":1142,"context_line":"        # unless it\u0027s a multipart/byteranges response"},{"line_number":1143,"context_line":"        while True:"}],"source_content_type":"text/x-python","patch_set":3,"id":"44d34ce9_8edf845f","line":1140,"in_reply_to":"563636e3_f4e54475","updated":"2023-09-19 06:38:53.000000000","message":"@jianjain feel free to vote with a -1. If you think there is something that needs to be fixed. If you turn out to be mistaken you can always come back and change your vote 😊 (I was told this by John many years ago, because I\u0027d vote 0 or +1 alot and not want to -1 (red pen) anyones patches).\n\nVoting 0, is a good for questions though.\n\nSometimes if its something we don\u0027t think is a blocker (block the patch from landing), could be fixed in a follow up or if another patchset needs to be applied then we say in the comment its a NIT (or nitpick). \n\nIn other words, we know you well enough to know a -1 isn\u0027t a critism from you 😊\n\n\u0027_\u0027 in python tends to be seen as a private method, we don\u0027t have a protected. When I dont see a \u0027_\u0027 is cold be mistaken as a public method/interface for this class. So I don\u0027t mind too much if it keeps the \u0027_\u0027. I\u0027ll let Al decide.","commit_id":"d068a553d6a3b42bb0bfff3b4a78c3d01b49eb73"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f2fe782853aaeca78f7fa49da9786094620ed27c","unresolved":true,"context_lines":[{"line_number":1296,"context_line":"            except ChunkReadTimeout:"},{"line_number":1297,"context_line":"                if not self._replace_source("},{"line_number":1298,"context_line":"                        \u0027Trying to read object during GET (retrying)\u0027):"},{"line_number":1299,"context_line":"                    raise StopIteration()"},{"line_number":1300,"context_line":""},{"line_number":1301,"context_line":"    def _iter_bytes_from_response_part(self, part_file, nbytes):"},{"line_number":1302,"context_line":"        # yield chunks of bytes from a single response part; if an error"}],"source_content_type":"text/x-python","patch_set":8,"id":"e3fafcfd_1e283b5e","side":"PARENT","line":1299,"updated":"2023-10-02 17:50:57.000000000","message":"it makes sense to me that ChunkReadTimeout on a GET is a backend timeout recoverable retry - it doesn\u0027t make sense to me that we can translate it to StopIteratoin (I think that\u0027s the bug the Related-Changed fixed, for EC only?)","commit_id":"f8c94d6bbcf1ea13a3ef43e1e57380780a24d2a4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c4f0c5cd079cb43837d6d724b6d648efa1d555a5","unresolved":false,"context_lines":[{"line_number":1296,"context_line":"            except ChunkReadTimeout:"},{"line_number":1297,"context_line":"                if not self._replace_source("},{"line_number":1298,"context_line":"                        \u0027Trying to read object during GET (retrying)\u0027):"},{"line_number":1299,"context_line":"                    raise StopIteration()"},{"line_number":1300,"context_line":""},{"line_number":1301,"context_line":"    def _iter_bytes_from_response_part(self, part_file, nbytes):"},{"line_number":1302,"context_line":"        # yield chunks of bytes from a single response part; if an error"}],"source_content_type":"text/x-python","patch_set":8,"id":"78a824c7_350ade1b","side":"PARENT","line":1299,"in_reply_to":"659d774a_62e39664","updated":"2023-10-06 16:57:42.000000000","message":"now topic of a pre-patch","commit_id":"f8c94d6bbcf1ea13a3ef43e1e57380780a24d2a4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c75f9d6656a0b25ce7239f490ac3206df624707d","unresolved":true,"context_lines":[{"line_number":1296,"context_line":"            except ChunkReadTimeout:"},{"line_number":1297,"context_line":"                if not self._replace_source("},{"line_number":1298,"context_line":"                        \u0027Trying to read object during GET (retrying)\u0027):"},{"line_number":1299,"context_line":"                    raise StopIteration()"},{"line_number":1300,"context_line":""},{"line_number":1301,"context_line":"    def _iter_bytes_from_response_part(self, part_file, nbytes):"},{"line_number":1302,"context_line":"        # yield chunks of bytes from a single response part; if an error"}],"source_content_type":"text/x-python","patch_set":8,"id":"659d774a_62e39664","side":"PARENT","line":1299,"in_reply_to":"961e7822_effd6b4d","updated":"2023-10-03 20:26:08.000000000","message":"oh wow, what a subtle difference - good on the EC path for getting the client error out while it still can.\n\nI would expect the equivilant in the replicated path would be a ChunkReadTimeout while getting *the first* chunk - is it still going to be to late to call start_response with the correct status code?\n\nRegardless of the downstream application in wsgi app_iter land, since we\u0027re reusing this code as backend (Resuming)ECFrag/Replicated-Getter it would be nice if there was a clear seperation of \"these are all the bytes\" and \"i failed to resume and can\u0027t provide anymore bytes\" - then on the backside let some new replicated-only code path do whatever EcAppIter does to make wsgi happy when we\u0027re going to fail.","commit_id":"f8c94d6bbcf1ea13a3ef43e1e57380780a24d2a4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"268a8f5de699dc7cf726cacc40d68d971194cb5e","unresolved":true,"context_lines":[{"line_number":1296,"context_line":"            except ChunkReadTimeout:"},{"line_number":1297,"context_line":"                if not self._replace_source("},{"line_number":1298,"context_line":"                        \u0027Trying to read object during GET (retrying)\u0027):"},{"line_number":1299,"context_line":"                    raise StopIteration()"},{"line_number":1300,"context_line":""},{"line_number":1301,"context_line":"    def _iter_bytes_from_response_part(self, part_file, nbytes):"},{"line_number":1302,"context_line":"        # yield chunks of bytes from a single response part; if an error"}],"source_content_type":"text/x-python","patch_set":8,"id":"961e7822_effd6b4d","side":"PARENT","line":1299,"in_reply_to":"e3fafcfd_1e283b5e","updated":"2023-10-03 15:23:05.000000000","message":"If we get to this line then retrying has failed (we didn\u0027t manage to replace the source).\n\nIt seems the related change raised the ChunkReadTimeout instead of StopIteration from ECFragGetter. As a result the client gets a 500 rather than a truncated body (at least in the scenario of TestECObjController.test_GET_with_multirange_unable_to_resume, where the ChunkReadTimeout means the proxy cannot assemble enough frag getters).\n\nI *think* the difference is that the EC path (in the test scenario) hasn\u0027t started the client response, so it can step in and return a 500.\n\nBut on the replicated path (here) the client response has started, so it\u0027s too late to return a 500, and the ChunkReadTimeout would spew a traceback.","commit_id":"f8c94d6bbcf1ea13a3ef43e1e57380780a24d2a4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f2fe782853aaeca78f7fa49da9786094620ed27c","unresolved":true,"context_lines":[{"line_number":1530,"context_line":"        node_timeout \u003d self.app.node_timeout"},{"line_number":1531,"context_line":"        if self.server_type \u003d\u003d \u0027Object\u0027 and not self.newest:"},{"line_number":1532,"context_line":"            node_timeout \u003d self.app.recoverable_node_timeout"},{"line_number":1533,"context_line":""},{"line_number":1534,"context_line":"        pile \u003d GreenAsyncPile(self.concurrency)"},{"line_number":1535,"context_line":""},{"line_number":1536,"context_line":"        for node in nodes:"}],"source_content_type":"text/x-python","patch_set":8,"id":"719f6524_eadd46fc","side":"PARENT","line":1533,"updated":"2023-10-02 17:50:57.000000000","message":"when i saw this red i was hoping it ended up in `__init__`","commit_id":"f8c94d6bbcf1ea13a3ef43e1e57380780a24d2a4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"268a8f5de699dc7cf726cacc40d68d971194cb5e","unresolved":true,"context_lines":[{"line_number":1530,"context_line":"        node_timeout \u003d self.app.node_timeout"},{"line_number":1531,"context_line":"        if self.server_type \u003d\u003d \u0027Object\u0027 and not self.newest:"},{"line_number":1532,"context_line":"            node_timeout \u003d self.app.recoverable_node_timeout"},{"line_number":1533,"context_line":""},{"line_number":1534,"context_line":"        pile \u003d GreenAsyncPile(self.concurrency)"},{"line_number":1535,"context_line":""},{"line_number":1536,"context_line":"        for node in nodes:"}],"source_content_type":"text/x-python","patch_set":8,"id":"da7f6b92_82501609","side":"PARENT","line":1533,"in_reply_to":"719f6524_eadd46fc","updated":"2023-10-03 15:23:05.000000000","message":"I\u0027d like it to but with x-newest we currently use both node_timeout and recoverable_node_timeout in the class!","commit_id":"f8c94d6bbcf1ea13a3ef43e1e57380780a24d2a4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"86411821381385aef077d8b04f7bd7d48e470f3e","unresolved":false,"context_lines":[{"line_number":1530,"context_line":"        node_timeout \u003d self.app.node_timeout"},{"line_number":1531,"context_line":"        if self.server_type \u003d\u003d \u0027Object\u0027 and not self.newest:"},{"line_number":1532,"context_line":"            node_timeout \u003d self.app.recoverable_node_timeout"},{"line_number":1533,"context_line":""},{"line_number":1534,"context_line":"        pile \u003d GreenAsyncPile(self.concurrency)"},{"line_number":1535,"context_line":""},{"line_number":1536,"context_line":"        for node in nodes:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ffb9b755_412900f6","side":"PARENT","line":1533,"in_reply_to":"da7f6b92_82501609","updated":"2024-02-07 11:12:42.000000000","message":"Done","commit_id":"f8c94d6bbcf1ea13a3ef43e1e57380780a24d2a4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f2fe782853aaeca78f7fa49da9786094620ed27c","unresolved":true,"context_lines":[{"line_number":1274,"context_line":"        if server_type \u003d\u003d \u0027Object\u0027:"},{"line_number":1275,"context_line":"            node_timeout \u003d app.recoverable_node_timeout"},{"line_number":1276,"context_line":"        else:"},{"line_number":1277,"context_line":"            node_timeout \u003d app.node_timeout"},{"line_number":1278,"context_line":"        super(GetOrHeadHandler, self).__init__("},{"line_number":1279,"context_line":"            app\u003dapp, req\u003dreq, node_iter\u003dnode_iter,"},{"line_number":1280,"context_line":"            partition\u003dpartition, policy\u003dpolicy, path\u003dpath,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ef6912f2_72b56351","line":1277,"updated":"2023-10-02 17:50:57.000000000","message":"this make sense;","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c4f0c5cd079cb43837d6d724b6d648efa1d555a5","unresolved":false,"context_lines":[{"line_number":1274,"context_line":"        if server_type \u003d\u003d \u0027Object\u0027:"},{"line_number":1275,"context_line":"            node_timeout \u003d app.recoverable_node_timeout"},{"line_number":1276,"context_line":"        else:"},{"line_number":1277,"context_line":"            node_timeout \u003d app.node_timeout"},{"line_number":1278,"context_line":"        super(GetOrHeadHandler, self).__init__("},{"line_number":1279,"context_line":"            app\u003dapp, req\u003dreq, node_iter\u003dnode_iter,"},{"line_number":1280,"context_line":"            partition\u003dpartition, policy\u003dpolicy, path\u003dpath,"}],"source_content_type":"text/x-python","patch_set":8,"id":"d93328c2_a6da9db5","line":1277,"in_reply_to":"ef6912f2_72b56351","updated":"2023-10-06 16:57:42.000000000","message":"Ack","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f2fe782853aaeca78f7fa49da9786094620ed27c","unresolved":true,"context_lines":[{"line_number":1279,"context_line":"            app\u003dapp, req\u003dreq, node_iter\u003dnode_iter,"},{"line_number":1280,"context_line":"            partition\u003dpartition, policy\u003dpolicy, path\u003dpath,"},{"line_number":1281,"context_line":"            backend_headers\u003dbackend_headers, node_timeout\u003dnode_timeout,"},{"line_number":1282,"context_line":"            resource_type\u003d\u0027object\u0027,"},{"line_number":1283,"context_line":"            logger\u003dlogger)"},{"line_number":1284,"context_line":"        self.server_type \u003d server_type"},{"line_number":1285,"context_line":"        self.used_nodes \u003d []"}],"source_content_type":"text/x-python","patch_set":8,"id":"90ef93e4_07a89c01","line":1282,"updated":"2023-10-02 17:50:57.000000000","message":"hard-coding resource-type here doesn\u0027t make sense","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c75f9d6656a0b25ce7239f490ac3206df624707d","unresolved":true,"context_lines":[{"line_number":1279,"context_line":"            app\u003dapp, req\u003dreq, node_iter\u003dnode_iter,"},{"line_number":1280,"context_line":"            partition\u003dpartition, policy\u003dpolicy, path\u003dpath,"},{"line_number":1281,"context_line":"            backend_headers\u003dbackend_headers, node_timeout\u003dnode_timeout,"},{"line_number":1282,"context_line":"            resource_type\u003d\u0027object\u0027,"},{"line_number":1283,"context_line":"            logger\u003dlogger)"},{"line_number":1284,"context_line":"        self.server_type \u003d server_type"},{"line_number":1285,"context_line":"        self.used_nodes \u003d []"}],"source_content_type":"text/x-python","patch_set":8,"id":"3715fefe_eccad610","line":1282,"in_reply_to":"1d8c5eb4_3efc515b","updated":"2023-10-03 20:26:08.000000000","message":"yeah squash it; let\u0027s get this code cleaned up!","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c4f0c5cd079cb43837d6d724b6d648efa1d555a5","unresolved":false,"context_lines":[{"line_number":1279,"context_line":"            app\u003dapp, req\u003dreq, node_iter\u003dnode_iter,"},{"line_number":1280,"context_line":"            partition\u003dpartition, policy\u003dpolicy, path\u003dpath,"},{"line_number":1281,"context_line":"            backend_headers\u003dbackend_headers, node_timeout\u003dnode_timeout,"},{"line_number":1282,"context_line":"            resource_type\u003d\u0027object\u0027,"},{"line_number":1283,"context_line":"            logger\u003dlogger)"},{"line_number":1284,"context_line":"        self.server_type \u003d server_type"},{"line_number":1285,"context_line":"        self.used_nodes \u003d []"}],"source_content_type":"text/x-python","patch_set":8,"id":"a830404c_eae760b9","line":1282,"in_reply_to":"3715fefe_eccad610","updated":"2023-10-06 16:57:42.000000000","message":"Done","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"268a8f5de699dc7cf726cacc40d68d971194cb5e","unresolved":true,"context_lines":[{"line_number":1279,"context_line":"            app\u003dapp, req\u003dreq, node_iter\u003dnode_iter,"},{"line_number":1280,"context_line":"            partition\u003dpartition, policy\u003dpolicy, path\u003dpath,"},{"line_number":1281,"context_line":"            backend_headers\u003dbackend_headers, node_timeout\u003dnode_timeout,"},{"line_number":1282,"context_line":"            resource_type\u003d\u0027object\u0027,"},{"line_number":1283,"context_line":"            logger\u003dlogger)"},{"line_number":1284,"context_line":"        self.server_type \u003d server_type"},{"line_number":1285,"context_line":"        self.used_nodes \u003d []"}],"source_content_type":"text/x-python","patch_set":8,"id":"1d8c5eb4_3efc515b","line":1282,"in_reply_to":"90ef93e4_07a89c01","updated":"2023-10-03 15:23:05.000000000","message":"It maybe makes sense once you understand that it\u0027s \u0027object\u0027 vs \u0027EC fragment\u0027 not \u0027object\u0027 vs \u0027container\u0027 vs \u0027account\u0027, which is why it\u0027s not named \u0027server_type\u0027. But it\u0027s not great :/\n\nThe follow on patch does generalise it to resource_type\u003dserver_type, which probably does make more sense to the reader, but is irrelevant to the code (until maybe we log something that isn\u0027t exclusively to do with object GET - the string is merely used when logging an object-GET specific error, but it\u0027s easy to forget that this class is used for container and account GETs too)  - see https://review.opendev.org/c/openstack/swift/+/896098 which I could squash in.","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f2fe782853aaeca78f7fa49da9786094620ed27c","unresolved":true,"context_lines":[{"line_number":1386,"context_line":"                           \u0027part_iter\u0027: part_iter}"},{"line_number":1387,"context_line":"                    self.pop_range()"},{"line_number":1388,"context_line":"            except StopIteration:"},{"line_number":1389,"context_line":"                req.environ[\u0027swift.non_client_disconnect\u0027] \u003d True"},{"line_number":1390,"context_line":"            finally:"},{"line_number":1391,"context_line":"                if part_iter:"},{"line_number":1392,"context_line":"                    part_iter.close()"}],"source_content_type":"text/x-python","patch_set":8,"id":"b25ddd56_9a13b8fd","line":1389,"updated":"2023-10-02 17:50:57.000000000","message":"I think StopIteration here is trying to say there is no more multi-range parts?","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c4f0c5cd079cb43837d6d724b6d648efa1d555a5","unresolved":false,"context_lines":[{"line_number":1386,"context_line":"                           \u0027part_iter\u0027: part_iter}"},{"line_number":1387,"context_line":"                    self.pop_range()"},{"line_number":1388,"context_line":"            except StopIteration:"},{"line_number":1389,"context_line":"                req.environ[\u0027swift.non_client_disconnect\u0027] \u003d True"},{"line_number":1390,"context_line":"            finally:"},{"line_number":1391,"context_line":"                if part_iter:"},{"line_number":1392,"context_line":"                    part_iter.close()"}],"source_content_type":"text/x-python","patch_set":8,"id":"43fd3df5_06273199","line":1389,"in_reply_to":"3c1db494_82c11c19","updated":"2023-10-06 16:57:42.000000000","message":"these changes are now in pre-patches","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"268a8f5de699dc7cf726cacc40d68d971194cb5e","unresolved":true,"context_lines":[{"line_number":1386,"context_line":"                           \u0027part_iter\u0027: part_iter}"},{"line_number":1387,"context_line":"                    self.pop_range()"},{"line_number":1388,"context_line":"            except StopIteration:"},{"line_number":1389,"context_line":"                req.environ[\u0027swift.non_client_disconnect\u0027] \u003d True"},{"line_number":1390,"context_line":"            finally:"},{"line_number":1391,"context_line":"                if part_iter:"},{"line_number":1392,"context_line":"                    part_iter.close()"}],"source_content_type":"text/x-python","patch_set":8,"id":"d9908e3e_364c5706","line":1389,"in_reply_to":"b25ddd56_9a13b8fd","updated":"2023-10-03 15:23:05.000000000","message":"yes, but looks like we get here in the happy path so not sure why we set \n\u0027swift.non_client_disconnect\u0027 ?? In fact, there is a comment saying exactly that in the EC path!!\n```\n\n(acoles) ~/0dev/openstack/swift{review/alistair_coles/p-get-path-cleanup,-,+,!} % git diff|cat\ndiff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py\nindex d43a4de5b..841fd6593 100644\n--- a/swift/proxy/controllers/base.py\n+++ b/swift/proxy/controllers/base.py\n@@ -1375,6 +1375,7 @@ class GetOrHeadHandler(GetterBase):\n                            \u0027part_iter\u0027: part_iter}\n                     self.pop_range()\n             except StopIteration:\n+                print(\u0027We got HERE\u0027)\n                 req.environ[\u0027swift.non_client_disconnect\u0027] \u003d True\n             finally:\n                 if part_iter:\ndiff --git a/test/unit/proxy/controllers/test_obj.py b/test/unit/proxy/controllers/test_obj.py\nindex 55ac07fb2..1a427fef9 100644\n--- a/test/unit/proxy/controllers/test_obj.py\n+++ b/test/unit/proxy/controllers/test_obj.py\n@@ -1904,6 +1904,7 @@ class TestReplicatedObjController(CommonObjectControllerMixin,\n             self.assertEqual(resp.status_int, 200)\n             resp_body \u003d resp.body\n         self.assertEqual(b\u0027length 8\u0027, resp_body)\n+        self.assertNotIn(\u0027swift.non_client_disconnect\u0027, req.environ)\n         self.assertEqual(len(log.requests), 3)\n         # NB: original range is not satisfiable but is ignored\n         self.assertEqual(\u0027bytes\u003d9-10, 20-30\u0027,\n\n\n(acoles) ~/0dev/openstack/swift{review/alistair_coles/p-get-path-cleanup,-,+,!} % pytest ./test/unit/proxy/controllers/test_obj.py::TestReplicatedObjController::test_GET_resuming --tb short\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d test session starts \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\nplatform darwin -- Python 3.8.15, pytest-7.2.0, pluggy-1.0.0 -- /Users/acoles/.pyenv/versions/3.8.15/envs/swift-3.8.15/bin/python3.8\ncachedir: .pytest_cache\nrootdir: /Users/acoles/0dev/openstack/swift, configfile: tox.ini\nplugins: cov-4.0.0\ncollected 1 item\n\ntest/unit/proxy/controllers/test_obj.py::TestReplicatedObjController::test_GET_resuming FAILED                                                     [100%]\n\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d FAILURES \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n_____________________________________________________ TestReplicatedObjController.test_GET_resuming ______________________________________________________\ntest/unit/proxy/controllers/test_obj.py:1907: in test_GET_resuming\n    self.assertNotIn(\u0027swift.non_client_disconnect\u0027, req.environ)\nE   AssertionError: \u0027swift.non_client_disconnect\u0027 unexpectedly found in {\u0027REQUEST_METHOD\u0027: \u0027GET\u0027, \u0027SCRIPT_NAME\u0027: \u0027\u0027, \u0027QUERY_STRING\u0027: \u0027\u0027, \u0027PATH_INFO\u0027: \u0027/v1/a/c/o\u0027, \u0027SERVER_NAME\u0027: \u0027localhost\u0027, \u0027SERVER_PORT\u0027: \u002780\u0027, \u0027HTTP_HOST\u0027: \u0027localhost:80\u0027, \u0027SERVER_PROTOCOL\u0027: \u0027HTTP/1.0\u0027, \u0027wsgi.version\u0027: (1, 0), \u0027wsgi.url_scheme\u0027: \u0027http\u0027, \u0027wsgi.errors\u0027: \u003c_io.StringIO object at 0x106c6d550\u003e, \u0027wsgi.multithread\u0027: False, \u0027wsgi.multiprocess\u0027: False, \u0027wsgi.input\u0027: \u003cswift.common.swob.WsgiBytesIO object at 0x106c81f90\u003e, \u0027HTTP_RANGE\u0027: \u0027bytes\u003d9-10, 20-30\u0027, \u0027HTTP_X_BACKEND_IGNORE_RANGE_IF_METADATA_PRESENT\u0027: \u0027X-Static-Large-Object\u0027, \u0027swift.infocache\u0027: {}, \u0027swift.trans_id\u0027: \u0027txde0de583dd4048b38f4bb-00651c2c24\u0027, \u0027HTTP_X_TRANS_ID\u0027: \u0027txde0de583dd4048b38f4bb-00651c2c24\u0027, \u0027swift.orig_req_method\u0027: \u0027GET\u0027, \u0027swob.ACL\u0027: None, \u0027HTTP_X_BACKEND_STORAGE_POLICY_INDEX\u0027: \u00270\u0027, \u0027swift_x_timestamp\u0027: \u00271696345124.00000\u0027, \u0027swift.non_client_disconnect\u0027: True}\n------------------------------------------------------------------ Captured stdout call ------------------------------------------------------------------\nproxy-server DEBUG: Loaded override config for (default): ProxyOverrideOptions({}, {\u0027sorting_method\u0027: \u0027shuffle\u0027, \u0027read_affinity\u0027: \u0027\u0027, \u0027write_affinity\u0027: \u0027\u0027, \u0027write_affinity_node_count\u0027: \u00272 * replicas\u0027, \u0027write_affinity_handoff_delete_count\u0027: None, \u0027rebalance_missing_suppression_count\u0027: 1, \u0027concurrent_gets\u0027: False, \u0027concurrency_timeout\u0027: 1.0, \u0027concurrent_ec_extra_requests\u0027: 0}, app) (txn: txn1) (client_ip: 127.0.0.2)\nproxy-server ERROR: Trying to read object during GET (retrying) 10.0.0.2:1002/sdc (txn: txde0de583dd4048b38f4bb-00651c2c24)\nproxy-server ERROR: Trying to read object during GET (retrying) 10.0.0.1:1001/sdb (txn: txde0de583dd4048b38f4bb-00651c2c24)\nWe got HERE\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d short test summary info \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\nFAILED test/unit/proxy/controllers/test_obj.py::TestReplicatedObjController::test_GET_resuming - AssertionError: \u0027swift.non_client_disconnect\u0027 unexpectedly found in {\u0027REQUEST_METHOD\u0027: \u0027GET\u0027, \u0027SCRIPT_NAME\u0027: \u0027\u0027, \u0027QUERY_STRING\u0027: \u0027\u0027, \u0027PATH_INFO\u0027: \u0027/v...\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d 1 failed in 0.58s \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n```","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c75f9d6656a0b25ce7239f490ac3206df624707d","unresolved":true,"context_lines":[{"line_number":1386,"context_line":"                           \u0027part_iter\u0027: part_iter}"},{"line_number":1387,"context_line":"                    self.pop_range()"},{"line_number":1388,"context_line":"            except StopIteration:"},{"line_number":1389,"context_line":"                req.environ[\u0027swift.non_client_disconnect\u0027] \u003d True"},{"line_number":1390,"context_line":"            finally:"},{"line_number":1391,"context_line":"                if part_iter:"},{"line_number":1392,"context_line":"                    part_iter.close()"}],"source_content_type":"text/x-python","patch_set":8,"id":"3c1db494_82c11c19","line":1389,"in_reply_to":"d9908e3e_364c5706","updated":"2023-10-03 20:26:08.000000000","message":"aside from the naming I\u0027m not sure i\u0027m too worried baout \"this_was_not_a_client_disconnect\" being present in the happy path.  But since it *was* named weird calling it out as \"don\u0027t be fooled, this isn\u0027t actually an error, StopIteration \u003d\u003d not an error, nothing to see here\" seemed reasonable and would be a useful comment to preserve.\n\n... unless we think we can drop the unusual env var setting in the happy path altogether once we get StopIteration to always mean happy ending and then non_client_disconnect only effects ChunkWriteTimeout vs ChunkReadTimeout/Other-Server-Error-Exception?","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f2fe782853aaeca78f7fa49da9786094620ed27c","unresolved":true,"context_lines":[{"line_number":1394,"context_line":"        except ChunkReadTimeout:"},{"line_number":1395,"context_line":"            self.app.exception_occurred(self.source.node, \u0027Object\u0027,"},{"line_number":1396,"context_line":"                                        \u0027Trying to read during GET\u0027)"},{"line_number":1397,"context_line":"            raise"},{"line_number":1398,"context_line":"        except ChunkWriteTimeout:"},{"line_number":1399,"context_line":"            self.logger.info("},{"line_number":1400,"context_line":"                \u0027Client did not read from proxy within %ss\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"62792a72_f33408c9","line":1397,"updated":"2023-10-02 17:50:57.000000000","message":"hopefully we retry/recover when we get a ReadTimeout from the backend","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"268a8f5de699dc7cf726cacc40d68d971194cb5e","unresolved":true,"context_lines":[{"line_number":1394,"context_line":"        except ChunkReadTimeout:"},{"line_number":1395,"context_line":"            self.app.exception_occurred(self.source.node, \u0027Object\u0027,"},{"line_number":1396,"context_line":"                                        \u0027Trying to read during GET\u0027)"},{"line_number":1397,"context_line":"            raise"},{"line_number":1398,"context_line":"        except ChunkWriteTimeout:"},{"line_number":1399,"context_line":"            self.logger.info("},{"line_number":1400,"context_line":"                \u0027Client did not read from proxy within %ss\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"d1bce23f_9a17d286","line":1397,"in_reply_to":"62792a72_f33408c9","updated":"2023-10-03 15:23:05.000000000","message":"retry happens in _iter_bytes_from_response_part - IIUC we get here when retrying has failed\n\nHowever, I\u0027m not totally sure we ever do get here - maybe because we switch the ChunkReadTimeout to a StopIteration ! I can\u0027t find a replicated GET test that asserts this log line.","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c75f9d6656a0b25ce7239f490ac3206df624707d","unresolved":true,"context_lines":[{"line_number":1394,"context_line":"        except ChunkReadTimeout:"},{"line_number":1395,"context_line":"            self.app.exception_occurred(self.source.node, \u0027Object\u0027,"},{"line_number":1396,"context_line":"                                        \u0027Trying to read during GET\u0027)"},{"line_number":1397,"context_line":"            raise"},{"line_number":1398,"context_line":"        except ChunkWriteTimeout:"},{"line_number":1399,"context_line":"            self.logger.info("},{"line_number":1400,"context_line":"                \u0027Client did not read from proxy within %ss\u0027,"}],"source_content_type":"text/x-python","patch_set":8,"id":"6930790e_4b6b0ddf","line":1397,"in_reply_to":"d1bce23f_9a17d286","updated":"2023-10-03 20:26:08.000000000","message":"oic, yes - looking at ChunkWriteTimeout and GeneratorExit it might seem reasonable to squelch the ChunkReadTimeout here as well","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f2fe782853aaeca78f7fa49da9786094620ed27c","unresolved":true,"context_lines":[{"line_number":1399,"context_line":"            self.logger.info("},{"line_number":1400,"context_line":"                \u0027Client did not read from proxy within %ss\u0027,"},{"line_number":1401,"context_line":"                self.app.client_timeout)"},{"line_number":1402,"context_line":"            self.logger.increment(\u0027client_timeouts\u0027)"},{"line_number":1403,"context_line":"        except GeneratorExit:"},{"line_number":1404,"context_line":"            warn \u003d True"},{"line_number":1405,"context_line":"            req_range \u003d self.backend_headers[\u0027Range\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"ef921d12_a0aec969","line":1402,"updated":"2023-10-02 17:50:57.000000000","message":"ChunkWriteTimeout on a GET is a client timeout","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c4f0c5cd079cb43837d6d724b6d648efa1d555a5","unresolved":false,"context_lines":[{"line_number":1399,"context_line":"            self.logger.info("},{"line_number":1400,"context_line":"                \u0027Client did not read from proxy within %ss\u0027,"},{"line_number":1401,"context_line":"                self.app.client_timeout)"},{"line_number":1402,"context_line":"            self.logger.increment(\u0027client_timeouts\u0027)"},{"line_number":1403,"context_line":"        except GeneratorExit:"},{"line_number":1404,"context_line":"            warn \u003d True"},{"line_number":1405,"context_line":"            req_range \u003d self.backend_headers[\u0027Range\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"3e072152_5f7dc614","line":1402,"in_reply_to":"ef921d12_a0aec969","updated":"2023-10-06 16:57:42.000000000","message":"Ack","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c75f9d6656a0b25ce7239f490ac3206df624707d","unresolved":true,"context_lines":[{"line_number":1416,"context_line":"            raise"},{"line_number":1417,"context_line":"        except Exception:"},{"line_number":1418,"context_line":"            self.logger.exception(\u0027Trying to send to client\u0027)"},{"line_number":1419,"context_line":"            raise"},{"line_number":1420,"context_line":"        finally:"},{"line_number":1421,"context_line":"            self.source.close()"},{"line_number":1422,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"c3b2974e_adc4c95c","line":1419,"updated":"2023-10-03 20:26:08.000000000","message":"I wonder what\u0027s the value of re-raising here?","commit_id":"cb8a0b497b330fa68ec1d9859813acb00ba75753"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c4f0c5cd079cb43837d6d724b6d648efa1d555a5","unresolved":true,"context_lines":[{"line_number":1162,"context_line":"                        self.source.parts_iter)"},{"line_number":1163,"context_line":"                return (start_byte, end_byte, length, headers, part)"},{"line_number":1164,"context_line":"            except ChunkReadTimeout:"},{"line_number":1165,"context_line":"                self.req.environ[\u0027swift.non_client_disconnect\u0027] \u003d True"},{"line_number":1166,"context_line":"                if not self._replace_source("},{"line_number":1167,"context_line":"                        \u0027Trying to read next part of %s multi-part GET \u0027"},{"line_number":1168,"context_line":"                        \u0027(retrying)\u0027 % self.resource_type):"}],"source_content_type":"text/x-python","patch_set":11,"id":"036936cc_65d4d6a7","line":1165,"updated":"2023-10-06 16:57:42.000000000","message":"IIRC this is not actually required...maybe because we don\u0027t get a GeneratorExit in _iter_parts_from_response this case? IDK if it might be prudent to set it anyway :shrug:","commit_id":"b9c9c7f35d88dd13d9520b542df728c4385d66bd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"86411821381385aef077d8b04f7bd7d48e470f3e","unresolved":false,"context_lines":[{"line_number":1162,"context_line":"                        self.source.parts_iter)"},{"line_number":1163,"context_line":"                return (start_byte, end_byte, length, headers, part)"},{"line_number":1164,"context_line":"            except ChunkReadTimeout:"},{"line_number":1165,"context_line":"                self.req.environ[\u0027swift.non_client_disconnect\u0027] \u003d True"},{"line_number":1166,"context_line":"                if not self._replace_source("},{"line_number":1167,"context_line":"                        \u0027Trying to read next part of %s multi-part GET \u0027"},{"line_number":1168,"context_line":"                        \u0027(retrying)\u0027 % self.resource_type):"}],"source_content_type":"text/x-python","patch_set":11,"id":"f5f949d8_5977f5be","line":1165,"in_reply_to":"036936cc_65d4d6a7","updated":"2024-02-07 11:12:42.000000000","message":"Done","commit_id":"b9c9c7f35d88dd13d9520b542df728c4385d66bd"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"ac405202f6e85b214e5f610d4ccfe0a8c4e399d8","unresolved":false,"context_lines":[{"line_number":1185,"context_line":"        self.policy \u003d policy"},{"line_number":1186,"context_line":"        self.path \u003d path"},{"line_number":1187,"context_line":"        self.backend_headers \u003d backend_headers"},{"line_number":1188,"context_line":"        # resource type is used in logs and isn\u0027t necessarily the server type"},{"line_number":1189,"context_line":"        self.resource_type \u003d resource_type"},{"line_number":1190,"context_line":"        self.node_timeout \u003d node_timeout"},{"line_number":1191,"context_line":"        self.logger \u003d logger or app.logger"}],"source_content_type":"text/x-python","patch_set":22,"id":"3af9cc1d_a9cdce50","line":1188,"updated":"2024-02-26 03:14:46.000000000","message":"Great use of a comment here. Because I was wondering!","commit_id":"c06296250cf3faca9037e1f4e4ca8c58353016f8"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"1fb235e88afed2db41d029b037a5cc2b6dad1e5d","unresolved":true,"context_lines":[{"line_number":1175,"context_line":""},{"line_number":1176,"context_line":""},{"line_number":1177,"context_line":"class GetterBase(object):"},{"line_number":1178,"context_line":"    def __init__(self, app, req, node_iter, partition, policy,"},{"line_number":1179,"context_line":"                 path, backend_headers, node_timeout, resource_type,"},{"line_number":1180,"context_line":"                 logger\u003dNone):"},{"line_number":1181,"context_line":"        self.app \u003d app"}],"source_content_type":"text/x-python","patch_set":23,"id":"58b9306f_08e59457","line":1178,"updated":"2024-02-22 02:30:24.000000000","message":"this class doesn\u0027t have docstring yet, maybe it\u0027s good time to add it now?","commit_id":"85d7aadbbfc3c64ee4e3dde410e99a87ecace195"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"1a29a3eaf077aff5b7c73ce9a5e22bcea1d595f5","unresolved":false,"context_lines":[{"line_number":1175,"context_line":""},{"line_number":1176,"context_line":""},{"line_number":1177,"context_line":"class GetterBase(object):"},{"line_number":1178,"context_line":"    def __init__(self, app, req, node_iter, partition, policy,"},{"line_number":1179,"context_line":"                 path, backend_headers, node_timeout, resource_type,"},{"line_number":1180,"context_line":"                 logger\u003dNone):"},{"line_number":1181,"context_line":"        self.app \u003d app"}],"source_content_type":"text/x-python","patch_set":23,"id":"efca3cf4_742d226f","line":1178,"in_reply_to":"58b9306f_08e59457","updated":"2024-02-23 01:32:17.000000000","message":"Done","commit_id":"85d7aadbbfc3c64ee4e3dde410e99a87ecace195"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"1fb235e88afed2db41d029b037a5cc2b6dad1e5d","unresolved":false,"context_lines":[{"line_number":2286,"context_line":"                                   partition, path, backend_headers,"},{"line_number":2287,"context_line":"                                   concurrency, policy\u003dpolicy,"},{"line_number":2288,"context_line":"                                   logger\u003dself.logger)"},{"line_number":2289,"context_line":"        res \u003d handler.get_working_response()"},{"line_number":2290,"context_line":""},{"line_number":2291,"context_line":"        if not res:"},{"line_number":2292,"context_line":"            res \u003d self.best_response("}],"source_content_type":"text/x-python","patch_set":23,"id":"a94fcce4_78633cc7","line":2289,"updated":"2024-02-22 02:30:24.000000000","message":"this is a good improvement on interface. we passed ``req`` into GetOrHeadHandler constructor, but hen later passed same ``req`` into ``get_working_response`` again. that looked stupid.","commit_id":"85d7aadbbfc3c64ee4e3dde410e99a87ecace195"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"ac405202f6e85b214e5f610d4ccfe0a8c4e399d8","unresolved":true,"context_lines":[{"line_number":1600,"context_line":""},{"line_number":1601,"context_line":"        node_timeout \u003d self.app.node_timeout"},{"line_number":1602,"context_line":"        if self.server_type \u003d\u003d \u0027Object\u0027 and not self.newest:"},{"line_number":1603,"context_line":"            node_timeout \u003d self.app.recoverable_node_timeout"},{"line_number":1604,"context_line":""},{"line_number":1605,"context_line":"        pile \u003d GreenAsyncPile(self.concurrency)"},{"line_number":1606,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"5661e4aa_70004034","side":"PARENT","line":1603,"updated":"2024-02-26 03:14:46.000000000","message":"I guess the point of having this defined outside of _make_node_request was so this conditional (Which would be the same for all nodes) would only have to be made once and not for node number of times.\n\nSo not sure why this had to be moved?","commit_id":"50336c509877e7c325823498cd67bf6cf4f4bef3"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"ac405202f6e85b214e5f610d4ccfe0a8c4e399d8","unresolved":true,"context_lines":[{"line_number":1403,"context_line":"        self.rebalance_missing_suppression_count \u003d min("},{"line_number":1404,"context_line":"            policy_options.rebalance_missing_suppression_count,"},{"line_number":1405,"context_line":"            node_iter.num_primary_nodes - 1)"},{"line_number":1406,"context_line":"        self.newest \u003d config_true_value(req.headers.get(\u0027x-newest\u0027, \u0027f\u0027))"},{"line_number":1407,"context_line":""},{"line_number":1408,"context_line":"        # populated when finding source"},{"line_number":1409,"context_line":"        self.statuses \u003d []"}],"source_content_type":"text/x-python","patch_set":24,"id":"5faa5c56_ae8f2602","line":1406,"updated":"2024-02-26 03:14:46.000000000","message":"Oh, so we haven\u0027t lost the x-newest, it\u0027s just passed it via the request itself (as it already was).","commit_id":"ffbb155effa19c672f26892d32d7f5db10221bb7"}],"swift/proxy/controllers/obj.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0fa448242cae1d2b2b16df6f2170c03c6d4e4936","unresolved":true,"context_lines":[{"line_number":2518,"context_line":"                if not self._replace_source("},{"line_number":2519,"context_line":"                        \u0027Trying to read next part of EC multi-part GET \u0027"},{"line_number":2520,"context_line":"                        \u0027(retrying)\u0027):"},{"line_number":2521,"context_line":"                    raise"},{"line_number":2522,"context_line":""},{"line_number":2523,"context_line":"    def _iter_bytes_from_response_part(self, part_file, nbytes):"},{"line_number":2524,"context_line":"        buf \u003d b\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"a31373a1_74aac466","side":"PARENT","line":2521,"updated":"2023-08-09 17:24:06.000000000","message":"this used to also `raise StopIteration` like the replicated policy case until the RelatedChange","commit_id":"793b05e5e7543c2af2a938147fc5950fbf52755d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"268a8f5de699dc7cf726cacc40d68d971194cb5e","unresolved":true,"context_lines":[{"line_number":2518,"context_line":"                if not self._replace_source("},{"line_number":2519,"context_line":"                        \u0027Trying to read next part of EC multi-part GET \u0027"},{"line_number":2520,"context_line":"                        \u0027(retrying)\u0027):"},{"line_number":2521,"context_line":"                    raise"},{"line_number":2522,"context_line":""},{"line_number":2523,"context_line":"    def _iter_bytes_from_response_part(self, part_file, nbytes):"},{"line_number":2524,"context_line":"        buf \u003d b\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"113ec8ba_c4fc19d4","side":"PARENT","line":2521,"in_reply_to":"0c5ea0dc_2578dc88","updated":"2023-10-03 15:23:05.000000000","message":"There\u0027s nothing in my comment that is doubting the related change.\n\nwhat was the bug? I infer by reverting the related change that it was client getting a 206 rather than a 500 in some scenarios?","commit_id":"793b05e5e7543c2af2a938147fc5950fbf52755d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c75f9d6656a0b25ce7239f490ac3206df624707d","unresolved":true,"context_lines":[{"line_number":2518,"context_line":"                if not self._replace_source("},{"line_number":2519,"context_line":"                        \u0027Trying to read next part of EC multi-part GET \u0027"},{"line_number":2520,"context_line":"                        \u0027(retrying)\u0027):"},{"line_number":2521,"context_line":"                    raise"},{"line_number":2522,"context_line":""},{"line_number":2523,"context_line":"    def _iter_bytes_from_response_part(self, part_file, nbytes):"},{"line_number":2524,"context_line":"        buf \u003d b\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"33c77ab9_1d764870","side":"PARENT","line":2521,"in_reply_to":"113ec8ba_c4fc19d4","updated":"2023-10-03 20:26:08.000000000","message":"I wasn\u0027t bothered, i think we\u0027re both just trying to understand the current state better with some additional context from history - the referenced change could have done a better job explaining the bug/beahvior that it fixed (on accident?)\n\nI think the assumption was that it wasn\u0027t really a significant client impacting behavior change - it was just some weird cruft that sort of accidently got simplified while trying to tease out whatever that weird code was actually trying to do.","commit_id":"793b05e5e7543c2af2a938147fc5950fbf52755d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"86411821381385aef077d8b04f7bd7d48e470f3e","unresolved":false,"context_lines":[{"line_number":2518,"context_line":"                if not self._replace_source("},{"line_number":2519,"context_line":"                        \u0027Trying to read next part of EC multi-part GET \u0027"},{"line_number":2520,"context_line":"                        \u0027(retrying)\u0027):"},{"line_number":2521,"context_line":"                    raise"},{"line_number":2522,"context_line":""},{"line_number":2523,"context_line":"    def _iter_bytes_from_response_part(self, part_file, nbytes):"},{"line_number":2524,"context_line":"        buf \u003d b\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"6ee10936_93868331","side":"PARENT","line":2521,"in_reply_to":"33c77ab9_1d764870","updated":"2024-02-07 11:12:42.000000000","message":"Done","commit_id":"793b05e5e7543c2af2a938147fc5950fbf52755d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f2fe782853aaeca78f7fa49da9786094620ed27c","unresolved":true,"context_lines":[{"line_number":2518,"context_line":"                if not self._replace_source("},{"line_number":2519,"context_line":"                        \u0027Trying to read next part of EC multi-part GET \u0027"},{"line_number":2520,"context_line":"                        \u0027(retrying)\u0027):"},{"line_number":2521,"context_line":"                    raise"},{"line_number":2522,"context_line":""},{"line_number":2523,"context_line":"    def _iter_bytes_from_response_part(self, part_file, nbytes):"},{"line_number":2524,"context_line":"        buf \u003d b\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"0c5ea0dc_2578dc88","side":"PARENT","line":2521,"in_reply_to":"a31373a1_74aac466","updated":"2023-10-02 17:50:57.000000000","message":"you mean here?\n\nhttps://review.opendev.org/c/openstack/swift/+/761475/comment/f1edcb89_c0f3f8a7/\n\n... ok, but that was a BUG; also a coulple of refactors ago.  It only makes sense to me that ChunkReadTimeout should be an errors - we can\u0027t translate a backend ReadTimeout to a StopIteration if StopIteration is part of the interface to single \"there is no multi-range docs\"","commit_id":"793b05e5e7543c2af2a938147fc5950fbf52755d"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0fa448242cae1d2b2b16df6f2170c03c6d4e4936","unresolved":true,"context_lines":[{"line_number":2649,"context_line":"                    query_string\u003dself.req.query_string)"},{"line_number":2650,"context_line":"            self.app.set_node_timing(node, time.time() - start_node_timing)"},{"line_number":2651,"context_line":""},{"line_number":2652,"context_line":"            with Timeout(self.node_timeout):"},{"line_number":2653,"context_line":"                possible_source \u003d conn.getresponse()"},{"line_number":2654,"context_line":"                # See NOTE: swift_conn at top of file about this."},{"line_number":2655,"context_line":"                possible_source.swift_conn \u003d conn"}],"source_content_type":"text/x-python","patch_set":1,"id":"d0be3da0_6da03891","line":2652,"updated":"2023-08-09 17:24:06.000000000","message":"newest isn\u0027t respected for EC, so we can use self.node_timeout","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c89c5a6c43383498cfa110959b71cf91d997caae","unresolved":false,"context_lines":[{"line_number":2649,"context_line":"                    query_string\u003dself.req.query_string)"},{"line_number":2650,"context_line":"            self.app.set_node_timing(node, time.time() - start_node_timing)"},{"line_number":2651,"context_line":""},{"line_number":2652,"context_line":"            with Timeout(self.node_timeout):"},{"line_number":2653,"context_line":"                possible_source \u003d conn.getresponse()"},{"line_number":2654,"context_line":"                # See NOTE: swift_conn at top of file about this."},{"line_number":2655,"context_line":"                possible_source.swift_conn \u003d conn"}],"source_content_type":"text/x-python","patch_set":1,"id":"8d53c2f7_bb795b89","line":2652,"in_reply_to":"d0be3da0_6da03891","updated":"2024-02-21 21:42:36.000000000","message":"Acknowledged","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f2fe782853aaeca78f7fa49da9786094620ed27c","unresolved":true,"context_lines":[{"line_number":2487,"context_line":"            app\u003dapp, req\u003dreq, node_iter\u003dnode_iter,"},{"line_number":2488,"context_line":"            partition\u003dpartition, policy\u003dpolicy, path\u003dpath,"},{"line_number":2489,"context_line":"            backend_headers\u003dbackend_headers,"},{"line_number":2490,"context_line":"            node_timeout\u003dapp.recoverable_node_timeout,"},{"line_number":2491,"context_line":"            resource_type\u003d\u0027EC fragment\u0027,"},{"line_number":2492,"context_line":"            logger\u003dlogger)"},{"line_number":2493,"context_line":"        self.header_provider \u003d header_provider"}],"source_content_type":"text/x-python","patch_set":8,"id":"631da73e_8f49696f","line":2490,"updated":"2023-10-02 17:50:57.000000000","message":"I don\u0027t really understand why this is param to `__init__` if we\u0027re already passing in an app - and also the base can change it in some weird x-newest scenario","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c4f0c5cd079cb43837d6d724b6d648efa1d555a5","unresolved":true,"context_lines":[{"line_number":2487,"context_line":"            app\u003dapp, req\u003dreq, node_iter\u003dnode_iter,"},{"line_number":2488,"context_line":"            partition\u003dpartition, policy\u003dpolicy, path\u003dpath,"},{"line_number":2489,"context_line":"            backend_headers\u003dbackend_headers,"},{"line_number":2490,"context_line":"            node_timeout\u003dapp.recoverable_node_timeout,"},{"line_number":2491,"context_line":"            resource_type\u003d\u0027EC fragment\u0027,"},{"line_number":2492,"context_line":"            logger\u003dlogger)"},{"line_number":2493,"context_line":"        self.header_provider \u003d header_provider"}],"source_content_type":"text/x-python","patch_set":8,"id":"d2de63dc_63a9227b","line":2490,"in_reply_to":"631da73e_8f49696f","updated":"2023-10-06 16:57:42.000000000","message":"still on my radar to fix this","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"86411821381385aef077d8b04f7bd7d48e470f3e","unresolved":true,"context_lines":[{"line_number":2487,"context_line":"            app\u003dapp, req\u003dreq, node_iter\u003dnode_iter,"},{"line_number":2488,"context_line":"            partition\u003dpartition, policy\u003dpolicy, path\u003dpath,"},{"line_number":2489,"context_line":"            backend_headers\u003dbackend_headers,"},{"line_number":2490,"context_line":"            node_timeout\u003dapp.recoverable_node_timeout,"},{"line_number":2491,"context_line":"            resource_type\u003d\u0027EC fragment\u0027,"},{"line_number":2492,"context_line":"            logger\u003dlogger)"},{"line_number":2493,"context_line":"        self.header_provider \u003d header_provider"}],"source_content_type":"text/x-python","patch_set":8,"id":"a7daf133_fc17ebb7","line":2490,"in_reply_to":"d2de63dc_63a9227b","updated":"2024-02-07 11:12:42.000000000","message":"The subclass needs to select which timeout the superclass should have.\n\nThe superclass does not deal with x-newest, that is specific to the GetOrHeadHandler subclass.","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f2fe782853aaeca78f7fa49da9786094620ed27c","unresolved":true,"context_lines":[{"line_number":2488,"context_line":"            partition\u003dpartition, policy\u003dpolicy, path\u003dpath,"},{"line_number":2489,"context_line":"            backend_headers\u003dbackend_headers,"},{"line_number":2490,"context_line":"            node_timeout\u003dapp.recoverable_node_timeout,"},{"line_number":2491,"context_line":"            resource_type\u003d\u0027EC fragment\u0027,"},{"line_number":2492,"context_line":"            logger\u003dlogger)"},{"line_number":2493,"context_line":"        self.header_provider \u003d header_provider"},{"line_number":2494,"context_line":"        self.fragment_size \u003d policy.fragment_size"}],"source_content_type":"text/x-python","patch_set":8,"id":"6d436479_b4b124a5","line":2491,"updated":"2023-10-02 17:50:57.000000000","message":"AFAIK this is totally new, and we used to just let the Base say EC requests were object requests\n\n... but, I think it\u0027s actually a good thing to differentiate.","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c75f9d6656a0b25ce7239f490ac3206df624707d","unresolved":true,"context_lines":[{"line_number":2488,"context_line":"            partition\u003dpartition, policy\u003dpolicy, path\u003dpath,"},{"line_number":2489,"context_line":"            backend_headers\u003dbackend_headers,"},{"line_number":2490,"context_line":"            node_timeout\u003dapp.recoverable_node_timeout,"},{"line_number":2491,"context_line":"            resource_type\u003d\u0027EC fragment\u0027,"},{"line_number":2492,"context_line":"            logger\u003dlogger)"},{"line_number":2493,"context_line":"        self.header_provider \u003d header_provider"},{"line_number":2494,"context_line":"        self.fragment_size \u003d policy.fragment_size"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f4bf7af_5b9cfd99","line":2491,"in_reply_to":"4ded25e7_4d238b1b","updated":"2023-10-03 20:26:08.000000000","message":"maybe the OOP pattern would be to have a get_node_timeout method - with the default/base class just returning self.app.node_timeout (or I guess recoverable_node_timeout is actually the most common case?), but then replicated class overrides get_node_timeout to inspects the request headers and returns either app.node_timeout or app.recoverable_timeout\n\nhaving callers pass in a node_timeout (that\u0027s really recoverable_node_timeout) and then sometimes not use it and instead steal the value from app is twisting my head.","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"268a8f5de699dc7cf726cacc40d68d971194cb5e","unresolved":true,"context_lines":[{"line_number":2488,"context_line":"            partition\u003dpartition, policy\u003dpolicy, path\u003dpath,"},{"line_number":2489,"context_line":"            backend_headers\u003dbackend_headers,"},{"line_number":2490,"context_line":"            node_timeout\u003dapp.recoverable_node_timeout,"},{"line_number":2491,"context_line":"            resource_type\u003d\u0027EC fragment\u0027,"},{"line_number":2492,"context_line":"            logger\u003dlogger)"},{"line_number":2493,"context_line":"        self.header_provider \u003d header_provider"},{"line_number":2494,"context_line":"        self.fragment_size \u003d policy.fragment_size"}],"source_content_type":"text/x-python","patch_set":8,"id":"4ded25e7_4d238b1b","line":2491,"in_reply_to":"6d436479_b4b124a5","updated":"2023-10-03 15:23:05.000000000","message":"yes, I called this out in the commit message as the one behavioural change.","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f2fe782853aaeca78f7fa49da9786094620ed27c","unresolved":true,"context_lines":[{"line_number":2633,"context_line":"        else:"},{"line_number":2634,"context_line":"            return HeaderKeyDict()"},{"line_number":2635,"context_line":""},{"line_number":2636,"context_line":"    def _make_node_request(self, node):"},{"line_number":2637,"context_line":"        # make a backend request; return a response if it has an acceptable"},{"line_number":2638,"context_line":"        # status code, otherwise None"},{"line_number":2639,"context_line":"        self.logger.thread_locals \u003d self.logger_thread_locals"}],"source_content_type":"text/x-python","patch_set":8,"id":"0c33080d_e6d913fd","line":2636,"updated":"2023-10-02 17:50:57.000000000","message":"i like this signature simplification","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"268a8f5de699dc7cf726cacc40d68d971194cb5e","unresolved":false,"context_lines":[{"line_number":2633,"context_line":"        else:"},{"line_number":2634,"context_line":"            return HeaderKeyDict()"},{"line_number":2635,"context_line":""},{"line_number":2636,"context_line":"    def _make_node_request(self, node):"},{"line_number":2637,"context_line":"        # make a backend request; return a response if it has an acceptable"},{"line_number":2638,"context_line":"        # status code, otherwise None"},{"line_number":2639,"context_line":"        self.logger.thread_locals \u003d self.logger_thread_locals"}],"source_content_type":"text/x-python","patch_set":8,"id":"e56f89bd_da860666","line":2636,"in_reply_to":"0c33080d_e6d913fd","updated":"2023-10-03 15:23:05.000000000","message":"Ack","commit_id":"149b927450847ec736604e50d2ec2a42816b3caa"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"1fb235e88afed2db41d029b037a5cc2b6dad1e5d","unresolved":true,"context_lines":[{"line_number":2717,"context_line":"        \"\"\""},{"line_number":2718,"context_line":"        Create an iterator over a single fragment response body."},{"line_number":2719,"context_line":""},{"line_number":2720,"context_line":"        :param req: a ``swob.Request``."},{"line_number":2721,"context_line":"        :return: an interator that yields chunks of bytes from a fragment"},{"line_number":2722,"context_line":"            response body."},{"line_number":2723,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":23,"id":"8b22d06a_d3ad4248","line":2720,"updated":"2024-02-22 02:30:24.000000000","message":"remove this comment line","commit_id":"85d7aadbbfc3c64ee4e3dde410e99a87ecace195"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"1a29a3eaf077aff5b7c73ce9a5e22bcea1d595f5","unresolved":false,"context_lines":[{"line_number":2717,"context_line":"        \"\"\""},{"line_number":2718,"context_line":"        Create an iterator over a single fragment response body."},{"line_number":2719,"context_line":""},{"line_number":2720,"context_line":"        :param req: a ``swob.Request``."},{"line_number":2721,"context_line":"        :return: an interator that yields chunks of bytes from a fragment"},{"line_number":2722,"context_line":"            response body."},{"line_number":2723,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":23,"id":"1d4bc4e3_2c98f92f","line":2720,"in_reply_to":"8b22d06a_d3ad4248","updated":"2024-02-23 01:32:17.000000000","message":"Done","commit_id":"85d7aadbbfc3c64ee4e3dde410e99a87ecace195"}],"test/unit/__init__.py":[{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"38d87ecfe56176e1ebe65e16862e89c26b9a6307","unresolved":true,"context_lines":[{"line_number":1116,"context_line":"        self.bytes_read \u003d 0"},{"line_number":1117,"context_line":""},{"line_number":1118,"context_line":"    def slowdown(self):"},{"line_number":1119,"context_line":"        if self.bytes_read \u003c self.slowdown_after:"},{"line_number":1120,"context_line":"            return"},{"line_number":1121,"context_line":"        try:"},{"line_number":1122,"context_line":"            wait \u003d next(self._slowdown)"}],"source_content_type":"text/x-python","patch_set":3,"id":"54688d12_3297e222","line":1119,"updated":"2023-09-19 00:34:13.000000000","message":"I don\u0027t understand why \"slowdown_after\" is introduced into this refactoring patch? to me, it seems like a behavioral change.","commit_id":"d068a553d6a3b42bb0bfff3b4a78c3d01b49eb73"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7f3287aef40ac69bf788d09ea9e0c50c5af19aaf","unresolved":true,"context_lines":[{"line_number":1116,"context_line":"        self.bytes_read \u003d 0"},{"line_number":1117,"context_line":""},{"line_number":1118,"context_line":"    def slowdown(self):"},{"line_number":1119,"context_line":"        if self.bytes_read \u003c self.slowdown_after:"},{"line_number":1120,"context_line":"            return"},{"line_number":1121,"context_line":"        try:"},{"line_number":1122,"context_line":"            wait \u003d next(self._slowdown)"}],"source_content_type":"text/x-python","patch_set":3,"id":"e155f18e_26d976df","line":1119,"in_reply_to":"54688d12_3297e222","updated":"2023-09-19 09:31:20.000000000","message":"I added some test coverage that required this, but existing tests aren\u0027t changed. Maybe I should split those into a different patch.","commit_id":"d068a553d6a3b42bb0bfff3b4a78c3d01b49eb73"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"acda13b18d4834bbae2b68b50bdb36575d3220c5","unresolved":false,"context_lines":[{"line_number":1116,"context_line":"        self.bytes_read \u003d 0"},{"line_number":1117,"context_line":""},{"line_number":1118,"context_line":"    def slowdown(self):"},{"line_number":1119,"context_line":"        if self.bytes_read \u003c self.slowdown_after:"},{"line_number":1120,"context_line":"            return"},{"line_number":1121,"context_line":"        try:"},{"line_number":1122,"context_line":"            wait \u003d next(self._slowdown)"}],"source_content_type":"text/x-python","patch_set":3,"id":"395b8489_7ac74efe","line":1119,"in_reply_to":"e155f18e_26d976df","updated":"2023-09-19 10:52:24.000000000","message":"I moved the new tests to a separate patch https://review.opendev.org/c/openstack/swift/+/895802","commit_id":"d068a553d6a3b42bb0bfff3b4a78c3d01b49eb73"}],"test/unit/proxy/controllers/test_base.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"c89c5a6c43383498cfa110959b71cf91d997caae","unresolved":true,"context_lines":[{"line_number":1684,"context_line":"        # app.recoverable_node_timeout"},{"line_number":1685,"context_line":"        getter \u003d GetOrHeadHandler("},{"line_number":1686,"context_line":"            app, req, \u0027Object\u0027, node_iter, None, None, {})"},{"line_number":1687,"context_line":"        self.assertEqual(3, getter.node_timeout)"},{"line_number":1688,"context_line":""},{"line_number":1689,"context_line":"        # x-newest not set"},{"line_number":1690,"context_line":"        req \u003d Request.blank(\u0027/v1/a/c/o\u0027)"}],"source_content_type":"text/x-python","patch_set":22,"id":"2320d0d1_de27813d","line":1687,"updated":"2024-02-21 21:42:36.000000000","message":"this changes in https://review.opendev.org/c/openstack/swift/+/908287?usp\u003demail","commit_id":"c06296250cf3faca9037e1f4e4ca8c58353016f8"}],"test/unit/proxy/controllers/test_obj.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0fa448242cae1d2b2b16df6f2170c03c6d4e4936","unresolved":true,"context_lines":[{"line_number":1720,"context_line":"        self.assertEqual(resp.status_int, 206)"},{"line_number":1721,"context_line":"        self.assertEqual(6, len(log))"},{"line_number":1722,"context_line":"        resp_boundary \u003d resp.headers[\u0027content-type\u0027].rsplit(\u0027\u003d\u0027, 1)[1].encode()"},{"line_number":1723,"context_line":"        self.assertEqual(b\u0027--%s--\u0027 % resp_boundary, actual_body)"},{"line_number":1724,"context_line":"        error_lines \u003d self.app.logger.get_lines_for_level(\u0027error\u0027)"},{"line_number":1725,"context_line":"        self.assertEqual(3, len(error_lines))"},{"line_number":1726,"context_line":"        for line in error_lines:"}],"source_content_type":"text/x-python","patch_set":1,"id":"a71cd090_3396a493","line":1723,"updated":"2023-08-09 17:24:06.000000000","message":"replicated policy - we\u0027ve fed the client a line of body (based on a backend content-type header?) before the iterator exits. I\u0027m not sure is that is consistent with a comment that says the _get_next_response_part WILL do IO for a multipart response :shrug:","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"acda13b18d4834bbae2b68b50bdb36575d3220c5","unresolved":false,"context_lines":[{"line_number":1720,"context_line":"        self.assertEqual(resp.status_int, 206)"},{"line_number":1721,"context_line":"        self.assertEqual(6, len(log))"},{"line_number":1722,"context_line":"        resp_boundary \u003d resp.headers[\u0027content-type\u0027].rsplit(\u0027\u003d\u0027, 1)[1].encode()"},{"line_number":1723,"context_line":"        self.assertEqual(b\u0027--%s--\u0027 % resp_boundary, actual_body)"},{"line_number":1724,"context_line":"        error_lines \u003d self.app.logger.get_lines_for_level(\u0027error\u0027)"},{"line_number":1725,"context_line":"        self.assertEqual(3, len(error_lines))"},{"line_number":1726,"context_line":"        for line in error_lines:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9c24de60_9049b181","line":1723,"in_reply_to":"a71cd090_3396a493","updated":"2023-09-19 10:52:24.000000000","message":"Ack","commit_id":"9dfbe034100e048e72bda977167cf655e1f281c4"},{"author":{"_account_id":34930,"name":"Jianjian Huo","email":"jhuo@nvidia.com","username":"jhuo"},"change_message_id":"1fb235e88afed2db41d029b037a5cc2b6dad1e5d","unresolved":false,"context_lines":[{"line_number":4996,"context_line":"        self.assertEqual(len(log), self.policy.ec_n_unique_fragments * 2)"},{"line_number":4997,"context_line":"        log_lines \u003d self.app.logger.get_lines_for_level(\u0027error\u0027)"},{"line_number":4998,"context_line":"        self.assertEqual(2, len(log_lines), log_lines)"},{"line_number":4999,"context_line":"        self.assertIn(\u0027Trying to read next part of EC fragment multi-part GET\u0027,"},{"line_number":5000,"context_line":"                      log_lines[0])"},{"line_number":5001,"context_line":"        self.assertIn(\u0027Trying to read during GET: ChunkReadTimeout\u0027,"},{"line_number":5002,"context_line":"                      log_lines[1])"}],"source_content_type":"text/x-python","patch_set":23,"id":"b09cc49c_b9267cf2","line":4999,"updated":"2024-02-22 02:30:24.000000000","message":"okay, so replicated object and EC will see different messages.","commit_id":"85d7aadbbfc3c64ee4e3dde410e99a87ecace195"}]}