)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c3122e5d494bf50a2517f06b1ab27ad28b3da3f4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a6749db9_4989df86","updated":"2023-10-10 00:33:43.000000000","message":"these mostly new tests are heavily inspired by Al\u0027s ground work - i\u0027ll update the commit (althought he may not want to claim all of them after I\u0027ve torn them up and changed them).\n\nI think I need to do more work to enumerate additional interesting scenarios that are not currently covered by tests - and try to adopt more consistent patterns accross tests/setup that are pre-existing or duplicated.\n\nMy goal is to merge as many as possible of these (all that I can get passing) into the test refactor on the current implementation.  If any new tests expose previously unknown bad behaviors I\u0027ll probably merge the test with an \"XXX existing bad\" that should get fixed in the refactor (we can file bugs if any seem like something someone would have noticed).","commit_id":"6cd041df0ce0e27589573f3b8828336b233d1882"}],"swift/common/middleware/slo.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"fca15c349a429ba8b0ec806ff352d11d09cbf726","unresolved":true,"context_lines":[{"line_number":975,"context_line":"        # range headers that turned out to be a real object, but if the client"},{"line_number":976,"context_line":"        # included a Range header we can trust swob to do the right conversion"},{"line_number":977,"context_line":"        # back into a 206"},{"line_number":978,"context_line":"        conditional_response \u003d is_success(self._response_status_int)"},{"line_number":979,"context_line":"        resp \u003d Response("},{"line_number":980,"context_line":"            status\u003dself._response_status,"},{"line_number":981,"context_line":"            headers\u003dself._response_headers,"}],"source_content_type":"text/x-python","patch_set":1,"id":"250f94a6_305c56e6","line":978,"updated":"2023-10-05 16:43:36.000000000","message":"I notice that the object server also sets conditional_response\u003dTrue for 404\u0027s, but that it doesn\u0027t resolve the etag-is-at header when doing so. Maybe that\u0027s a bug? Anyway, IDK if we should broaden this condition to include 404s.","commit_id":"725bac53d09d886044f10a3e5fa8e8102a9c58a4"}],"test/unit/common/middleware/test_slo.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"fca15c349a429ba8b0ec806ff352d11d09cbf726","unresolved":true,"context_lines":[{"line_number":4800,"context_line":"        self.assertNotIn(\u0027X-Backend-Etag-Is-At\u0027, self.app.headers[0])"},{"line_number":4801,"context_line":"        self.assertNotIn(\u0027X-Backend-Etag-Is-At\u0027, self.app.headers[1])"},{"line_number":4802,"context_line":""},{"line_number":4803,"context_line":"    def test_if_match_matches_alternate_etag_passthrough(self):"},{"line_number":4804,"context_line":"        self.app.register("},{"line_number":4805,"context_line":"            \u0027GET\u0027, \u0027/v1/AUTH_test/c/my-manifest?multipart-manifest\u003dget\u0027,"},{"line_number":4806,"context_line":"            swob.HTTPOk,"}],"source_content_type":"text/x-python","patch_set":1,"id":"e8c9a226_5bd21bd5","line":4803,"updated":"2023-10-05 16:43:36.000000000","message":"the first few new tests pass without the slo.py change but AFAICT are new coverage","commit_id":"725bac53d09d886044f10a3e5fa8e8102a9c58a4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c3122e5d494bf50a2517f06b1ab27ad28b3da3f4","unresolved":true,"context_lines":[{"line_number":4875,"context_line":"        self.assertEqual(status, \u0027412 Precondition Failed\u0027)"},{"line_number":4876,"context_line":"        # note: the body was read"},{"line_number":4877,"context_line":""},{"line_number":4878,"context_line":"    def test_if_match_matches_alternate_etag_non_slo_after_refetch(self):"},{"line_number":4879,"context_line":"        self.app.register("},{"line_number":4880,"context_line":"            \u0027GET\u0027, \u0027/v1/AUTH_test/c/my-manifest\u0027,"},{"line_number":4881,"context_line":"            swob.HTTPOk,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1af529bd_4d5411d4","line":4878,"updated":"2023-10-10 00:33:43.000000000","message":"this is the only one that fails with the fix reverted:\n\n```\nFAILED swift/test/unit/common/middleware/test_slo.py::TestSloConditionalGetOldManifest::test_if_match_mismatches_alternate_etag_non_slo_after_refetch - AssertionError: \u0027200 OK\u0027 !\u003d \u0027412 Precondition Failed\u0027\nFAILED swift/test/unit/common/middleware/test_slo.py::TestSloConditionalGetNewManifest::test_if_match_mismatches_alternate_etag_non_slo_after_refetch - AssertionError: \u0027200 OK\u0027 !\u003d \u0027412 Precondition Failed\u0027\n```\n\nnice!","commit_id":"725bac53d09d886044f10a3e5fa8e8102a9c58a4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c3122e5d494bf50a2517f06b1ab27ad28b3da3f4","unresolved":true,"context_lines":[{"line_number":4882,"context_line":"            {\u0027Etag\u0027: \u0027manifest-etag\u0027,"},{"line_number":4883,"context_line":"             \u0027X-Static-Large-Object\u0027: \u0027true\u0027,"},{"line_number":4884,"context_line":"             \u0027X-Object-Sysmeta-Alt-Etag\u0027: \u0027alt-manifest-etag\u0027},"},{"line_number":4885,"context_line":"            body\u003db\u0027[]\u0027"},{"line_number":4886,"context_line":"        )"},{"line_number":4887,"context_line":""},{"line_number":4888,"context_line":"        orig_refetch \u003d slo.SloGetContext._need_to_refetch_manifest"}],"source_content_type":"text/x-python","patch_set":1,"id":"c824a545_94b5a0f2","line":4885,"updated":"2023-10-10 00:33:43.000000000","message":"so this test case is actually in TestSloConditionalGetOldManifest, but because it doesn\u0027t use the maybe_add_modern_manifest_headers when it sets up the manifest response we\u0027re actually testing the exact same thing again in TestSloConditionalGetNewManifest\n\nwhich isn\u0027t terrible - it still fails, but someone might be confused why a \"new\" manifest doesn\u0027t have size/etag metadata and gets refected until they look into the setup.  We could do this, but we should probably skip if self.modern_metadata to help future authors focus on the right thing.","commit_id":"725bac53d09d886044f10a3e5fa8e8102a9c58a4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c3122e5d494bf50a2517f06b1ab27ad28b3da3f4","unresolved":true,"context_lines":[{"line_number":4898,"context_line":"                {\u0027Etag\u0027: \u0027object-etag\u0027,"},{"line_number":4899,"context_line":"                 \u0027X-Object-Sysmeta-Alt-Etag\u0027: \u0027alt-object-etag\u0027},"},{"line_number":4900,"context_line":"                body\u003db\u0027banana\u0027"},{"line_number":4901,"context_line":"            )"},{"line_number":4902,"context_line":"            # first response gets a 412 so we\u0027ll refetch"},{"line_number":4903,"context_line":"            refetch \u003d orig_refetch(*args, **kwargs)"},{"line_number":4904,"context_line":"            refetch_calls.append(refetch)"}],"source_content_type":"text/x-python","patch_set":1,"id":"4ab8746f_08de144d","line":4901,"updated":"2023-10-10 00:33:43.000000000","message":"I think we just want to use self.app.register_responses(method, path, [responses]) instead of this mock/swap","commit_id":"725bac53d09d886044f10a3e5fa8e8102a9c58a4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"fca15c349a429ba8b0ec806ff352d11d09cbf726","unresolved":true,"context_lines":[{"line_number":4918,"context_line":"        self.assertIn((\u0027Etag\u0027, \u0027object-etag\u0027), headers)"},{"line_number":4919,"context_line":"        self.assertEqual([True], refetch_calls)"},{"line_number":4920,"context_line":""},{"line_number":4921,"context_line":"    def test_if_match_mismatches_alternate_etag_non_slo_after_refetch(self):"},{"line_number":4922,"context_line":"        self.app.register("},{"line_number":4923,"context_line":"            \u0027GET\u0027, \u0027/v1/AUTH_test/c/my-manifest\u0027,"},{"line_number":4924,"context_line":"            swob.HTTPOk,"}],"source_content_type":"text/x-python","patch_set":1,"id":"15410736_171dab6a","line":4921,"updated":"2023-10-05 16:43:36.000000000","message":"IIRC this test fails without the change in slo.py","commit_id":"725bac53d09d886044f10a3e5fa8e8102a9c58a4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"fca15c349a429ba8b0ec806ff352d11d09cbf726","unresolved":true,"context_lines":[{"line_number":4945,"context_line":"            # first response gets a 412 so we\u0027ll refetch"},{"line_number":4946,"context_line":"            refetch \u003d orig_refetch(*args, **kwargs)"},{"line_number":4947,"context_line":"            refetch_calls.append(refetch)"},{"line_number":4948,"context_line":"            return refetch"},{"line_number":4949,"context_line":""},{"line_number":4950,"context_line":"        req \u003d Request.blank("},{"line_number":4951,"context_line":"            \u0027/v1/AUTH_test/c/my-manifest\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"47c48480_5753afda","line":4948,"updated":"2023-10-05 16:43:36.000000000","message":"maybe make this a class method/closure type thing to avoid duplication?","commit_id":"725bac53d09d886044f10a3e5fa8e8102a9c58a4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c3122e5d494bf50a2517f06b1ab27ad28b3da3f4","unresolved":true,"context_lines":[{"line_number":4962,"context_line":"        # the refetched body was not read"},{"line_number":4963,"context_line":"        self.expected_unread_requests["},{"line_number":4964,"context_line":"            (\u0027GET\u0027, \u0027/v1/AUTH_test/c/my-manifest\u0027)] \u003d 1"},{"line_number":4965,"context_line":"        self.assertEqual([1], refetch_calls)"},{"line_number":4966,"context_line":""},{"line_number":4967,"context_line":"    def test_range_resume_download(self):"},{"line_number":4968,"context_line":"        req \u003d Request.blank("}],"source_content_type":"text/x-python","patch_set":1,"id":"f2d87b2f_1a964e86","line":4965,"updated":"2023-10-10 00:33:43.000000000","message":"holy crap look at all these new tests!?  This is *exactly* what the refactor needed - i\u0027m going to squash this and move myself to co-author.","commit_id":"725bac53d09d886044f10a3e5fa8e8102a9c58a4"}]}