)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"ad8499fcd360a5a64d84617a6c706e5d91503ce5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"493c8293_66ee4126","updated":"2024-01-04 16:28:58.000000000","message":"I attempted similar here https://review.opendev.org/c/openstack/swift/+/897343","commit_id":"78a40666153e641bbf613998841ec3f7d7d0c78c"},{"author":{"_account_id":6476,"name":"Thomas Goirand","email":"thomas@goirand.fr","username":"thomas-goirand"},"change_message_id":"2f071618eea230517d8036c9f67c60a91c4032dd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e217a277_42574a14","updated":"2024-01-11 11:58:35.000000000","message":"This fixes the bug partially for me (ie: there\u0027s still one unit test failure remaining, but unrelated to this patch).","commit_id":"78a40666153e641bbf613998841ec3f7d7d0c78c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"05168a5b0920d52b690b4027276e35f93e8a04c9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2b0b8abb_77724b70","in_reply_to":"e217a277_42574a14","updated":"2024-01-24 19:04:45.000000000","message":"On the remaining unit test failure: is it captured in https://bugs.launchpad.net/swift/+bug/2051067, or something new?","commit_id":"78a40666153e641bbf613998841ec3f7d7d0c78c"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"dcb4783af3b96023371a3080ba8f59fc6a7a267e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"1bcfc86f_b54d19d8","updated":"2024-02-15 01:58:43.000000000","message":"Yup, much better!","commit_id":"c522f5676e33dd0b9422019818a9106cde1d39f7"}],"swift/common/utils/__init__.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"035e95e2d4847dc7a67e7c8dd4ea3c1e8807c824","unresolved":true,"context_lines":[{"line_number":4448,"context_line":"        def string_along(useful_iter, useless_iter_iter, logger):"},{"line_number":4449,"context_line":"            try:"},{"line_number":4450,"context_line":"                with closing_if_possible(useful_iter):"},{"line_number":4451,"context_line":"                    yield None  # sync point"},{"line_number":4452,"context_line":"                    for x in useful_iter:"},{"line_number":4453,"context_line":"                        yield x"},{"line_number":4454,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"5284e23b_6704125d","line":4451,"updated":"2024-01-08 17:08:20.000000000","message":"I\u0027m not convinced that this (i.e. yielding the None) is formally ensuring that the other iters are closed, or just setting up the garbage collection to be more timely.\n\nWhen the test calls resp.app_iter.close() there is no path for that close() to propagate to the nested iters.\n\nI previously made a start towards a generic ClosingIterator class that would ensure the close is always propagated, over here: https://review.opendev.org/c/openstack/swift/+/899197/2/swift/common/utils/__init__.py#3691\n\nApplying that idea in this context might look like https://review.opendev.org/c/openstack/swift/+/905011","commit_id":"78a40666153e641bbf613998841ec3f7d7d0c78c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"da33fd8ea7f607de049744db10297ef9ad37cb7f","unresolved":false,"context_lines":[{"line_number":4448,"context_line":"        def string_along(useful_iter, useless_iter_iter, logger):"},{"line_number":4449,"context_line":"            try:"},{"line_number":4450,"context_line":"                with closing_if_possible(useful_iter):"},{"line_number":4451,"context_line":"                    yield None  # sync point"},{"line_number":4452,"context_line":"                    for x in useful_iter:"},{"line_number":4453,"context_line":"                        yield x"},{"line_number":4454,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"19811a0f_ec4c419d","line":4451,"in_reply_to":"5284e23b_6704125d","updated":"2024-01-25 17:50:29.000000000","message":"Done","commit_id":"78a40666153e641bbf613998841ec3f7d7d0c78c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d563909be0d481ce42e8ab001e66bd9685153679","unresolved":true,"context_lines":[{"line_number":4460,"context_line":"                    logger.warning("},{"line_number":4461,"context_line":"                        \"More than one part in a single-part response?\")"},{"line_number":4462,"context_line":"            finally:"},{"line_number":4463,"context_line":"                close_if_possible(useless_iter_iter)"},{"line_number":4464,"context_line":""},{"line_number":4465,"context_line":"        result \u003d string_along(response_body_iter, ranges_iter, logger)"},{"line_number":4466,"context_line":"        next(result)  # get to sync point, above"}],"source_content_type":"text/x-python","patch_set":1,"id":"f05ebc10_036045fd","line":4463,"updated":"2024-01-04 05:40:18.000000000","message":"Interestingly, if I skip this and just add the sync point, the test passes on py311 and py312, but starts failing for py310 (and presumably, earlier?)","commit_id":"78a40666153e641bbf613998841ec3f7d7d0c78c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"466c9241cd60a4f5a5c6daa13c275491bff17372","unresolved":false,"context_lines":[{"line_number":4460,"context_line":"                    logger.warning("},{"line_number":4461,"context_line":"                        \"More than one part in a single-part response?\")"},{"line_number":4462,"context_line":"            finally:"},{"line_number":4463,"context_line":"                close_if_possible(useless_iter_iter)"},{"line_number":4464,"context_line":""},{"line_number":4465,"context_line":"        result \u003d string_along(response_body_iter, ranges_iter, logger)"},{"line_number":4466,"context_line":"        next(result)  # get to sync point, above"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ccc7318_9d63c4d4","line":4463,"in_reply_to":"2138cebd_965837f4","updated":"2024-01-25 20:20:54.000000000","message":"Mooted.","commit_id":"78a40666153e641bbf613998841ec3f7d7d0c78c"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b9547fd90b8419121a8da012ebbfc901d0fda509","unresolved":true,"context_lines":[{"line_number":4460,"context_line":"                    logger.warning("},{"line_number":4461,"context_line":"                        \"More than one part in a single-part response?\")"},{"line_number":4462,"context_line":"            finally:"},{"line_number":4463,"context_line":"                close_if_possible(useless_iter_iter)"},{"line_number":4464,"context_line":""},{"line_number":4465,"context_line":"        result \u003d string_along(response_body_iter, ranges_iter, logger)"},{"line_number":4466,"context_line":"        next(result)  # get to sync point, above"}],"source_content_type":"text/x-python","patch_set":1,"id":"2138cebd_965837f4","line":4463,"in_reply_to":"f05ebc10_036045fd","updated":"2024-01-04 16:34:38.000000000","message":"Oh, but probably better to use `with closing_if_possible` like Al was doing in https://review.opendev.org/c/openstack/swift/+/897343 ...","commit_id":"78a40666153e641bbf613998841ec3f7d7d0c78c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"da33fd8ea7f607de049744db10297ef9ad37cb7f","unresolved":true,"context_lines":[{"line_number":3731,"context_line":"    next \u003d __next__  # py2"},{"line_number":3732,"context_line":""},{"line_number":3733,"context_line":"    def close(self):"},{"line_number":3734,"context_line":"        for wrapped in self.closeables:"},{"line_number":3735,"context_line":"            close_if_possible(wrapped)"},{"line_number":3736,"context_line":""},{"line_number":3737,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"022fe687_699d29df","line":3734,"updated":"2024-01-25 17:50:29.000000000","message":"@Tim, so IIUC you\u0027re proposing guarding this with ``if not already_closed``? - can\u0027t see how that would hurt and it would make assertions less confusing","commit_id":"8f15680d2f68247246b7fc379fba148ba959a650"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"466c9241cd60a4f5a5c6daa13c275491bff17372","unresolved":false,"context_lines":[{"line_number":3731,"context_line":"    next \u003d __next__  # py2"},{"line_number":3732,"context_line":""},{"line_number":3733,"context_line":"    def close(self):"},{"line_number":3734,"context_line":"        for wrapped in self.closeables:"},{"line_number":3735,"context_line":"            close_if_possible(wrapped)"},{"line_number":3736,"context_line":""},{"line_number":3737,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1b3d61dc_a1d961db","line":3734,"in_reply_to":"022fe687_699d29df","updated":"2024-01-25 20:20:54.000000000","message":"Yup.","commit_id":"8f15680d2f68247246b7fc379fba148ba959a650"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"18b8128ae4e13f7f72069f3a90a00c9880c45f17","unresolved":true,"context_lines":[{"line_number":4505,"context_line":"        # We need to make sure ranges_iter does not get garbage-collected"},{"line_number":4506,"context_line":"        # before response_body_iter is exhausted. The reason is that"},{"line_number":4507,"context_line":"        # ranges_iter has a finally block that calls close_swift_conn, and"},{"line_number":4508,"context_line":"        # so if that finally block fires before we read response_body_iter,"},{"line_number":4509,"context_line":"        result \u003d StringAlong(response_body_iter, ranges_iter, logger)"},{"line_number":4510,"context_line":"        return result"},{"line_number":4511,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5da786e6_a66f64b2","line":4508,"updated":"2024-01-26 11:08:11.000000000","message":"we lost a line of comment","commit_id":"08e4312aa391818e93ec55e713b2515eb7bec31f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4657a6bfb8ea0a96dccdad90aed5f200acfceb2d","unresolved":false,"context_lines":[{"line_number":4505,"context_line":"        # We need to make sure ranges_iter does not get garbage-collected"},{"line_number":4506,"context_line":"        # before response_body_iter is exhausted. The reason is that"},{"line_number":4507,"context_line":"        # ranges_iter has a finally block that calls close_swift_conn, and"},{"line_number":4508,"context_line":"        # so if that finally block fires before we read response_body_iter,"},{"line_number":4509,"context_line":"        result \u003d StringAlong(response_body_iter, ranges_iter, logger)"},{"line_number":4510,"context_line":"        return result"},{"line_number":4511,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"c88e408a_506acee9","line":4508,"in_reply_to":"5da786e6_a66f64b2","updated":"2024-02-12 11:39:40.000000000","message":"Done","commit_id":"08e4312aa391818e93ec55e713b2515eb7bec31f"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"2624110d86dfac1383c6260c8b41c1078032a796","unresolved":true,"context_lines":[{"line_number":4467,"context_line":"                pass"},{"line_number":4468,"context_line":"            else:"},{"line_number":4469,"context_line":"                self.logger.warning("},{"line_number":4470,"context_line":"                    \"More than one part in a single-part response?\")"},{"line_number":4471,"context_line":"            finally:"},{"line_number":4472,"context_line":"                raise"},{"line_number":4473,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"eb3ba5b9_2cda8fbc","line":4470,"range":{"start_line":4470,"start_character":20,"end_line":4470,"end_character":67},"updated":"2024-02-09 01:19:51.000000000","message":"If we\u0027re pulling this out of the function below, then this warning probably needs to either be more generic or the ability to pass in the warning we want. Because if we every reuse this inter then this is confusing.","commit_id":"29131a30af85d8ea13f823ea3613c263aca97630"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4657a6bfb8ea0a96dccdad90aed5f200acfceb2d","unresolved":false,"context_lines":[{"line_number":4467,"context_line":"                pass"},{"line_number":4468,"context_line":"            else:"},{"line_number":4469,"context_line":"                self.logger.warning("},{"line_number":4470,"context_line":"                    \"More than one part in a single-part response?\")"},{"line_number":4471,"context_line":"            finally:"},{"line_number":4472,"context_line":"                raise"},{"line_number":4473,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"10ebdcdf_1cc3242a","line":4470,"range":{"start_line":4470,"start_character":20,"end_line":4470,"end_character":67},"in_reply_to":"eb3ba5b9_2cda8fbc","updated":"2024-02-12 11:39:40.000000000","message":"Done","commit_id":"29131a30af85d8ea13f823ea3613c263aca97630"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"dcb4783af3b96023371a3080ba8f59fc6a7a267e","unresolved":true,"context_lines":[{"line_number":4460,"context_line":"    def __init__(self, iterable, other_iter, unexpected_items_func):"},{"line_number":4461,"context_line":"        super(StringAlong, self).__init__(iterable, [other_iter])"},{"line_number":4462,"context_line":"        self.other_iter \u003d other_iter"},{"line_number":4463,"context_line":"        self.unexpected_items_func \u003d unexpected_items_func"},{"line_number":4464,"context_line":""},{"line_number":4465,"context_line":"    def _get_next_item(self):"},{"line_number":4466,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba991387_7f1bfa70","line":4463,"updated":"2024-02-15 01:58:43.000000000","message":"Oh I like how this removes the need for passing in the logger and makes is more general. I mean we may only every use this in the single case, but now it\u0027s at least resuable!","commit_id":"c522f5676e33dd0b9422019818a9106cde1d39f7"}],"test/unit/__init__.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"18b8128ae4e13f7f72069f3a90a00c9880c45f17","unresolved":true,"context_lines":[{"line_number":1562,"context_line":"            self.wrapped(*args, **kwargs),"},{"line_number":1563,"context_line":"            functools.partial(self.log_call, self.instance_count))"},{"line_number":1564,"context_line":""},{"line_number":1565,"context_line":""},{"line_number":1566,"context_line":"def get_node_error_stats(proxy_app, ring_node):"},{"line_number":1567,"context_line":"    node_key \u003d proxy_app.error_limiter.node_key(ring_node)"},{"line_number":1568,"context_line":"    return proxy_app.error_limiter.stats.get(node_key) or {}"}],"source_content_type":"text/x-python","patch_set":5,"id":"fc69b119_7667c736","line":1565,"updated":"2024-01-26 11:08:11.000000000","message":"oops, I copied too much across from the other patch when I cribbed CaptureIterator - the functions below duplicate those in proxy/test_server.py. They *will* eventually be more useful up here in unit/__init__.py, but they aren\u0027t imported from here yet.","commit_id":"08e4312aa391818e93ec55e713b2515eb7bec31f"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"4657a6bfb8ea0a96dccdad90aed5f200acfceb2d","unresolved":false,"context_lines":[{"line_number":1562,"context_line":"            self.wrapped(*args, **kwargs),"},{"line_number":1563,"context_line":"            functools.partial(self.log_call, self.instance_count))"},{"line_number":1564,"context_line":""},{"line_number":1565,"context_line":""},{"line_number":1566,"context_line":"def get_node_error_stats(proxy_app, ring_node):"},{"line_number":1567,"context_line":"    node_key \u003d proxy_app.error_limiter.node_key(ring_node)"},{"line_number":1568,"context_line":"    return proxy_app.error_limiter.stats.get(node_key) or {}"}],"source_content_type":"text/x-python","patch_set":5,"id":"8eca145c_117cc85f","line":1565,"in_reply_to":"fc69b119_7667c736","updated":"2024-02-12 11:39:40.000000000","message":"Done","commit_id":"08e4312aa391818e93ec55e713b2515eb7bec31f"}],"test/unit/common/test_utils.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"5d870c8969d7ed6138e111eb6a32e631d33e27ec","unresolved":true,"context_lines":[{"line_number":9635,"context_line":"        it \u003d utils.ClosingIterator(utils.ClosingIterator(wrapped))"},{"line_number":9636,"context_line":"        actual \u003d [x for x in it]"},{"line_number":9637,"context_line":"        self.assertEqual([1, 2, 3], actual)"},{"line_number":9638,"context_line":"        self.assertEqual(2, wrapped.close_call_count)"},{"line_number":9639,"context_line":"        it.close()"},{"line_number":9640,"context_line":"        self.assertEqual(3, wrapped.close_call_count)"},{"line_number":9641,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"a884f516_9cc9e82b","line":9638,"updated":"2024-01-24 20:44:17.000000000","message":"So this one is a bit of a funny wart in removing the `close_on_exit` option from Al\u0027s patch and always having it be `True`. Maybe it\u0027d be reasonable to make `ClosingIterator.close` edge-triggered, so both here and below have `wrapped.close_call_count \u003d\u003d 1`?","commit_id":"336fe574ead2fbfd34678fe44ed71918d5ab6db6"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"466c9241cd60a4f5a5c6daa13c275491bff17372","unresolved":false,"context_lines":[{"line_number":9635,"context_line":"        it \u003d utils.ClosingIterator(utils.ClosingIterator(wrapped))"},{"line_number":9636,"context_line":"        actual \u003d [x for x in it]"},{"line_number":9637,"context_line":"        self.assertEqual([1, 2, 3], actual)"},{"line_number":9638,"context_line":"        self.assertEqual(2, wrapped.close_call_count)"},{"line_number":9639,"context_line":"        it.close()"},{"line_number":9640,"context_line":"        self.assertEqual(3, wrapped.close_call_count)"},{"line_number":9641,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"381de3f7_c7cb3a04","line":9638,"in_reply_to":"a884f516_9cc9e82b","updated":"2024-01-25 20:20:54.000000000","message":"Done","commit_id":"336fe574ead2fbfd34678fe44ed71918d5ab6db6"}],"test/unit/proxy/controllers/test_base.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"da33fd8ea7f607de049744db10297ef9ad37cb7f","unresolved":true,"context_lines":[{"line_number":1709,"context_line":"        with mock.patch.object(handler, \u0027_find_source\u0027,"},{"line_number":1710,"context_line":"                               mock_find_source):"},{"line_number":1711,"context_line":"            resp \u003d handler.get_working_response(req)"},{"line_number":1712,"context_line":"            resp.app_iter.close()"},{"line_number":1713,"context_line":"        self.assertEqual([\"Client disconnected on read of \u0027some-path\u0027\"],"},{"line_number":1714,"context_line":"                         self.logger.get_lines_for_level(\u0027info\u0027))"},{"line_number":1715,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"c9638e61_d3e6c744","line":1712,"updated":"2024-01-25 17:50:29.000000000","message":"@Tim - In another patch (https://review.opendev.org/c/openstack/swift/+/897343/9) I had developed a more explicit test that the iter is existing. I have pulled it out as a follow-up here https://review.opendev.org/c/openstack/swift/+/906747 - could be squashed if you like it?","commit_id":"8f15680d2f68247246b7fc379fba148ba959a650"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"466c9241cd60a4f5a5c6daa13c275491bff17372","unresolved":false,"context_lines":[{"line_number":1709,"context_line":"        with mock.patch.object(handler, \u0027_find_source\u0027,"},{"line_number":1710,"context_line":"                               mock_find_source):"},{"line_number":1711,"context_line":"            resp \u003d handler.get_working_response(req)"},{"line_number":1712,"context_line":"            resp.app_iter.close()"},{"line_number":1713,"context_line":"        self.assertEqual([\"Client disconnected on read of \u0027some-path\u0027\"],"},{"line_number":1714,"context_line":"                         self.logger.get_lines_for_level(\u0027info\u0027))"},{"line_number":1715,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"44c9fd49_50f23ff8","line":1712,"in_reply_to":"c9638e61_d3e6c744","updated":"2024-01-25 20:20:54.000000000","message":"Done","commit_id":"8f15680d2f68247246b7fc379fba148ba959a650"}]}