)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"de6418312f71a91e5e121aca41810591cc6ec1a9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8e54d540_8a732321","updated":"2023-06-26 17:25:55.000000000","message":"this is an alternative to https://review.opendev.org/c/openstack/swift/+/883878","commit_id":"ac2efb883b0fbf624aed302229057c2c389c1fbd"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5e1ff3c066cf6b9a5cbb26dbf66325f7445ca560","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"03151733_cd7cbee5","updated":"2023-07-10 19:33:07.000000000","message":"i noticed i had a draft comment","commit_id":"456f539b16053240e3d2a2f874a13738f8a39b4c"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"a2dcf68cae3c37d6e9d00c7da9a78336ba5cae91","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"90ec74e1_36a97416","updated":"2023-07-20 01:27:44.000000000","message":"Really more questions then anything else at this stage. I like the refactor idea though. It does give us some better close /filelike options.","commit_id":"2934c2cfa29116668241ba818d11a0f255cf6bcf"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"577bc14e76f7668f0d511c808d922dc298df5183","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"a91f7232_ef2fa5a4","updated":"2023-07-24 09:06:38.000000000","message":"@Matt ugh I forgot to post my replies to your review, but belated thanks!","commit_id":"4de0d9ea86d4cfeb44f8c9f9ae495dd9a9bb226b"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"ff3f030f45d2b59d50d396901fd770ea6dc237a3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"c16a3ff1_7a0e64ad","updated":"2023-07-24 03:40:19.000000000","message":"Just added a no timestamp header part of the timestamp test.","commit_id":"4de0d9ea86d4cfeb44f8c9f9ae495dd9a9bb226b"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"0810d60e21348cd03bd5ca9234452b64e34bea7d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"120eccd2_da147eda","updated":"2023-07-24 05:13:21.000000000","message":"Really like the encapsulation used here. Makes it easier to read.","commit_id":"4de0d9ea86d4cfeb44f8c9f9ae495dd9a9bb226b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"a191b197_b025834e","updated":"2023-08-03 15:34:27.000000000","message":"Thanks for the reviews!","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"3427996f7a7e408a6736d10eb500c8ec12cf9738","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"89e0937f_363367b6","updated":"2023-07-25 00:45:24.000000000","message":"just rebased it.","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d96024b8d8688bc77b8ae217c4de2600c85cfe8b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"4b6876c6_29e1c58b","updated":"2023-08-02 15:52:24.000000000","message":"recheck\n\nFAILED test/probe/test_replication_servers_working.py::TestReplicatorFunctionsReservedNames::test_main\nseems unrelated - expected files not found after replication, proxy not involved","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1c023e59ba271dccdc190c6e06b019b0ee0d9c3b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"920d39b5_214a0d93","updated":"2023-07-26 12:45:46.000000000","message":"recheck\nTimed out test job","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"842b13556369c9eaf87bd19e5be221791e4583e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"3c705ff7_76b33808","updated":"2023-07-25 09:42:10.000000000","message":"recheck \nhttps://bugs.launchpad.net/swift/+bug/2028175","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"50228b70_f0e17064","updated":"2023-08-02 23:18:45.000000000","message":"test changes look good - this is a solid refactor, no expected behavior change.\n\nCurious to see what Al thinks about using the pattern `Type.function(instance)` to spell `instance.method()`, maybe it\u0027s quite clever or even idiomatic, but I don\u0027t think I\u0027d seen it used in production code before.  It\u0027s one less call stack than the lambda tho so mabye worth my surprise!\n\nPassing GetterSource an app to get object_chunk_size seems heavy handed now, but may prove prescient if want to encapsulate additional behaviors in the Source; not a big hang up for me.\n\nI wonder what it would look like to have a self.source stay closer to an \"is a\" implementation of a Response.  We hang swift_conn off a response; I wonder why we didn\u0027t just hang the node or the _parts_iter gen off the response instance as well? \n I generally prefer \"has a\" tho, so I wouldn\u0027t expect I\u0027d find it any more appealing than self.source.resp.xxx everywhere; and I\u0027m quite fine with this as-is.","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c0bc85310891dc8ee0e0e59da8c8591e363b956e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"3b9045d0_b389f738","updated":"2023-08-04 23:06:46.000000000","message":"Unit test could use a little work, but otherwise looking good! Happy to carry.","commit_id":"24d2976f63191f778bcf351d601876bdf988dbc4"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ae45fff7f682933e4cd23d5686b81cfd69c31a3f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"60c70db7_dd729ef1","updated":"2023-08-07 22:47:44.000000000","message":"recheck\n\nFailure in `test/probe/test_replication_servers_working.py::TestReplicatorFunctionsReservedNames::test_main` -- looks like we somehow replicated when we didn\u0027t mean to? Or something?\n```\n            self.replicators.once()\n            # Check, that files not replicated.\n            for directory in os.listdir(os.path.join(\n                    test_node, data_dir)):\n                for input_dir in os.listdir(os.path.join(\n                        test_node, data_dir, directory)):\n                    self.assertFalse(os.path.isdir(\n                        os.path.join(test_node, data_dir,\n\u003e                                    directory, input_dir)))\nE                                    AssertionError: True is not false\n```","commit_id":"f8a6dee622e47cec7111007069dd4ca74b56b49c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"59318e8108c3429c6ff1557baeaaf5ec59ee937e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"59a068a4_a93214ec","updated":"2023-08-07 14:15:11.000000000","message":"recheck\n\nseems to be an intermittent failure in test/unit/proxy/controllers/test_obj.py::TestECObjController::test_GET_disconnect, I\u0027m trying to reproduce locally, but don\u0027t believe it is related to the patch","commit_id":"f8a6dee622e47cec7111007069dd4ca74b56b49c"}],"swift/proxy/controllers/base.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"2e31a6020637719b18931c0e41dcdafe95806537","unresolved":true,"context_lines":[{"line_number":1544,"context_line":"            update_headers(res, self.source.resp.getheaders())"},{"line_number":1545,"context_line":"            if req.method \u003d\u003d \u0027GET\u0027 and \\"},{"line_number":1546,"context_line":"                    self.source.resp.status in (HTTP_OK, HTTP_PARTIAL_CONTENT):"},{"line_number":1547,"context_line":"                res.app_iter \u003d self._make_app_iter(req)"},{"line_number":1548,"context_line":"                # See NOTE: swift_conn at top of file about this."},{"line_number":1549,"context_line":"                res.swift_conn \u003d self.source.resp.swift_conn"},{"line_number":1550,"context_line":"            if not res.environ:"}],"source_content_type":"text/x-python","patch_set":2,"id":"712b43c2_5e622329","line":1547,"updated":"2023-06-26 17:20:48.000000000","message":"IIUC this will trigger the GetterSource _parts_iter to be constructed","commit_id":"8476a721dbec6c28d9faf7b5c392d00d18200034"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"577bc14e76f7668f0d511c808d922dc298df5183","unresolved":false,"context_lines":[{"line_number":1544,"context_line":"            update_headers(res, self.source.resp.getheaders())"},{"line_number":1545,"context_line":"            if req.method \u003d\u003d \u0027GET\u0027 and \\"},{"line_number":1546,"context_line":"                    self.source.resp.status in (HTTP_OK, HTTP_PARTIAL_CONTENT):"},{"line_number":1547,"context_line":"                res.app_iter \u003d self._make_app_iter(req)"},{"line_number":1548,"context_line":"                # See NOTE: swift_conn at top of file about this."},{"line_number":1549,"context_line":"                res.swift_conn \u003d self.source.resp.swift_conn"},{"line_number":1550,"context_line":"            if not res.environ:"}],"source_content_type":"text/x-python","patch_set":2,"id":"34d3b67f_4ad6ca01","line":1547,"in_reply_to":"712b43c2_5e622329","updated":"2023-07-24 09:06:38.000000000","message":"Ack","commit_id":"8476a721dbec6c28d9faf7b5c392d00d18200034"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"a2dcf68cae3c37d6e9d00c7da9a78336ba5cae91","unresolved":true,"context_lines":[{"line_number":1053,"context_line":""},{"line_number":1054,"context_line":"    def _get_source_and_node(self):"},{"line_number":1055,"context_line":"        \"\"\""},{"line_number":1056,"context_line":"        Look for a suitable new source and if one is found then set"},{"line_number":1057,"context_line":"        ``self.source``."},{"line_number":1058,"context_line":""},{"line_number":1059,"context_line":"        :return: ``True`` if ``self.source`` has been updated, ``False``"},{"line_number":1060,"context_line":"            otherwise."}],"source_content_type":"text/x-python","patch_set":7,"id":"dddb0d3e_8e28fba3","line":1057,"range":{"start_line":1056,"start_character":8,"end_line":1057,"end_character":24},"updated":"2023-07-20 01:27:44.000000000","message":"Yeah, so we now update an instance variable rather then return. Like I mentioned elsewhere, this is ok. But a little unextected from the function name. Maybe we should change it from get to find?\n\nOr maybe I\u0027m over thinking it.","commit_id":"2934c2cfa29116668241ba818d11a0f255cf6bcf"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"577bc14e76f7668f0d511c808d922dc298df5183","unresolved":false,"context_lines":[{"line_number":1053,"context_line":""},{"line_number":1054,"context_line":"    def _get_source_and_node(self):"},{"line_number":1055,"context_line":"        \"\"\""},{"line_number":1056,"context_line":"        Look for a suitable new source and if one is found then set"},{"line_number":1057,"context_line":"        ``self.source``."},{"line_number":1058,"context_line":""},{"line_number":1059,"context_line":"        :return: ``True`` if ``self.source`` has been updated, ``False``"},{"line_number":1060,"context_line":"            otherwise."}],"source_content_type":"text/x-python","patch_set":7,"id":"96198a11_9ac3e310","line":1057,"range":{"start_line":1056,"start_character":8,"end_line":1057,"end_character":24},"in_reply_to":"dddb0d3e_8e28fba3","updated":"2023-07-24 09:06:38.000000000","message":"Done","commit_id":"2934c2cfa29116668241ba818d11a0f255cf6bcf"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"01795e90ce1c728444ac8b3c153245a4a2243754","unresolved":false,"context_lines":[{"line_number":1053,"context_line":""},{"line_number":1054,"context_line":"    def _get_source_and_node(self):"},{"line_number":1055,"context_line":"        \"\"\""},{"line_number":1056,"context_line":"        Look for a suitable new source and if one is found then set"},{"line_number":1057,"context_line":"        ``self.source``."},{"line_number":1058,"context_line":""},{"line_number":1059,"context_line":"        :return: ``True`` if ``self.source`` has been updated, ``False``"},{"line_number":1060,"context_line":"            otherwise."}],"source_content_type":"text/x-python","patch_set":7,"id":"a53b8d0b_f6705e6a","line":1057,"range":{"start_line":1056,"start_character":8,"end_line":1057,"end_character":24},"in_reply_to":"dddb0d3e_8e28fba3","updated":"2023-07-24 03:39:06.000000000","message":"Done","commit_id":"2934c2cfa29116668241ba818d11a0f255cf6bcf"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"a2dcf68cae3c37d6e9d00c7da9a78336ba5cae91","unresolved":true,"context_lines":[{"line_number":1493,"context_line":"        if self.sources:"},{"line_number":1494,"context_line":"            self.sources.sort(key\u003dlambda s: source_key(s[0]))"},{"line_number":1495,"context_line":"            source, node \u003d self.sources.pop()"},{"line_number":1496,"context_line":"            for src, _junk in self.sources:"},{"line_number":1497,"context_line":"                close_swift_conn(src)"},{"line_number":1498,"context_line":"            self.used_nodes.append(node)"},{"line_number":1499,"context_line":"            src_headers \u003d dict("}],"source_content_type":"text/x-python","patch_set":7,"id":"c1c86e4d_49b52b60","line":1496,"range":{"start_line":1496,"start_character":30,"end_line":1496,"end_character":42},"updated":"2023-07-20 01:27:44.000000000","message":"Should our self.sources be a list of GetterSource objects, so we encapsulate them from the onset? We could make them sortable etc. But better we could make use of the close method?","commit_id":"2934c2cfa29116668241ba818d11a0f255cf6bcf"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"577bc14e76f7668f0d511c808d922dc298df5183","unresolved":false,"context_lines":[{"line_number":1493,"context_line":"        if self.sources:"},{"line_number":1494,"context_line":"            self.sources.sort(key\u003dlambda s: source_key(s[0]))"},{"line_number":1495,"context_line":"            source, node \u003d self.sources.pop()"},{"line_number":1496,"context_line":"            for src, _junk in self.sources:"},{"line_number":1497,"context_line":"                close_swift_conn(src)"},{"line_number":1498,"context_line":"            self.used_nodes.append(node)"},{"line_number":1499,"context_line":"            src_headers \u003d dict("}],"source_content_type":"text/x-python","patch_set":7,"id":"b8ab1fab_027cabfa","line":1496,"range":{"start_line":1496,"start_character":30,"end_line":1496,"end_character":42},"in_reply_to":"c1c86e4d_49b52b60","updated":"2023-07-24 09:06:38.000000000","message":"good point! Done","commit_id":"2934c2cfa29116668241ba818d11a0f255cf6bcf"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"a2dcf68cae3c37d6e9d00c7da9a78336ba5cae91","unresolved":true,"context_lines":[{"line_number":1548,"context_line":""},{"line_number":1549,"context_line":"    def get_working_response(self, req):"},{"line_number":1550,"context_line":"        res \u003d None"},{"line_number":1551,"context_line":"        if self._get_source_and_node():"},{"line_number":1552,"context_line":"            res \u003d Response(request\u003dreq)"},{"line_number":1553,"context_line":"            res.status \u003d self.source.resp.status"},{"line_number":1554,"context_line":"            update_headers(res, self.source.resp.getheaders())"}],"source_content_type":"text/x-python","patch_set":7,"id":"e02929dd_03667fa4","line":1551,"range":{"start_line":1551,"start_character":17,"end_line":1551,"end_character":36},"updated":"2023-07-20 01:27:44.000000000","message":"Would this still be called `_get_source_and_node` when it doesn\u0027t actually return them anymore?\n\nI guess it did get them, just put them in some instance variables. Just feel the name is now wrong.\n\nMaybe _find_source_and_node? So we know that the method will go attempt to do something?","commit_id":"2934c2cfa29116668241ba818d11a0f255cf6bcf"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"577bc14e76f7668f0d511c808d922dc298df5183","unresolved":false,"context_lines":[{"line_number":1548,"context_line":""},{"line_number":1549,"context_line":"    def get_working_response(self, req):"},{"line_number":1550,"context_line":"        res \u003d None"},{"line_number":1551,"context_line":"        if self._get_source_and_node():"},{"line_number":1552,"context_line":"            res \u003d Response(request\u003dreq)"},{"line_number":1553,"context_line":"            res.status \u003d self.source.resp.status"},{"line_number":1554,"context_line":"            update_headers(res, self.source.resp.getheaders())"}],"source_content_type":"text/x-python","patch_set":7,"id":"3fe041c1_63d11f25","line":1551,"range":{"start_line":1551,"start_character":17,"end_line":1551,"end_character":36},"in_reply_to":"e02929dd_03667fa4","updated":"2023-07-24 09:06:38.000000000","message":"agree. get implies a getter which returns the thing - will change to _find_source","commit_id":"2934c2cfa29116668241ba818d11a0f255cf6bcf"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":99,"context_line":"    return Timestamp(resp.getheader(\u0027x-backend-data-timestamp\u0027) or"},{"line_number":100,"context_line":"                     resp.getheader(\u0027x-backend-timestamp\u0027) or"},{"line_number":101,"context_line":"                     resp.getheader(\u0027x-put-timestamp\u0027) or"},{"line_number":102,"context_line":"                     resp.getheader(\u0027x-timestamp\u0027) or 0)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"def delay_denial(func):"}],"source_content_type":"text/x-python","patch_set":10,"id":"80ba0e9a_76dfa7c0","side":"PARENT","line":102,"updated":"2023-08-02 23:18:45.000000000","message":"ok, and this is now a source method","commit_id":"8b900becd5bed2e0c0555cc55bcaef372910b88b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":99,"context_line":"    return Timestamp(resp.getheader(\u0027x-backend-data-timestamp\u0027) or"},{"line_number":100,"context_line":"                     resp.getheader(\u0027x-backend-timestamp\u0027) or"},{"line_number":101,"context_line":"                     resp.getheader(\u0027x-put-timestamp\u0027) or"},{"line_number":102,"context_line":"                     resp.getheader(\u0027x-timestamp\u0027) or 0)"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"def delay_denial(func):"}],"source_content_type":"text/x-python","patch_set":10,"id":"b2fb22b0_ceebf316","side":"PARENT","line":102,"in_reply_to":"80ba0e9a_76dfa7c0","updated":"2023-08-03 15:34:27.000000000","message":"Ack","commit_id":"8b900becd5bed2e0c0555cc55bcaef372910b88b"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d11af0a5e183ea69c64fbbc2d2e51f1dd953fe36","unresolved":true,"context_lines":[{"line_number":1277,"context_line":"        try:"},{"line_number":1278,"context_line":""},{"line_number":1279,"context_line":"            # This is safe; it sets up a generator but does not call next()"},{"line_number":1280,"context_line":"            # on it, so no IO is performed."},{"line_number":1281,"context_line":"            self.source_parts_iter \u003d http_response_to_document_iters("},{"line_number":1282,"context_line":"                self.source, read_chunk_size\u003dself.app.object_chunk_size)"},{"line_number":1283,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"b4c462d1_38beef08","side":"PARENT","line":1280,"updated":"2023-08-03 00:08:47.000000000","message":"Oh, wait -- so why\u0027d we want to be all cautious about lazy-loading `GetterSource.parts_iter`?","commit_id":"8b900becd5bed2e0c0555cc55bcaef372910b88b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1277,"context_line":"        try:"},{"line_number":1278,"context_line":""},{"line_number":1279,"context_line":"            # This is safe; it sets up a generator but does not call next()"},{"line_number":1280,"context_line":"            # on it, so no IO is performed."},{"line_number":1281,"context_line":"            self.source_parts_iter \u003d http_response_to_document_iters("},{"line_number":1282,"context_line":"                self.source, read_chunk_size\u003dself.app.object_chunk_size)"},{"line_number":1283,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ce4820a6_395416a6","side":"PARENT","line":1280,"in_reply_to":"b4c462d1_38beef08","updated":"2023-08-03 15:34:27.000000000","message":"IIUC because it\u0027s only needed for a GET","commit_id":"8b900becd5bed2e0c0555cc55bcaef372910b88b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":1280,"context_line":"            # on it, so no IO is performed."},{"line_number":1281,"context_line":"            self.source_parts_iter \u003d http_response_to_document_iters("},{"line_number":1282,"context_line":"                self.source, read_chunk_size\u003dself.app.object_chunk_size)"},{"line_number":1283,"context_line":""},{"line_number":1284,"context_line":"            part_iter \u003d None"},{"line_number":1285,"context_line":"            try:"},{"line_number":1286,"context_line":"                while True:"}],"source_content_type":"text/x-python","patch_set":10,"id":"6e1ae0d5_cb1865fa","side":"PARENT","line":1283,"updated":"2023-08-02 23:18:45.000000000","message":"ah, is this why it\u0027s useful to make the source.part_iter lazy load?","commit_id":"8b900becd5bed2e0c0555cc55bcaef372910b88b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1280,"context_line":"            # on it, so no IO is performed."},{"line_number":1281,"context_line":"            self.source_parts_iter \u003d http_response_to_document_iters("},{"line_number":1282,"context_line":"                self.source, read_chunk_size\u003dself.app.object_chunk_size)"},{"line_number":1283,"context_line":""},{"line_number":1284,"context_line":"            part_iter \u003d None"},{"line_number":1285,"context_line":"            try:"},{"line_number":1286,"context_line":"                while True:"}],"source_content_type":"text/x-python","patch_set":10,"id":"8ca39c73_88d42f3f","side":"PARENT","line":1283,"in_reply_to":"6e1ae0d5_cb1865fa","updated":"2023-08-03 15:34:27.000000000","message":"yes\n\non master, we _get_source_and_node() at the start of get_working_response() but only create the parts_iter if we reach here for a GET (HEAD doesn\u0027t _make_app_iter()).\n\nwith this patch, we find_source() at the start of get_working_response() but don\u0027t want to create the part_iter there, so lazily create it when the property is accessed (in get_next_doc_part)","commit_id":"8b900becd5bed2e0c0555cc55bcaef372910b88b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":1336,"context_line":"        finally:"},{"line_number":1337,"context_line":"            # Close-out the connection as best as possible."},{"line_number":1338,"context_line":"            if getattr(self.source, \u0027swift_conn\u0027, None):"},{"line_number":1339,"context_line":"                close_swift_conn(self.source)"},{"line_number":1340,"context_line":""},{"line_number":1341,"context_line":"    @property"},{"line_number":1342,"context_line":"    def last_status(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"d4cd7d90_471dd1de","side":"PARENT","line":1339,"updated":"2023-08-02 23:18:45.000000000","message":"it\u0027s actually quite nice to have this pattern encapsalated - kudos","commit_id":"8b900becd5bed2e0c0555cc55bcaef372910b88b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1336,"context_line":"        finally:"},{"line_number":1337,"context_line":"            # Close-out the connection as best as possible."},{"line_number":1338,"context_line":"            if getattr(self.source, \u0027swift_conn\u0027, None):"},{"line_number":1339,"context_line":"                close_swift_conn(self.source)"},{"line_number":1340,"context_line":""},{"line_number":1341,"context_line":"    @property"},{"line_number":1342,"context_line":"    def last_status(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"2a18ec9e_2f931272","side":"PARENT","line":1339,"in_reply_to":"d4cd7d90_471dd1de","updated":"2023-08-03 15:34:27.000000000","message":"Ack","commit_id":"8b900becd5bed2e0c0555cc55bcaef372910b88b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":1499,"context_line":"            # has two versions of the same object, we might end up switching"},{"line_number":1500,"context_line":"            # between old and new mid-stream and giving garbage to the client."},{"line_number":1501,"context_line":"            self.used_source_etag \u003d normalize_etag(src_headers.get(\u0027etag\u0027, \u0027\u0027))"},{"line_number":1502,"context_line":"            self.node \u003d node"},{"line_number":1503,"context_line":"            return source, node"},{"line_number":1504,"context_line":"        return None, None"},{"line_number":1505,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"0d9be0f7_d1561263","side":"PARENT","line":1502,"updated":"2023-08-02 23:18:45.000000000","message":"maybe weird this didn\u0027t already set self.source","commit_id":"8b900becd5bed2e0c0555cc55bcaef372910b88b"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1499,"context_line":"            # has two versions of the same object, we might end up switching"},{"line_number":1500,"context_line":"            # between old and new mid-stream and giving garbage to the client."},{"line_number":1501,"context_line":"            self.used_source_etag \u003d normalize_etag(src_headers.get(\u0027etag\u0027, \u0027\u0027))"},{"line_number":1502,"context_line":"            self.node \u003d node"},{"line_number":1503,"context_line":"            return source, node"},{"line_number":1504,"context_line":"        return None, None"},{"line_number":1505,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"1d6cde47_59027376","side":"PARENT","line":1502,"in_reply_to":"0d9be0f7_d1561263","updated":"2023-08-03 15:34:27.000000000","message":"Ack","commit_id":"8b900becd5bed2e0c0555cc55bcaef372910b88b"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":1006,"context_line":""},{"line_number":1007,"context_line":"    def __init__(self, app, resp, node):"},{"line_number":1008,"context_line":"        self.app \u003d app"},{"line_number":1009,"context_line":"        self.resp \u003d resp"},{"line_number":1010,"context_line":"        self.node \u003d node"},{"line_number":1011,"context_line":"        self._parts_iter \u003d None"},{"line_number":1012,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"2106890f_0d73060f","line":1009,"updated":"2023-08-02 23:18:45.000000000","message":"the self.source object ends up having it\u0027s resp accessed a lot - it\u0027s almost like it *is* a resp (with some other attributes hanging off)","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1006,"context_line":""},{"line_number":1007,"context_line":"    def __init__(self, app, resp, node):"},{"line_number":1008,"context_line":"        self.app \u003d app"},{"line_number":1009,"context_line":"        self.resp \u003d resp"},{"line_number":1010,"context_line":"        self.node \u003d node"},{"line_number":1011,"context_line":"        self._parts_iter \u003d None"},{"line_number":1012,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"13d46943_b90ae641","line":1009,"in_reply_to":"2106890f_0d73060f","updated":"2023-08-03 15:34:27.000000000","message":"Ack","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d11af0a5e183ea69c64fbbc2d2e51f1dd953fe36","unresolved":true,"context_lines":[{"line_number":1010,"context_line":"        self.node \u003d node"},{"line_number":1011,"context_line":"        self._parts_iter \u003d None"},{"line_number":1012,"context_line":""},{"line_number":1013,"context_line":"    def timestamp(self):"},{"line_number":1014,"context_line":"        \"\"\""},{"line_number":1015,"context_line":"        Provide the timestamp of the swift http response as a floating"},{"line_number":1016,"context_line":"        point value.  Used as a sort key."}],"source_content_type":"text/x-python","patch_set":10,"id":"a454d642_8a2c61f1","line":1013,"updated":"2023-08-03 00:08:47.000000000","message":"Feels a little weird to leave it as a method -- why not a property?","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1010,"context_line":"        self.node \u003d node"},{"line_number":1011,"context_line":"        self._parts_iter \u003d None"},{"line_number":1012,"context_line":""},{"line_number":1013,"context_line":"    def timestamp(self):"},{"line_number":1014,"context_line":"        \"\"\""},{"line_number":1015,"context_line":"        Provide the timestamp of the swift http response as a floating"},{"line_number":1016,"context_line":"        point value.  Used as a sort key."}],"source_content_type":"text/x-python","patch_set":10,"id":"6e156780_c548d51a","line":1013,"in_reply_to":"a454d642_8a2c61f1","updated":"2023-08-03 15:34:27.000000000","message":"it needs to be callable to be used as a sort key, but I can see that has stirred a debate in itself! will change","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":1027,"context_line":"        # lazy load a source parts iter if and when the source is actually read"},{"line_number":1028,"context_line":"        if self.resp and not self._parts_iter:"},{"line_number":1029,"context_line":"            self._parts_iter \u003d http_response_to_document_iters("},{"line_number":1030,"context_line":"                self.resp, read_chunk_size\u003dself.app.object_chunk_size)"},{"line_number":1031,"context_line":"        return self._parts_iter"},{"line_number":1032,"context_line":""},{"line_number":1033,"context_line":"    def close(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"9f26fdbc_c3a96aff","line":1030,"updated":"2023-08-02 23:18:45.000000000","message":"are we passing in app just to get this one object_chunk_size configuration value?","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1027,"context_line":"        # lazy load a source parts iter if and when the source is actually read"},{"line_number":1028,"context_line":"        if self.resp and not self._parts_iter:"},{"line_number":1029,"context_line":"            self._parts_iter \u003d http_response_to_document_iters("},{"line_number":1030,"context_line":"                self.resp, read_chunk_size\u003dself.app.object_chunk_size)"},{"line_number":1031,"context_line":"        return self._parts_iter"},{"line_number":1032,"context_line":""},{"line_number":1033,"context_line":"    def close(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"144b0aec_ded7865c","line":1030,"in_reply_to":"9f26fdbc_c3a96aff","updated":"2023-08-03 15:34:27.000000000","message":"yes, which is a little frustrating. However, I tend to prefer passing around a reference to the \"mother-ship\" so that implementations have access to all they might need, rather than passing a subset of dereferenced attributes and potentially having to plumb though more in future.","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":1028,"context_line":"        if self.resp and not self._parts_iter:"},{"line_number":1029,"context_line":"            self._parts_iter \u003d http_response_to_document_iters("},{"line_number":1030,"context_line":"                self.resp, read_chunk_size\u003dself.app.object_chunk_size)"},{"line_number":1031,"context_line":"        return self._parts_iter"},{"line_number":1032,"context_line":""},{"line_number":1033,"context_line":"    def close(self):"},{"line_number":1034,"context_line":"        # Close-out the connection as best as possible."}],"source_content_type":"text/x-python","patch_set":10,"id":"84fb2fbf_d6dc89e3","line":1031,"updated":"2023-08-02 23:18:45.000000000","message":"sort of thining through when self._parts_iter is an un-started generator or something that\u0027s being itered or having next called on\n\n... i guess a generator is a generator\n\n\t\u003e\u003e\u003e x \u003d [1, 2, 3]\n\t\u003e\u003e\u003e x\n\t[1, 2, 3]\n\t\u003e\u003e\u003e id(x)\n\t140670819546048\n\t\u003e\u003e\u003e id(iter(x))\n\t140670821350752\n\t\u003e\u003e\u003e def g():\n\t...     for i in range(10):\n\t...         yield i\n\t... \n\t\u003e\u003e\u003e x \u003d g()\n\t\u003e\u003e\u003e id(x)\n\t140670821182128\n\t\u003e\u003e\u003e id(iter(x))\n\t140670821182128","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d11af0a5e183ea69c64fbbc2d2e51f1dd953fe36","unresolved":true,"context_lines":[{"line_number":1032,"context_line":""},{"line_number":1033,"context_line":"    def close(self):"},{"line_number":1034,"context_line":"        # Close-out the connection as best as possible."},{"line_number":1035,"context_line":"        if self.resp and getattr(self.resp, \u0027swift_conn\u0027, None):"},{"line_number":1036,"context_line":"            close_swift_conn(self.resp)"},{"line_number":1037,"context_line":""},{"line_number":1038,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"bfa6e8d3_9aa61700","line":1035,"range":{"start_line":1035,"start_character":21,"end_line":1035,"end_character":63},"updated":"2023-08-03 00:08:47.000000000","message":"nit: Given the nature of `close_swift_conn`, this seems unnecessarily cautious 😜","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1032,"context_line":""},{"line_number":1033,"context_line":"    def close(self):"},{"line_number":1034,"context_line":"        # Close-out the connection as best as possible."},{"line_number":1035,"context_line":"        if self.resp and getattr(self.resp, \u0027swift_conn\u0027, None):"},{"line_number":1036,"context_line":"            close_swift_conn(self.resp)"},{"line_number":1037,"context_line":""},{"line_number":1038,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"cff6eec3_64befade","line":1035,"range":{"start_line":1035,"start_character":21,"end_line":1035,"end_character":63},"in_reply_to":"bfa6e8d3_9aa61700","updated":"2023-08-03 15:34:27.000000000","message":"Done","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":1056,"context_line":"        ``self.source``."},{"line_number":1057,"context_line":""},{"line_number":1058,"context_line":"        :return: ``True`` if ``self.source`` has been updated, ``False``"},{"line_number":1059,"context_line":"            otherwise."},{"line_number":1060,"context_line":"        \"\"\""},{"line_number":1061,"context_line":"        # Subclasses must implement this method"},{"line_number":1062,"context_line":"        raise NotImplementedError()"}],"source_content_type":"text/x-python","patch_set":10,"id":"82680859_e1696f8d","line":1059,"updated":"2023-08-02 23:18:45.000000000","message":"oh, you even called out the return signature change","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1056,"context_line":"        ``self.source``."},{"line_number":1057,"context_line":""},{"line_number":1058,"context_line":"        :return: ``True`` if ``self.source`` has been updated, ``False``"},{"line_number":1059,"context_line":"            otherwise."},{"line_number":1060,"context_line":"        \"\"\""},{"line_number":1061,"context_line":"        # Subclasses must implement this method"},{"line_number":1062,"context_line":"        raise NotImplementedError()"}],"source_content_type":"text/x-python","patch_set":10,"id":"ad4f275d_d04560ce","line":1059,"in_reply_to":"82680859_e1696f8d","updated":"2023-08-03 15:34:27.000000000","message":"Ack","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d11af0a5e183ea69c64fbbc2d2e51f1dd953fe36","unresolved":true,"context_lines":[{"line_number":1062,"context_line":"        raise NotImplementedError()"},{"line_number":1063,"context_line":""},{"line_number":1064,"context_line":"    def _replace_source(self, err_msg):"},{"line_number":1065,"context_line":"        # _find_source can modify self.source so stash current source"},{"line_number":1066,"context_line":"        old_source \u003d self.source"},{"line_number":1067,"context_line":"        if not self._find_source():"},{"line_number":1068,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":10,"id":"916a3455_ff82c4f7","line":1065,"updated":"2023-08-03 00:08:47.000000000","message":"Off-topic: this is still a smell -- I feel like we could probably find a way to get rid of `_replace_source` entirely and have `_find_source` take responsibility for closing out any existing, error\u0027ed source since it\u0027s the one replacing `self.source`.","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"db011ae97a4f9158bfe27793cd03676d9a76b571","unresolved":false,"context_lines":[{"line_number":1062,"context_line":"        raise NotImplementedError()"},{"line_number":1063,"context_line":""},{"line_number":1064,"context_line":"    def _replace_source(self, err_msg):"},{"line_number":1065,"context_line":"        # _find_source can modify self.source so stash current source"},{"line_number":1066,"context_line":"        old_source \u003d self.source"},{"line_number":1067,"context_line":"        if not self._find_source():"},{"line_number":1068,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":10,"id":"c5ce0dd9_678850da","line":1065,"in_reply_to":"916a3455_ff82c4f7","updated":"2023-08-04 14:05:26.000000000","message":"see 890527: proxy-server: error limit nodes that cannot be replaced | https://review.opendev.org/c/openstack/swift/+/890527","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":1065,"context_line":"        # _find_source can modify self.source so stash current source"},{"line_number":1066,"context_line":"        old_source \u003d self.source"},{"line_number":1067,"context_line":"        if not self._find_source():"},{"line_number":1068,"context_line":"            return False"},{"line_number":1069,"context_line":""},{"line_number":1070,"context_line":"        self.app.error_occurred(old_source.node, err_msg)"},{"line_number":1071,"context_line":"        old_source.close()"}],"source_content_type":"text/x-python","patch_set":10,"id":"eb7d9ede_d62adc5b","line":1068,"updated":"2023-08-02 23:18:45.000000000","message":"i guess it\u0027s keeping existing behavior, but I wonder if it\u0027d be reasonable to close old_source in a finally","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"db011ae97a4f9158bfe27793cd03676d9a76b571","unresolved":false,"context_lines":[{"line_number":1065,"context_line":"        # _find_source can modify self.source so stash current source"},{"line_number":1066,"context_line":"        old_source \u003d self.source"},{"line_number":1067,"context_line":"        if not self._find_source():"},{"line_number":1068,"context_line":"            return False"},{"line_number":1069,"context_line":""},{"line_number":1070,"context_line":"        self.app.error_occurred(old_source.node, err_msg)"},{"line_number":1071,"context_line":"        old_source.close()"}],"source_content_type":"text/x-python","patch_set":10,"id":"08016771_6673dc65","line":1068,"in_reply_to":"eb7d9ede_d62adc5b","updated":"2023-08-04 14:05:26.000000000","message":"not sure if it addresses this comment but see 890527: proxy-server: error limit nodes that cannot be replaced | https://review.opendev.org/c/openstack/swift/+/890527","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d11af0a5e183ea69c64fbbc2d2e51f1dd953fe36","unresolved":true,"context_lines":[{"line_number":1067,"context_line":"        if not self._find_source():"},{"line_number":1068,"context_line":"            return False"},{"line_number":1069,"context_line":""},{"line_number":1070,"context_line":"        self.app.error_occurred(old_source.node, err_msg)"},{"line_number":1071,"context_line":"        old_source.close()"},{"line_number":1072,"context_line":"        return True"},{"line_number":1073,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"a7613dde_823d2178","line":1070,"updated":"2023-08-03 00:08:47.000000000","message":"Off-topic: Kinda weird that we only call `error_occurred` if we can find a replacement... I guess it\u0027s that some higher-level error handler takes care of it (and closing `self.source`)?","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1067,"context_line":"        if not self._find_source():"},{"line_number":1068,"context_line":"            return False"},{"line_number":1069,"context_line":""},{"line_number":1070,"context_line":"        self.app.error_occurred(old_source.node, err_msg)"},{"line_number":1071,"context_line":"        old_source.close()"},{"line_number":1072,"context_line":"        return True"},{"line_number":1073,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"44f7e85a_439b80f9","line":1070,"in_reply_to":"a7613dde_823d2178","updated":"2023-08-03 15:34:27.000000000","message":"Actually, I have been wondering for a while if that is just a bug (i.e. that we only error limit an errored node if a new node is found) - or is it subtle cleverness (\"no new node found, better keep the old one available\"). Given that until recently the *new* node might be erroneously error limited, I\u0027m not too confident in the cleverness theory.\n\nI\u0027d like to like it separate from this patch, but I\u0027d entertain moving line 1070-1071 ahead of 1067.","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":1295,"context_line":"            try:"},{"line_number":1296,"context_line":"                while True:"},{"line_number":1297,"context_line":"                    start_byte, end_byte, length, headers, part \u003d \\"},{"line_number":1298,"context_line":"                        self.get_next_doc_part()"},{"line_number":1299,"context_line":"                    self.learn_size_from_content_range("},{"line_number":1300,"context_line":"                        start_byte, end_byte, length)"},{"line_number":1301,"context_line":"                    self.bytes_used_from_backend \u003d 0"}],"source_content_type":"text/x-python","patch_set":10,"id":"9e587ea5_3118d061","line":1298,"updated":"2023-08-02 23:18:45.000000000","message":"this method is going ot initialize/access self.source.part_iter","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1295,"context_line":"            try:"},{"line_number":1296,"context_line":"                while True:"},{"line_number":1297,"context_line":"                    start_byte, end_byte, length, headers, part \u003d \\"},{"line_number":1298,"context_line":"                        self.get_next_doc_part()"},{"line_number":1299,"context_line":"                    self.learn_size_from_content_range("},{"line_number":1300,"context_line":"                        start_byte, end_byte, length)"},{"line_number":1301,"context_line":"                    self.bytes_used_from_backend \u003d 0"}],"source_content_type":"text/x-python","patch_set":10,"id":"9d0a5b21_fcfeace6","line":1298,"in_reply_to":"9e587ea5_3118d061","updated":"2023-08-03 15:34:27.000000000","message":"Ack","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d11af0a5e183ea69c64fbbc2d2e51f1dd953fe36","unresolved":true,"context_lines":[{"line_number":1315,"context_line":"                req.environ[\u0027swift.non_client_disconnect\u0027] \u003d True"},{"line_number":1316,"context_line":"            finally:"},{"line_number":1317,"context_line":"                if part_iter:"},{"line_number":1318,"context_line":"                    part_iter.close()"},{"line_number":1319,"context_line":""},{"line_number":1320,"context_line":"        except ChunkReadTimeout:"},{"line_number":1321,"context_line":"            self.app.exception_occurred(self.node, \u0027Object\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"bce1b17e_6fed5045","line":1318,"updated":"2023-08-03 00:08:47.000000000","message":"🤔 Oh yeah, we want to close part_iters promptly... Should we have a\n```\nif self._parts_iter:\n    self._parts_iter.close()\n```\nin `GetterSource.close`?","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c0bc85310891dc8ee0e0e59da8c8591e363b956e","unresolved":false,"context_lines":[{"line_number":1315,"context_line":"                req.environ[\u0027swift.non_client_disconnect\u0027] \u003d True"},{"line_number":1316,"context_line":"            finally:"},{"line_number":1317,"context_line":"                if part_iter:"},{"line_number":1318,"context_line":"                    part_iter.close()"},{"line_number":1319,"context_line":""},{"line_number":1320,"context_line":"        except ChunkReadTimeout:"},{"line_number":1321,"context_line":"            self.app.exception_occurred(self.node, \u0027Object\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"0584f8b6_ed474fc6","line":1318,"in_reply_to":"7c84ed1d_082340e4","updated":"2023-08-04 23:06:46.000000000","message":"👍","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1315,"context_line":"                req.environ[\u0027swift.non_client_disconnect\u0027] \u003d True"},{"line_number":1316,"context_line":"            finally:"},{"line_number":1317,"context_line":"                if part_iter:"},{"line_number":1318,"context_line":"                    part_iter.close()"},{"line_number":1319,"context_line":""},{"line_number":1320,"context_line":"        except ChunkReadTimeout:"},{"line_number":1321,"context_line":"            self.app.exception_occurred(self.node, \u0027Object\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"7c84ed1d_082340e4","line":1318,"in_reply_to":"bce1b17e_6fed5045","updated":"2023-08-03 15:34:27.000000000","message":"I\u0027m not sure...can we consider that off-topic? - elsewhere I am digging into how and when iters are closed https://review.opendev.org/c/openstack/swift/+/890336?usp\u003ddashboard","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d11af0a5e183ea69c64fbbc2d2e51f1dd953fe36","unresolved":false,"context_lines":[{"line_number":1344,"context_line":"            self.logger.exception(\u0027Trying to send to client\u0027)"},{"line_number":1345,"context_line":"            raise"},{"line_number":1346,"context_line":"        finally:"},{"line_number":1347,"context_line":"            self.source.close()"},{"line_number":1348,"context_line":""},{"line_number":1349,"context_line":"    @property"},{"line_number":1350,"context_line":"    def last_status(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"20f9505d_dace8617","line":1347,"updated":"2023-08-03 00:08:47.000000000","message":"I think I get why we can\u0027t, but this really makes me want a `with closing(self.source):` somewhere...","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":1459,"context_line":"                                    self.bodies[-1])"},{"line_number":1460,"context_line":"        return False"},{"line_number":1461,"context_line":""},{"line_number":1462,"context_line":"    def _find_source(self):"},{"line_number":1463,"context_line":"        self.statuses \u003d []"},{"line_number":1464,"context_line":"        self.reasons \u003d []"},{"line_number":1465,"context_line":"        self.bodies \u003d []"}],"source_content_type":"text/x-python","patch_set":10,"id":"b112af86_f5ddba01","line":1462,"updated":"2023-08-02 23:18:45.000000000","message":"ok, and the concrete implementaiton of _get_source_and_node also gets renamed","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1459,"context_line":"                                    self.bodies[-1])"},{"line_number":1460,"context_line":"        return False"},{"line_number":1461,"context_line":""},{"line_number":1462,"context_line":"    def _find_source(self):"},{"line_number":1463,"context_line":"        self.statuses \u003d []"},{"line_number":1464,"context_line":"        self.reasons \u003d []"},{"line_number":1465,"context_line":"        self.bodies \u003d []"}],"source_content_type":"text/x-python","patch_set":10,"id":"00790295_ff2a6f85","line":1462,"in_reply_to":"b112af86_f5ddba01","updated":"2023-08-03 15:34:27.000000000","message":"Ack","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":1493,"context_line":"                            if s.timestamp() \u003e\u003d self.latest_404_timestamp]"},{"line_number":1494,"context_line":""},{"line_number":1495,"context_line":"        if self.sources:"},{"line_number":1496,"context_line":"            self.sources.sort(key\u003dGetterSource.timestamp)"},{"line_number":1497,"context_line":"            source \u003d self.sources.pop()"},{"line_number":1498,"context_line":"            for unused_source in self.sources:"},{"line_number":1499,"context_line":"                unused_source.close()"}],"source_content_type":"text/x-python","patch_set":10,"id":"bd4ec1f9_894732c1","line":1496,"updated":"2023-08-02 23:18:45.000000000","message":"GetterSource.timestamp is not a classmethod, this is kind of an interesting pattern\n\nmaybe `key\u003dlambda s: s.timestamp()` would be slightly more obvious to me at a glance or even make timestamp a property","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d11af0a5e183ea69c64fbbc2d2e51f1dd953fe36","unresolved":true,"context_lines":[{"line_number":1493,"context_line":"                            if s.timestamp() \u003e\u003d self.latest_404_timestamp]"},{"line_number":1494,"context_line":""},{"line_number":1495,"context_line":"        if self.sources:"},{"line_number":1496,"context_line":"            self.sources.sort(key\u003dGetterSource.timestamp)"},{"line_number":1497,"context_line":"            source \u003d self.sources.pop()"},{"line_number":1498,"context_line":"            for unused_source in self.sources:"},{"line_number":1499,"context_line":"                unused_source.close()"}],"source_content_type":"text/x-python","patch_set":10,"id":"f6be6c83_4d66f01e","line":1496,"range":{"start_line":1496,"start_character":34,"end_line":1496,"end_character":56},"updated":"2023-08-03 00:08:47.000000000","message":"This feels dirty -- could we at least do `operator.attrgetter(\u0027timestamp\u0027)` or something? If we ever want to subclass the thing with some alternative notion of `timestamp`, this feels like a pretty good foot-gun.\n\nAlternatively, maybe we\u0027d want to implement a `__lt__` for `GetterSource`, so we don\u0027t have to specify a key?","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1493,"context_line":"                            if s.timestamp() \u003e\u003d self.latest_404_timestamp]"},{"line_number":1494,"context_line":""},{"line_number":1495,"context_line":"        if self.sources:"},{"line_number":1496,"context_line":"            self.sources.sort(key\u003dGetterSource.timestamp)"},{"line_number":1497,"context_line":"            source \u003d self.sources.pop()"},{"line_number":1498,"context_line":"            for unused_source in self.sources:"},{"line_number":1499,"context_line":"                unused_source.close()"}],"source_content_type":"text/x-python","patch_set":10,"id":"007217cb_9097c2af","line":1496,"in_reply_to":"bd4ec1f9_894732c1","updated":"2023-08-03 15:34:27.000000000","message":"This was a learning experience for me when I wrote it, sort_key just needs to be a callable that takes a single argument, and that is what GetterSource.timestamp is. I admit to being surprised. But in fact the example in the docs uses str.lower which is not a classmethod. https://docs.python.org/3/howto/sorting.html#key-functions\n\nHowever, I see I can avoid both your\u0027s and Tim\u0027s objections using attrgetter and a property!","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1493,"context_line":"                            if s.timestamp() \u003e\u003d self.latest_404_timestamp]"},{"line_number":1494,"context_line":""},{"line_number":1495,"context_line":"        if self.sources:"},{"line_number":1496,"context_line":"            self.sources.sort(key\u003dGetterSource.timestamp)"},{"line_number":1497,"context_line":"            source \u003d self.sources.pop()"},{"line_number":1498,"context_line":"            for unused_source in self.sources:"},{"line_number":1499,"context_line":"                unused_source.close()"}],"source_content_type":"text/x-python","patch_set":10,"id":"dba37f73_228914f2","line":1496,"range":{"start_line":1496,"start_character":34,"end_line":1496,"end_character":56},"in_reply_to":"f6be6c83_4d66f01e","updated":"2023-08-03 15:34:27.000000000","message":"ok, the `attrgetter` + property approach seems good 👍\n\nI avoided `__lt__` just because it seemed a big intuitive leap to think that sources would *by default* be sorted by timestamps - I think I\u0027d forever be going to read the `__lt__` to know what the sort order was.","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d11af0a5e183ea69c64fbbc2d2e51f1dd953fe36","unresolved":true,"context_lines":[{"line_number":1500,"context_line":"            self.used_nodes.append(source.node)"},{"line_number":1501,"context_line":"            src_headers \u003d dict("},{"line_number":1502,"context_line":"                (k.lower(), v) for k, v in"},{"line_number":1503,"context_line":"                source.resp.getheaders())"},{"line_number":1504,"context_line":""},{"line_number":1505,"context_line":"            # Save off the source etag so that, if we lose the connection"},{"line_number":1506,"context_line":"            # and have to resume from a different node, we can be sure that"}],"source_content_type":"text/x-python","patch_set":10,"id":"9a5b12f8_94817303","line":1503,"updated":"2023-08-03 00:08:47.000000000","message":"Off-topic: feels weird that we get all the headers when we\u0027re only interested in the etag -- I wonder why we don\u0027t just say `self.used_source_etag \u003d normalize_etag(source.resp.getheader(\u0027etag\u0027) or \u0027\u0027)` or something.","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":true,"context_lines":[{"line_number":1500,"context_line":"            self.used_nodes.append(source.node)"},{"line_number":1501,"context_line":"            src_headers \u003d dict("},{"line_number":1502,"context_line":"                (k.lower(), v) for k, v in"},{"line_number":1503,"context_line":"                source.resp.getheaders())"},{"line_number":1504,"context_line":""},{"line_number":1505,"context_line":"            # Save off the source etag so that, if we lose the connection"},{"line_number":1506,"context_line":"            # and have to resume from a different node, we can be sure that"}],"source_content_type":"text/x-python","patch_set":10,"id":"f694f16b_cc610590","line":1503,"in_reply_to":"9a5b12f8_94817303","updated":"2023-08-03 15:34:27.000000000","message":"maybe the lower-casing is significant, but it could still be written better","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c0bc85310891dc8ee0e0e59da8c8591e363b956e","unresolved":false,"context_lines":[{"line_number":1500,"context_line":"            self.used_nodes.append(source.node)"},{"line_number":1501,"context_line":"            src_headers \u003d dict("},{"line_number":1502,"context_line":"                (k.lower(), v) for k, v in"},{"line_number":1503,"context_line":"                source.resp.getheaders())"},{"line_number":1504,"context_line":""},{"line_number":1505,"context_line":"            # Save off the source etag so that, if we lose the connection"},{"line_number":1506,"context_line":"            # and have to resume from a different node, we can be sure that"}],"source_content_type":"text/x-python","patch_set":10,"id":"6a182d80_1131f69e","line":1503,"in_reply_to":"f694f16b_cc610590","updated":"2023-08-04 23:06:46.000000000","message":"Pretty sure it\u0027s not significant:\n```\n\u003e\u003e\u003e c\u003dhttp.client.HTTPSConnection(\u0027google.com\u0027)\n\u003e\u003e\u003e c.request(\u0027GET\u0027, \u0027/\u0027)\n\u003e\u003e\u003e r\u003dc.getresponse()\n\u003e\u003e\u003e r.getheaders()[0]\n(\u0027Location\u0027, \u0027https://www.google.com/\u0027)\n\u003e\u003e\u003e r.getheader(\u0027Location\u0027)\n\u0027https://www.google.com/\u0027\n\u003e\u003e\u003e r.getheader(\u0027location\u0027)\n\u0027https://www.google.com/\u0027\n\u003e\u003e\u003e r.getheader(\u0027LOCATION\u0027)\n\u0027https://www.google.com/\u0027\n```\nPut up https://review.opendev.org/c/openstack/swift/+/890578 for it.","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":1510,"context_line":"            self.used_source_etag \u003d normalize_etag(src_headers.get(\u0027etag\u0027, \u0027\u0027))"},{"line_number":1511,"context_line":"            self.source \u003d source"},{"line_number":1512,"context_line":"            return True"},{"line_number":1513,"context_line":"        return False"},{"line_number":1514,"context_line":""},{"line_number":1515,"context_line":"    def _make_app_iter(self, req):"},{"line_number":1516,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"faccb2b0_6a20c310","line":1513,"updated":"2023-08-02 23:18:45.000000000","message":"and here\u0027s the return value to say explictily if we successfully _find_source","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1510,"context_line":"            self.used_source_etag \u003d normalize_etag(src_headers.get(\u0027etag\u0027, \u0027\u0027))"},{"line_number":1511,"context_line":"            self.source \u003d source"},{"line_number":1512,"context_line":"            return True"},{"line_number":1513,"context_line":"        return False"},{"line_number":1514,"context_line":""},{"line_number":1515,"context_line":"    def _make_app_iter(self, req):"},{"line_number":1516,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"f4e71c31_e9992bc2","line":1513,"in_reply_to":"faccb2b0_6a20c310","updated":"2023-08-03 15:34:27.000000000","message":"Ack","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":1558,"context_line":"                    self.source.resp.status in (HTTP_OK, HTTP_PARTIAL_CONTENT):"},{"line_number":1559,"context_line":"                res.app_iter \u003d self._make_app_iter(req)"},{"line_number":1560,"context_line":"                # See NOTE: swift_conn at top of file about this."},{"line_number":1561,"context_line":"                res.swift_conn \u003d self.source.resp.swift_conn"},{"line_number":1562,"context_line":"            if not res.environ:"},{"line_number":1563,"context_line":"                res.environ \u003d {}"},{"line_number":1564,"context_line":"            res.environ[\u0027swift_x_timestamp\u0027] \u003d self.source.resp.getheader("}],"source_content_type":"text/x-python","patch_set":10,"id":"d3e9a3bf_ff7dc1a8","line":1561,"updated":"2023-08-02 23:18:45.000000000","message":"ok, this isn\u0027t \"leaking\" we\u0027re transfering the backend connection to the client facing response object.","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1558,"context_line":"                    self.source.resp.status in (HTTP_OK, HTTP_PARTIAL_CONTENT):"},{"line_number":1559,"context_line":"                res.app_iter \u003d self._make_app_iter(req)"},{"line_number":1560,"context_line":"                # See NOTE: swift_conn at top of file about this."},{"line_number":1561,"context_line":"                res.swift_conn \u003d self.source.resp.swift_conn"},{"line_number":1562,"context_line":"            if not res.environ:"},{"line_number":1563,"context_line":"                res.environ \u003d {}"},{"line_number":1564,"context_line":"            res.environ[\u0027swift_x_timestamp\u0027] \u003d self.source.resp.getheader("}],"source_content_type":"text/x-python","patch_set":10,"id":"982de380_5751a1c8","line":1561,"in_reply_to":"d3e9a3bf_ff7dc1a8","updated":"2023-08-03 15:34:27.000000000","message":"Ack","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":1563,"context_line":"                res.environ \u003d {}"},{"line_number":1564,"context_line":"            res.environ[\u0027swift_x_timestamp\u0027] \u003d self.source.resp.getheader("},{"line_number":1565,"context_line":"                \u0027x-timestamp\u0027)"},{"line_number":1566,"context_line":"            res.accept_ranges \u003d \u0027bytes\u0027"},{"line_number":1567,"context_line":"            res.content_length \u003d self.source.resp.getheader(\u0027Content-Length\u0027)"},{"line_number":1568,"context_line":"            if self.source.resp.getheader(\u0027Content-Type\u0027):"},{"line_number":1569,"context_line":"                res.charset \u003d None"}],"source_content_type":"text/x-python","patch_set":10,"id":"88313cf4_d78ea108","line":1566,"updated":"2023-08-02 23:18:45.000000000","message":"hey i was looking for this!  i should do more reviews!","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1563,"context_line":"                res.environ \u003d {}"},{"line_number":1564,"context_line":"            res.environ[\u0027swift_x_timestamp\u0027] \u003d self.source.resp.getheader("},{"line_number":1565,"context_line":"                \u0027x-timestamp\u0027)"},{"line_number":1566,"context_line":"            res.accept_ranges \u003d \u0027bytes\u0027"},{"line_number":1567,"context_line":"            res.content_length \u003d self.source.resp.getheader(\u0027Content-Length\u0027)"},{"line_number":1568,"context_line":"            if self.source.resp.getheader(\u0027Content-Type\u0027):"},{"line_number":1569,"context_line":"                res.charset \u003d None"}],"source_content_type":"text/x-python","patch_set":10,"id":"3d45fe83_bdebd3d7","line":1566,"in_reply_to":"88313cf4_d78ea108","updated":"2023-08-03 15:34:27.000000000","message":"😄","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"}],"swift/proxy/controllers/obj.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5e1ff3c066cf6b9a5cbb26dbf66325f7445ca560","unresolved":true,"context_lines":[{"line_number":2514,"context_line":"        if source:"},{"line_number":2515,"context_line":"            self.source \u003d GetterSource(self.app, source, node)"},{"line_number":2516,"context_line":"            return self._get_response_parts_iter(req)"},{"line_number":2517,"context_line":"        return None"},{"line_number":2518,"context_line":""},{"line_number":2519,"context_line":"    def get_next_doc_part(self):"},{"line_number":2520,"context_line":"        node_timeout \u003d self.app.recoverable_node_timeout"}],"source_content_type":"text/x-python","patch_set":3,"id":"814f43b4_d4e61d90","line":2517,"updated":"2023-07-10 19:33:07.000000000","message":"I think i prefered the single return","commit_id":"ac2efb883b0fbf624aed302229057c2c389c1fbd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"37590707c32e4c0ab09ad8d17f2b1d3b0cc5b0c5","unresolved":false,"context_lines":[{"line_number":2514,"context_line":"        if source:"},{"line_number":2515,"context_line":"            self.source \u003d GetterSource(self.app, source, node)"},{"line_number":2516,"context_line":"            return self._get_response_parts_iter(req)"},{"line_number":2517,"context_line":"        return None"},{"line_number":2518,"context_line":""},{"line_number":2519,"context_line":"    def get_next_doc_part(self):"},{"line_number":2520,"context_line":"        node_timeout \u003d self.app.recoverable_node_timeout"}],"source_content_type":"text/x-python","patch_set":3,"id":"5059bac3_49e255af","line":2517,"in_reply_to":"814f43b4_d4e61d90","updated":"2023-07-11 11:46:32.000000000","message":"Done","commit_id":"ac2efb883b0fbf624aed302229057c2c389c1fbd"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"a2dcf68cae3c37d6e9d00c7da9a78336ba5cae91","unresolved":true,"context_lines":[{"line_number":2508,"context_line":"    def response_parts_iter(self, req):"},{"line_number":2509,"context_line":"        it \u003d None"},{"line_number":2510,"context_line":"        try:"},{"line_number":2511,"context_line":"            source, node \u003d next(self.source_and_node_iter)"},{"line_number":2512,"context_line":"        except StopIteration:"},{"line_number":2513,"context_line":"            pass"},{"line_number":2514,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"58743e9e_59431de6","line":2511,"range":{"start_line":2511,"start_character":32,"end_line":2511,"end_character":57},"updated":"2023-07-20 01:27:44.000000000","message":"Should this iter return GetterSource instances? Would that simplify things? Just thinking out loud. We\u0027ve emcapsulated source and nodes into GetterSource objects, so should we use them more?","commit_id":"2934c2cfa29116668241ba818d11a0f255cf6bcf"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"577bc14e76f7668f0d511c808d922dc298df5183","unresolved":false,"context_lines":[{"line_number":2508,"context_line":"    def response_parts_iter(self, req):"},{"line_number":2509,"context_line":"        it \u003d None"},{"line_number":2510,"context_line":"        try:"},{"line_number":2511,"context_line":"            source, node \u003d next(self.source_and_node_iter)"},{"line_number":2512,"context_line":"        except StopIteration:"},{"line_number":2513,"context_line":"            pass"},{"line_number":2514,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5d004d5a_47b6fb31","line":2511,"range":{"start_line":2511,"start_character":32,"end_line":2511,"end_character":57},"in_reply_to":"58743e9e_59431de6","updated":"2023-07-24 09:06:38.000000000","message":"Done","commit_id":"2934c2cfa29116668241ba818d11a0f255cf6bcf"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"01795e90ce1c728444ac8b3c153245a4a2243754","unresolved":false,"context_lines":[{"line_number":2508,"context_line":"    def response_parts_iter(self, req):"},{"line_number":2509,"context_line":"        it \u003d None"},{"line_number":2510,"context_line":"        try:"},{"line_number":2511,"context_line":"            source, node \u003d next(self.source_and_node_iter)"},{"line_number":2512,"context_line":"        except StopIteration:"},{"line_number":2513,"context_line":"            pass"},{"line_number":2514,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"ae2df05f_22b150c9","line":2511,"range":{"start_line":2511,"start_character":32,"end_line":2511,"end_character":57},"in_reply_to":"58743e9e_59431de6","updated":"2023-07-24 03:39:06.000000000","message":"Done","commit_id":"2934c2cfa29116668241ba818d11a0f255cf6bcf"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":2324,"context_line":"        for t_frag, frag_set in frag_sets.items():"},{"line_number":2325,"context_line":"            t_frag \u003d Timestamp(t_frag)"},{"line_number":2326,"context_line":"            self._get_bucket(t_frag).add_alternate_nodes("},{"line_number":2327,"context_line":"                get.source.node, frag_set)"},{"line_number":2328,"context_line":"        # If the response includes a durable timestamp then mark that bucket as"},{"line_number":2329,"context_line":"        # durable. Note that this may be a different bucket than the one this"},{"line_number":2330,"context_line":"        # response got added to, and that we may never go and get a durable"}],"source_content_type":"text/x-python","patch_set":10,"id":"0470a6ce_1d05b2a6","line":2327,"updated":"2023-08-02 23:18:45.000000000","message":"so ECFragGetter\u0027s used to have a source and a node - now they have a source... that has a resp and a node.","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":2324,"context_line":"        for t_frag, frag_set in frag_sets.items():"},{"line_number":2325,"context_line":"            t_frag \u003d Timestamp(t_frag)"},{"line_number":2326,"context_line":"            self._get_bucket(t_frag).add_alternate_nodes("},{"line_number":2327,"context_line":"                get.source.node, frag_set)"},{"line_number":2328,"context_line":"        # If the response includes a durable timestamp then mark that bucket as"},{"line_number":2329,"context_line":"        # durable. Note that this may be a different bucket than the one this"},{"line_number":2330,"context_line":"        # response got added to, and that we may never go and get a durable"}],"source_content_type":"text/x-python","patch_set":10,"id":"927f80fc_47524df6","line":2327,"in_reply_to":"0470a6ce_1d05b2a6","updated":"2023-08-03 15:34:27.000000000","message":"Ack","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d11af0a5e183ea69c64fbbc2d2e51f1dd953fe36","unresolved":true,"context_lines":[{"line_number":2752,"context_line":"            if source:"},{"line_number":2753,"context_line":"                yield GetterSource(self.app, source, node)"},{"line_number":2754,"context_line":"            else:"},{"line_number":2755,"context_line":"                yield None"},{"line_number":2756,"context_line":"            self.status \u003d self.reason \u003d self.body \u003d self.source_headers \u003d None"},{"line_number":2757,"context_line":""},{"line_number":2758,"context_line":"    def _find_source(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"aa0fe352_23ec7839","line":2755,"updated":"2023-08-03 00:08:47.000000000","message":"Off-topic: I wonder why we do this, and then `if not source: continue` in `_find_source` -- why not just have `_source_gen` yield actual sources?","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"467cbba6105e33be678fde174efeb5cb200dc0f1","unresolved":false,"context_lines":[{"line_number":2752,"context_line":"            if source:"},{"line_number":2753,"context_line":"                yield GetterSource(self.app, source, node)"},{"line_number":2754,"context_line":"            else:"},{"line_number":2755,"context_line":"                yield None"},{"line_number":2756,"context_line":"            self.status \u003d self.reason \u003d self.body \u003d self.source_headers \u003d None"},{"line_number":2757,"context_line":""},{"line_number":2758,"context_line":"    def _find_source(self):"}],"source_content_type":"text/x-python","patch_set":10,"id":"3b0ab964_8b8285f2","line":2755,"in_reply_to":"aa0fe352_23ec7839","updated":"2023-08-03 15:57:00.000000000","message":"the source_iter is used in 2 places:\n\n_find_source: yielded None is ignore as per your observation\nresponse_parts_iter: yielded None is significant\n\nIt\u0027s confusing!","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":2755,"context_line":"                yield None"},{"line_number":2756,"context_line":"            self.status \u003d self.reason \u003d self.body \u003d self.source_headers \u003d None"},{"line_number":2757,"context_line":""},{"line_number":2758,"context_line":"    def _find_source(self):"},{"line_number":2759,"context_line":"        # capture last used etag before continuation"},{"line_number":2760,"context_line":"        used_etag \u003d self.last_headers.get(\u0027X-Object-Sysmeta-EC-ETag\u0027)"},{"line_number":2761,"context_line":"        for source in self.source_iter:"}],"source_content_type":"text/x-python","patch_set":10,"id":"6ed5b2ce_af03847b","line":2758,"updated":"2023-08-02 23:18:45.000000000","message":"and the other concreate implementation gets renamed as well","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":2755,"context_line":"                yield None"},{"line_number":2756,"context_line":"            self.status \u003d self.reason \u003d self.body \u003d self.source_headers \u003d None"},{"line_number":2757,"context_line":""},{"line_number":2758,"context_line":"    def _find_source(self):"},{"line_number":2759,"context_line":"        # capture last used etag before continuation"},{"line_number":2760,"context_line":"        used_etag \u003d self.last_headers.get(\u0027X-Object-Sysmeta-EC-ETag\u0027)"},{"line_number":2761,"context_line":"        for source in self.source_iter:"}],"source_content_type":"text/x-python","patch_set":10,"id":"b6500dd7_94e9afb0","line":2758,"in_reply_to":"6ed5b2ce_af03847b","updated":"2023-08-03 15:34:27.000000000","message":"Ack","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":2770,"context_line":"            else:"},{"line_number":2771,"context_line":"                self.source \u003d source"},{"line_number":2772,"context_line":"                return True"},{"line_number":2773,"context_line":"        return False"},{"line_number":2774,"context_line":""},{"line_number":2775,"context_line":""},{"line_number":2776,"context_line":"@ObjectControllerRouter.register(EC_POLICY)"}],"source_content_type":"text/x-python","patch_set":10,"id":"b0ad698f_355c4b88","line":2773,"updated":"2023-08-02 23:18:45.000000000","message":"oh, i had missed the return type change in the replicated path","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":2770,"context_line":"            else:"},{"line_number":2771,"context_line":"                self.source \u003d source"},{"line_number":2772,"context_line":"                return True"},{"line_number":2773,"context_line":"        return False"},{"line_number":2774,"context_line":""},{"line_number":2775,"context_line":""},{"line_number":2776,"context_line":"@ObjectControllerRouter.register(EC_POLICY)"}],"source_content_type":"text/x-python","patch_set":10,"id":"211c40e7_a306885b","line":2773,"in_reply_to":"b0ad698f_355c4b88","updated":"2023-08-03 15:34:27.000000000","message":"Ack","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"}],"test/unit/proxy/controllers/test_base.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"01795e90ce1c728444ac8b3c153245a4a2243754","unresolved":true,"context_lines":[{"line_number":1651,"context_line":"        headers[\u0027x-backend-data-timestamp\u0027] \u003d \u00271234567.33333\u0027"},{"line_number":1652,"context_line":"        src \u003d self._make_source(headers, self.node)"},{"line_number":1653,"context_line":"        self.assertIsInstance(src.timestamp(), Timestamp)"},{"line_number":1654,"context_line":"        self.assertEqual(Timestamp(1234567.33333), src.timestamp())"},{"line_number":1655,"context_line":""},{"line_number":1656,"context_line":"    def test_sort(self):"},{"line_number":1657,"context_line":"        # verify sorting by timestamp"}],"source_content_type":"text/x-python","patch_set":8,"id":"c223904b_8f8674de","line":1654,"updated":"2023-07-24 03:39:06.000000000","message":"The code dafaulted to 0 if there was no timestamp in  headers. So we should probably test that case here too.\n\nI\u0027ll push a new patchset so I can give this a better score ;)\nI hope you don\u0027t mind","commit_id":"46e0cc0a11f5afeb121907582c8184deec3e6ddd"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"577bc14e76f7668f0d511c808d922dc298df5183","unresolved":false,"context_lines":[{"line_number":1651,"context_line":"        headers[\u0027x-backend-data-timestamp\u0027] \u003d \u00271234567.33333\u0027"},{"line_number":1652,"context_line":"        src \u003d self._make_source(headers, self.node)"},{"line_number":1653,"context_line":"        self.assertIsInstance(src.timestamp(), Timestamp)"},{"line_number":1654,"context_line":"        self.assertEqual(Timestamp(1234567.33333), src.timestamp())"},{"line_number":1655,"context_line":""},{"line_number":1656,"context_line":"    def test_sort(self):"},{"line_number":1657,"context_line":"        # verify sorting by timestamp"}],"source_content_type":"text/x-python","patch_set":8,"id":"48887df9_16300d6e","line":1654,"in_reply_to":"c223904b_8f8674de","updated":"2023-07-24 09:06:38.000000000","message":"thanks!","commit_id":"46e0cc0a11f5afeb121907582c8184deec3e6ddd"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":1295,"context_line":""},{"line_number":1296,"context_line":"        def mock_find_source():"},{"line_number":1297,"context_line":"            handler.source \u003d GetterSource(self.app, source, node)"},{"line_number":1298,"context_line":"            return True"},{"line_number":1299,"context_line":""},{"line_number":1300,"context_line":"        with mock.patch.object(handler, \u0027_find_source\u0027,"},{"line_number":1301,"context_line":"                               mock_find_source):"}],"source_content_type":"text/x-python","patch_set":10,"id":"952567a1_0395715f","line":1298,"updated":"2023-08-02 23:18:45.000000000","message":"nice","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1295,"context_line":""},{"line_number":1296,"context_line":"        def mock_find_source():"},{"line_number":1297,"context_line":"            handler.source \u003d GetterSource(self.app, source, node)"},{"line_number":1298,"context_line":"            return True"},{"line_number":1299,"context_line":""},{"line_number":1300,"context_line":"        with mock.patch.object(handler, \u0027_find_source\u0027,"},{"line_number":1301,"context_line":"                               mock_find_source):"}],"source_content_type":"text/x-python","patch_set":10,"id":"09af64c0_1a7f9671","line":1298,"in_reply_to":"952567a1_0395715f","updated":"2023-08-03 15:34:27.000000000","message":"Ack","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":1688,"context_line":"        src.resp.swift_conn \u003d mock.MagicMock()"},{"line_number":1689,"context_line":"        src.resp.nuke_from_orbit \u003d mock.MagicMock()"},{"line_number":1690,"context_line":"        src.close()"},{"line_number":1691,"context_line":"        src.resp.nuke_from_orbit.assert_called_once_with()"}],"source_content_type":"text/x-python","patch_set":10,"id":"5d2d00d1_339f1095","line":1691,"updated":"2023-08-02 23:18:45.000000000","message":"i guess you could also test close doesn\u0027t blow up when there is no swift_conn attribute, but there\u0027s probably lots of existing tests that already cover *that* branch","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"41261eb80509e21305ccedf5a7eb0c5d78c1be11","unresolved":false,"context_lines":[{"line_number":1688,"context_line":"        src.resp.swift_conn \u003d mock.MagicMock()"},{"line_number":1689,"context_line":"        src.resp.nuke_from_orbit \u003d mock.MagicMock()"},{"line_number":1690,"context_line":"        src.close()"},{"line_number":1691,"context_line":"        src.resp.nuke_from_orbit.assert_called_once_with()"}],"source_content_type":"text/x-python","patch_set":10,"id":"a570a5df_589bcf7c","line":1691,"in_reply_to":"5d2d00d1_339f1095","updated":"2023-08-03 15:34:27.000000000","message":"Done","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"c0bc85310891dc8ee0e0e59da8c8591e363b956e","unresolved":true,"context_lines":[{"line_number":1695,"context_line":"        # source has no resp"},{"line_number":1696,"context_line":"        src \u003d GetterSource(self.app, None, self.node)"},{"line_number":1697,"context_line":"        src.close()"},{"line_number":1698,"context_line":"        # resp has no swift_conn"},{"line_number":1699,"context_line":"        src \u003d GetterSource(self.app, mock.MagicMock(), self.node)"},{"line_number":1700,"context_line":"        src.close()"}],"source_content_type":"text/x-python","patch_set":11,"id":"30faeeb1_89d11962","line":1698,"updated":"2023-08-04 23:06:46.000000000","message":"You sure about that? I tried adding an assertion:\n```\nself \u003d \u003ctest.unit.proxy.controllers.test_base.TestGetterSource testMethod\u003dtest_close\u003e\n\n    def test_close(self):\n        src \u003d GetterSource(self.app, self.resp, self.node)\n        src.resp.swift_conn \u003d mock.MagicMock()\n        src.resp.nuke_from_orbit \u003d mock.MagicMock()\n        src.close()\n        src.resp.nuke_from_orbit.assert_called_once_with()\n    \n        # verify close is robust...\n        # source has no resp\n        src \u003d GetterSource(self.app, None, self.node)\n        src.close()\n        # resp has no swift_conn\n        src \u003d GetterSource(self.app, mock.MagicMock(), self.node)\n\u003e       self.assertFalse(hasattr(src.resp, \u0027swift_conn\u0027))\nE       AssertionError: True is not false\n\ntest/unit/proxy/controllers/test_base.py:1700: AssertionError\n```\nI think we want a `Namespace` -- it\u0027s even already imported.","commit_id":"24d2976f63191f778bcf351d601876bdf988dbc4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"59318e8108c3429c6ff1557baeaaf5ec59ee937e","unresolved":false,"context_lines":[{"line_number":1695,"context_line":"        # source has no resp"},{"line_number":1696,"context_line":"        src \u003d GetterSource(self.app, None, self.node)"},{"line_number":1697,"context_line":"        src.close()"},{"line_number":1698,"context_line":"        # resp has no swift_conn"},{"line_number":1699,"context_line":"        src \u003d GetterSource(self.app, mock.MagicMock(), self.node)"},{"line_number":1700,"context_line":"        src.close()"}],"source_content_type":"text/x-python","patch_set":11,"id":"6acca082_8c06f6b0","line":1698,"in_reply_to":"30faeeb1_89d11962","updated":"2023-08-07 14:15:11.000000000","message":"Done","commit_id":"24d2976f63191f778bcf351d601876bdf988dbc4"}],"test/unit/proxy/controllers/test_obj.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"5a92cc66b6bfbf3296d59ee42661c6a198290d40","unresolved":true,"context_lines":[{"line_number":6864,"context_line":"        self.app.client_timeout \u003d 0.8"},{"line_number":6865,"context_line":"        it \u003d self.getter.iter_bytes_from_response_part(part, nbytes\u003d9)"},{"line_number":6866,"context_line":"        with mock.patch.object(self.getter, \u0027_find_source\u0027,"},{"line_number":6867,"context_line":"                               return_value\u003dFalse):"},{"line_number":6868,"context_line":"            with mock.patch.object(part, \u0027read\u0027,"},{"line_number":6869,"context_line":"                                   side_effect\u003d[b\u0027some\u0027, ChunkReadTimeout(9)]):"},{"line_number":6870,"context_line":"                with self.assertRaises(ChunkReadTimeout) as cm:"}],"source_content_type":"text/x-python","patch_set":10,"id":"35645651_7ee0db16","line":6867,"updated":"2023-08-02 23:18:45.000000000","message":"nice","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"59318e8108c3429c6ff1557baeaaf5ec59ee937e","unresolved":false,"context_lines":[{"line_number":6864,"context_line":"        self.app.client_timeout \u003d 0.8"},{"line_number":6865,"context_line":"        it \u003d self.getter.iter_bytes_from_response_part(part, nbytes\u003d9)"},{"line_number":6866,"context_line":"        with mock.patch.object(self.getter, \u0027_find_source\u0027,"},{"line_number":6867,"context_line":"                               return_value\u003dFalse):"},{"line_number":6868,"context_line":"            with mock.patch.object(part, \u0027read\u0027,"},{"line_number":6869,"context_line":"                                   side_effect\u003d[b\u0027some\u0027, ChunkReadTimeout(9)]):"},{"line_number":6870,"context_line":"                with self.assertRaises(ChunkReadTimeout) as cm:"}],"source_content_type":"text/x-python","patch_set":10,"id":"4eb4a191_0d310621","line":6867,"in_reply_to":"35645651_7ee0db16","updated":"2023-08-07 14:15:11.000000000","message":"Ack","commit_id":"1d7d58dd4c3db09d53cd90bfa49855f83ba5ae6e"}]}