)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"37c34f0d9c5014c2a2f93643925243791a13680f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7094934b_d0aa3b1a","updated":"2023-12-18 17:12:12.000000000","message":"i\u0027m not sure slo was much better prefactor; but i don\u0027t understand yet how this is related to part-num exactly (beyond it makes it easier to trigger the bug with path manipulation because it always needs to refetch manifest)","commit_id":"0f6c2a9aea9c457be9384288c45d1a4f4d21dfda"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6316eaac73095dcd4d43debb96de70d6e4815828","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c14b0f3e_4aedafd2","updated":"2023-12-19 01:35:34.000000000","message":"FWIW I got some *unittests* to hit the HEAD?part-number\u0026version-id error (via s3api) over in:\n\nhttps://review.opendev.org/c/openstack/swift/+/903847/3/test/unit/common/middleware/s3api/test_multi_get.py#235\n\nI think we can squash this if we can figure out what the func test failure is about","commit_id":"488e646a20a1d3082755564c6245b0b1841c2b17"}],"swift/common/middleware/slo.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"37c34f0d9c5014c2a2f93643925243791a13680f","unresolved":true,"context_lines":[{"line_number":1181,"context_line":""},{"line_number":1182,"context_line":"            if self._is_manifest_and_need_to_refetch("},{"line_number":1183,"context_line":"                    req, resp_attrs, part_num):"},{"line_number":1184,"context_line":"                req.path_info \u003d orig_path_info"},{"line_number":1185,"context_line":"                resp_attrs, resp_iter \u003d self._refetch_manifest("},{"line_number":1186,"context_line":"                    req, resp_iter, resp_attrs)"},{"line_number":1187,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"e63c7dff_3a63a84b","line":1184,"updated":"2023-12-18 17:12:12.000000000","message":"so this fix makes it look like ranged-get on versioned legacy manifest was already broken on master\n\nFWIW I don\u0027t the timeline would allow you get a legacy manifest into a versioned container; maybe using COPY.","commit_id":"0f6c2a9aea9c457be9384288c45d1a4f4d21dfda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f68550891d196531cb6f1cb0005bca4022ae0411","unresolved":false,"context_lines":[{"line_number":1181,"context_line":""},{"line_number":1182,"context_line":"            if self._is_manifest_and_need_to_refetch("},{"line_number":1183,"context_line":"                    req, resp_attrs, part_num):"},{"line_number":1184,"context_line":"                req.path_info \u003d orig_path_info"},{"line_number":1185,"context_line":"                resp_attrs, resp_iter \u003d self._refetch_manifest("},{"line_number":1186,"context_line":"                    req, resp_iter, resp_attrs)"},{"line_number":1187,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"e4b359fe_1a1393b9","line":1184,"in_reply_to":"e63c7dff_3a63a84b","updated":"2023-12-19 16:17:46.000000000","message":"Acknowledged","commit_id":"0f6c2a9aea9c457be9384288c45d1a4f4d21dfda"}],"test/functional/test_object_versioning.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"6316eaac73095dcd4d43debb96de70d6e4815828","unresolved":true,"context_lines":[{"line_number":2575,"context_line":"        self.assertEqual(version_id_2, resp_headers.get(\u0027X-Object-Version-Id\u0027))"},{"line_number":2576,"context_line":"        self.assertEqual(\u00271\u0027, resp_headers.get(\u0027X-Parts-Count\u0027))"},{"line_number":2577,"context_line":"        self.assertEqual(\u0027bytes */%s\u0027 % sizes[0],"},{"line_number":2578,"context_line":"                         resp_headers.get(\u0027Content-Range\u0027))"},{"line_number":2579,"context_line":""},{"line_number":2580,"context_line":""},{"line_number":2581,"context_line":"class TestSloWithVersioningUTF8(Base2, TestSloWithVersioning):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3df4a296_5446a486","line":2578,"updated":"2023-12-19 01:35:34.000000000","message":"well spotted tracking down this mpu+versioning functional tests!","commit_id":"488e646a20a1d3082755564c6245b0b1841c2b17"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f68550891d196531cb6f1cb0005bca4022ae0411","unresolved":false,"context_lines":[{"line_number":2575,"context_line":"        self.assertEqual(version_id_2, resp_headers.get(\u0027X-Object-Version-Id\u0027))"},{"line_number":2576,"context_line":"        self.assertEqual(\u00271\u0027, resp_headers.get(\u0027X-Parts-Count\u0027))"},{"line_number":2577,"context_line":"        self.assertEqual(\u0027bytes */%s\u0027 % sizes[0],"},{"line_number":2578,"context_line":"                         resp_headers.get(\u0027Content-Range\u0027))"},{"line_number":2579,"context_line":""},{"line_number":2580,"context_line":""},{"line_number":2581,"context_line":"class TestSloWithVersioningUTF8(Base2, TestSloWithVersioning):"}],"source_content_type":"text/x-python","patch_set":2,"id":"5583aec8_e7f0a000","line":2578,"in_reply_to":"3df4a296_5446a486","updated":"2023-12-19 16:17:46.000000000","message":"Acknowledged","commit_id":"488e646a20a1d3082755564c6245b0b1841c2b17"}],"test/unit/common/middleware/helpers.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e8db3086e9c5323eafd8b261b98912be6e091c22","unresolved":true,"context_lines":[{"line_number":149,"context_line":"        # proper behavior across rolling upgrades, having a FakeSwift not act"},{"line_number":150,"context_line":"        # like modern swift is now opt-in."},{"line_number":151,"context_line":"        self.can_ignore_range \u003d True"},{"line_number":152,"context_line":"        self.call_hook \u003d None"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"    def _find_response(self, method, path):"},{"line_number":155,"context_line":"        path \u003d normalize_path(path)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fac11f9d_abf354f8","line":152,"updated":"2023-12-18 16:30:45.000000000","message":"a comment would be helpful\n\n```\nA call_hook can be installed. For example, middleware tests might want to simulate requests being modified by other middlewares.\n```","commit_id":"0f6c2a9aea9c457be9384288c45d1a4f4d21dfda"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"37c34f0d9c5014c2a2f93643925243791a13680f","unresolved":true,"context_lines":[{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    def __call__(self, env, start_response):"},{"line_number":237,"context_line":"        if self.call_hook:"},{"line_number":238,"context_line":"            self.call_hook(env)"},{"line_number":239,"context_line":"        method \u003d env[\u0027REQUEST_METHOD\u0027]"},{"line_number":240,"context_line":"        if method not in self.ALLOWED_METHODS:"},{"line_number":241,"context_line":"            return HTTPMethodNotAllowed()(env, start_response)"}],"source_content_type":"text/x-python","patch_set":1,"id":"41a2f7dd_286d9660","line":238,"updated":"2023-12-18 17:12:12.000000000","message":"I feel like this is sort of already what middleware is for","commit_id":"0f6c2a9aea9c457be9384288c45d1a4f4d21dfda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f68550891d196531cb6f1cb0005bca4022ae0411","unresolved":false,"context_lines":[{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    def __call__(self, env, start_response):"},{"line_number":237,"context_line":"        if self.call_hook:"},{"line_number":238,"context_line":"            self.call_hook(env)"},{"line_number":239,"context_line":"        method \u003d env[\u0027REQUEST_METHOD\u0027]"},{"line_number":240,"context_line":"        if method not in self.ALLOWED_METHODS:"},{"line_number":241,"context_line":"            return HTTPMethodNotAllowed()(env, start_response)"}],"source_content_type":"text/x-python","patch_set":1,"id":"de8454c5_612b705a","line":238,"in_reply_to":"41a2f7dd_286d9660","updated":"2023-12-19 16:17:46.000000000","message":"yup, agree. IIRC I hacked this up before I realised that we needed to improve how FakeApp is built - so I think the right way is to make it easier to plug in arbitrary middlewares in the test setup.","commit_id":"0f6c2a9aea9c457be9384288c45d1a4f4d21dfda"}],"test/unit/common/middleware/test_helpers.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f68550891d196531cb6f1cb0005bca4022ae0411","unresolved":true,"context_lines":[{"line_number":131,"context_line":"        resp \u003d req.get_response(swift)"},{"line_number":132,"context_line":"        self.assertEqual(404, resp.status_int)"},{"line_number":133,"context_line":"        self.assertEqual([\u0027/v1/a/c/o\u0027], hook_calls)"},{"line_number":134,"context_line":"        self.assertEqual(\u0027/v1/a/c/oblah\u0027, req.path_info)"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def test_GET_registered_with_query_string(self):"},{"line_number":137,"context_line":"        # verify that a single registered GET response is sufficient to handle"}],"source_content_type":"text/x-python","patch_set":1,"id":"ea81e645_86a57b02","line":134,"updated":"2023-12-19 16:17:46.000000000","message":"so long, bad idea.","commit_id":"0f6c2a9aea9c457be9384288c45d1a4f4d21dfda"}],"test/unit/common/middleware/test_slo.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e8db3086e9c5323eafd8b261b98912be6e091c22","unresolved":true,"context_lines":[{"line_number":5718,"context_line":"            environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027HEAD\u0027})"},{"line_number":5719,"context_line":"        hook_calls \u003d []"},{"line_number":5720,"context_line":""},{"line_number":5721,"context_line":"        def pseudo_middleware(env):"},{"line_number":5722,"context_line":"            hook_calls.append((env[\u0027REQUEST_METHOD\u0027], env[\u0027PATH_INFO\u0027]))"},{"line_number":5723,"context_line":"            # pretend another middleware modified the path"},{"line_number":5724,"context_line":"            # note: for convenience, the path \"modification\" actually results"}],"source_content_type":"text/x-python","patch_set":1,"id":"82a3052b_48c11a24","line":5721,"updated":"2023-12-18 16:30:45.000000000","message":"Alternatively, I could have imported object_versioing, constructed a pipeline with an instance of that ... and relied on the fact that it modifies the request. But this is more explicit, and easier.","commit_id":"0f6c2a9aea9c457be9384288c45d1a4f4d21dfda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"e8db3086e9c5323eafd8b261b98912be6e091c22","unresolved":true,"context_lines":[{"line_number":5722,"context_line":"            hook_calls.append((env[\u0027REQUEST_METHOD\u0027], env[\u0027PATH_INFO\u0027]))"},{"line_number":5723,"context_line":"            # pretend another middleware modified the path"},{"line_number":5724,"context_line":"            # note: for convenience, the path \"modification\" actually results"},{"line_number":5725,"context_line":"            # in one of the pre-registered paths"},{"line_number":5726,"context_line":"            env[\u0027PATH_INFO\u0027] +\u003d \u0027fest-bc\u0027"},{"line_number":5727,"context_line":""},{"line_number":5728,"context_line":"        with patch.object(self.app, \u0027call_hook\u0027, pseudo_middleware):"}],"source_content_type":"text/x-python","patch_set":1,"id":"438f35c2_cc1efad2","line":5725,"updated":"2023-12-18 16:30:45.000000000","message":"given more time I guess I could set up manifest-bc with a version suffix","commit_id":"0f6c2a9aea9c457be9384288c45d1a4f4d21dfda"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"37c34f0d9c5014c2a2f93643925243791a13680f","unresolved":true,"context_lines":[{"line_number":5725,"context_line":"            # in one of the pre-registered paths"},{"line_number":5726,"context_line":"            env[\u0027PATH_INFO\u0027] +\u003d \u0027fest-bc\u0027"},{"line_number":5727,"context_line":""},{"line_number":5728,"context_line":"        with patch.object(self.app, \u0027call_hook\u0027, pseudo_middleware):"},{"line_number":5729,"context_line":"            status, headers, body \u003d self.call_slo(req)"},{"line_number":5730,"context_line":""},{"line_number":5731,"context_line":"        # pseudo-middleware gets the original path for the refetch"}],"source_content_type":"text/x-python","patch_set":1,"id":"341814ce_806398a8","line":5728,"updated":"2023-12-18 17:12:12.000000000","message":"so this is where we use FakeSwift\u0027s new \"call_hook\" interface; the patch.object is probably to make sure it\u0027s gets reset (but since self.app isn\u0027t used again this test it seems like you could\n\n    self.app.call_hook \u003d pesudo_mw\n    \n... otherwise, why not just `patch.object(\u0027__call__\u0027)`?\n\nTurns out patching __call__ is quite tricky:\n\nhttps://stackoverflow.com/questions/34261111/patch-call-of-a-function\n\n... but this works:\n\n\tdiff --git a/test/unit/common/middleware/test_slo.py b/test/unit/common/middleware/test_slo.py\n\tindex 9761f1793..70e6f4af5 100644\n\t--- a/test/unit/common/middleware/test_slo.py\n\t+++ b/test/unit/common/middleware/test_slo.py\n\t@@ -5717,16 +5717,18 @@ class TestPartNumber(SloGETorHEADTestCase):\n\t\t     \u0027/v1/AUTH_test/gettest/mani?part-number\u003d1\u0027,\n\t\t     environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027HEAD\u0027})\n\t\t hook_calls \u003d []\n\t+        orig_call \u003d FakeSwift.__call__\n\t \n\t-        def pseudo_middleware(env):\n\t+        def pseudo_middleware(app, env, start_response):\n\t\t     hook_calls.append((env[\u0027REQUEST_METHOD\u0027], env[\u0027PATH_INFO\u0027]))\n\t\t     # pretend another middleware modified the path\n\t\t     # note: for convenience, the path \"modification\" actually results\n\t\t     # in one of the pre-registered paths\n\t\t     env[\u0027PATH_INFO\u0027] +\u003d \u0027fest-bc\u0027\n\t+            return orig_call(app, env, start_response)\n\t \n\t-        self.app.call_hook \u003d pseudo_middleware\n\t-        status, headers, body \u003d self.call_slo(req)\n\t+        with patch.object(FakeSwift, \u0027__call__\u0027, pseudo_middleware):\n\t+            status, headers, body \u003d self.call_slo(req)\n\t \n\t\t # pseudo-middleware gets the original path for the refetch\n\t\t self.assertEqual([(\u0027HEAD\u0027, \u0027/v1/AUTH_test/gettest/mani\u0027),","commit_id":"0f6c2a9aea9c457be9384288c45d1a4f4d21dfda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f68550891d196531cb6f1cb0005bca4022ae0411","unresolved":false,"context_lines":[{"line_number":5725,"context_line":"            # in one of the pre-registered paths"},{"line_number":5726,"context_line":"            env[\u0027PATH_INFO\u0027] +\u003d \u0027fest-bc\u0027"},{"line_number":5727,"context_line":""},{"line_number":5728,"context_line":"        with patch.object(self.app, \u0027call_hook\u0027, pseudo_middleware):"},{"line_number":5729,"context_line":"            status, headers, body \u003d self.call_slo(req)"},{"line_number":5730,"context_line":""},{"line_number":5731,"context_line":"        # pseudo-middleware gets the original path for the refetch"}],"source_content_type":"text/x-python","patch_set":1,"id":"0332ea61_148ed159","line":5728,"in_reply_to":"341814ce_806398a8","updated":"2023-12-19 16:17:46.000000000","message":"thanks for making this better, I think at this point I needed fresh eyes!","commit_id":"0f6c2a9aea9c457be9384288c45d1a4f4d21dfda"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"37c34f0d9c5014c2a2f93643925243791a13680f","unresolved":true,"context_lines":[{"line_number":5736,"context_line":"        expected_calls \u003d ["},{"line_number":5737,"context_line":"            # original path is modified..."},{"line_number":5738,"context_line":"            (\u0027HEAD\u0027, \u0027/v1/AUTH_test/gettest/manifest-bc?part-number\u003d1\u0027),"},{"line_number":5739,"context_line":"            # refetch: the *original* path is modified..."},{"line_number":5740,"context_line":"            (\u0027GET\u0027, \u0027/v1/AUTH_test/gettest/manifest-bc?part-number\u003d1\u0027)"},{"line_number":5741,"context_line":"        ]"},{"line_number":5742,"context_line":"        self.assertEqual(self.app.calls, expected_calls)"}],"source_content_type":"text/x-python","patch_set":1,"id":"fee57692_b136de93","line":5739,"updated":"2023-12-18 17:12:12.000000000","message":"*un*modified?","commit_id":"0f6c2a9aea9c457be9384288c45d1a4f4d21dfda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f68550891d196531cb6f1cb0005bca4022ae0411","unresolved":true,"context_lines":[{"line_number":5736,"context_line":"        expected_calls \u003d ["},{"line_number":5737,"context_line":"            # original path is modified..."},{"line_number":5738,"context_line":"            (\u0027HEAD\u0027, \u0027/v1/AUTH_test/gettest/manifest-bc?part-number\u003d1\u0027),"},{"line_number":5739,"context_line":"            # refetch: the *original* path is modified..."},{"line_number":5740,"context_line":"            (\u0027GET\u0027, \u0027/v1/AUTH_test/gettest/manifest-bc?part-number\u003d1\u0027)"},{"line_number":5741,"context_line":"        ]"},{"line_number":5742,"context_line":"        self.assertEqual(self.app.calls, expected_calls)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ac8bfab2_f754fd2e","line":5739,"in_reply_to":"fee57692_b136de93","updated":"2023-12-19 16:17:46.000000000","message":"no, it is modified - remember the original is truncated and fest-bc is appended. The point is that the *original* is modified, rather than the modified HEAD path being further modified.\n\nThe bug would result in ``/v1/AUTH_test/gettest/manifest-bcfest-bc``\nIt\u0027s a little lame but best I came up with in short time.","commit_id":"0f6c2a9aea9c457be9384288c45d1a4f4d21dfda"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f68550891d196531cb6f1cb0005bca4022ae0411","unresolved":true,"context_lines":[{"line_number":5719,"context_line":"        hook_calls \u003d []"},{"line_number":5720,"context_line":"        orig_call \u003d FakeSwift.__call__"},{"line_number":5721,"context_line":""},{"line_number":5722,"context_line":"        def pseudo_middleware(app, env, start_response):"},{"line_number":5723,"context_line":"            hook_calls.append((env[\u0027REQUEST_METHOD\u0027], env[\u0027PATH_INFO\u0027]))"},{"line_number":5724,"context_line":"            # pretend another middleware modified the path"},{"line_number":5725,"context_line":"            # note: for convenience, the path \"modification\" actually results"}],"source_content_type":"text/x-python","patch_set":2,"id":"6816c17d_26bf49a7","line":5722,"range":{"start_line":5722,"start_character":30,"end_line":5722,"end_character":33},"updated":"2023-12-19 16:17:46.000000000","message":"ok, so the __call__ on the class gets passed the instance.","commit_id":"488e646a20a1d3082755564c6245b0b1841c2b17"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f68550891d196531cb6f1cb0005bca4022ae0411","unresolved":true,"context_lines":[{"line_number":5727,"context_line":"            env[\u0027PATH_INFO\u0027] +\u003d \u0027fest-bc\u0027"},{"line_number":5728,"context_line":"            return orig_call(app, env, start_response)"},{"line_number":5729,"context_line":""},{"line_number":5730,"context_line":"        with patch.object(FakeSwift, \u0027__call__\u0027, pseudo_middleware):"},{"line_number":5731,"context_line":"            status, headers, body \u003d self.call_slo(req)"},{"line_number":5732,"context_line":""},{"line_number":5733,"context_line":"        # pseudo-middleware gets the original path for the refetch"}],"source_content_type":"text/x-python","patch_set":2,"id":"81c93d28_2615e04c","line":5730,"updated":"2023-12-19 16:17:46.000000000","message":"that turned out better :)","commit_id":"488e646a20a1d3082755564c6245b0b1841c2b17"}]}