)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"792006a19152e8a9bf61c1606c47d28746643ff3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"36288c41_71a1e0c6","updated":"2023-09-19 22:48:54.000000000","message":"More tests are good, and they pass -- but I think we\u0027re turning up some bugs that could use fixing -- which is just as well since some of the assertions don\u0027t entirely make sense to me.","commit_id":"cb4ac71d8993f1b4e09061bfbcf59e03ea6d0f64"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"746ff3cb0ff0a3775d09f894e9f8a06b35d1d858","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8af8fafd_b357b1db","updated":"2023-09-21 15:32:21.000000000","message":"I\u0027m still a little nervous about that behavior in `test_GET_with_multirange_slow_body_unable_to_resume`, but as long as we\u0027re clear that the test is only meant to be descriptive (not normative), I think I\u0027m good with it.","commit_id":"f8c94d6bbcf1ea13a3ef43e1e57380780a24d2a4"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7e21ff67e9a6441412f04f6d35fc310f722d1bb5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"6433e23b_0bbdcb4f","updated":"2023-09-21 13:25:57.000000000","message":"added some comment and another test scenario (timeout after multipart *body* has started)","commit_id":"f8c94d6bbcf1ea13a3ef43e1e57380780a24d2a4"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"870d5a7453e813e710fb43829bfa5dc66960cc29","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e57ba889_4d007b3b","updated":"2023-09-27 16:39:51.000000000","message":"sounds like the weird multi-part mime error handling can be improved in a follow-up","commit_id":"f8c94d6bbcf1ea13a3ef43e1e57380780a24d2a4"}],"test/unit/proxy/controllers/test_obj.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"792006a19152e8a9bf61c1606c47d28746643ff3","unresolved":true,"context_lines":[{"line_number":1654,"context_line":"        responses \u003d ["},{"line_number":1655,"context_line":"            StubResponse(206, resp_body, headers, slowdown\u003d0.1,"},{"line_number":1656,"context_line":"                         slowdown_after\u003dslowdown_after),"},{"line_number":1657,"context_line":"            StubResponse(206, resp_body, headers)"},{"line_number":1658,"context_line":"        ]"},{"line_number":1659,"context_line":"        req_range_hdrs \u003d []"},{"line_number":1660,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"a51c80c0_a952ea43","line":1657,"range":{"start_line":1657,"start_character":30,"end_line":1657,"end_character":39},"updated":"2023-09-19 22:48:54.000000000","message":"So what happens when the resuming body has a different boundary? Which one gets used in the client response?\n\nOh; neither: `swift.common.exceptions.MimeInvalid: invalid starting boundary: wanted b\u0027--81eb9c110b32ced5fe\u0027, got b\u0027--ef5dec23b011c9be18\\r\\n\u0027\n`","commit_id":"cb4ac71d8993f1b4e09061bfbcf59e03ea6d0f64"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7e21ff67e9a6441412f04f6d35fc310f722d1bb5","unresolved":true,"context_lines":[{"line_number":1654,"context_line":"        responses \u003d ["},{"line_number":1655,"context_line":"            StubResponse(206, resp_body, headers, slowdown\u003d0.1,"},{"line_number":1656,"context_line":"                         slowdown_after\u003dslowdown_after),"},{"line_number":1657,"context_line":"            StubResponse(206, resp_body, headers)"},{"line_number":1658,"context_line":"        ]"},{"line_number":1659,"context_line":"        req_range_hdrs \u003d []"},{"line_number":1660,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"f10dc568_176f5c5f","line":1657,"range":{"start_line":1657,"start_character":30,"end_line":1657,"end_character":39},"in_reply_to":"a51c80c0_a952ea43","updated":"2023-09-21 13:25:57.000000000","message":"the boundary in the first backend response is used in the client response\n\nsee GetOrHeadHandler._make_app_iter","commit_id":"cb4ac71d8993f1b4e09061bfbcf59e03ea6d0f64"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"746ff3cb0ff0a3775d09f894e9f8a06b35d1d858","unresolved":false,"context_lines":[{"line_number":1654,"context_line":"        responses \u003d ["},{"line_number":1655,"context_line":"            StubResponse(206, resp_body, headers, slowdown\u003d0.1,"},{"line_number":1656,"context_line":"                         slowdown_after\u003dslowdown_after),"},{"line_number":1657,"context_line":"            StubResponse(206, resp_body, headers)"},{"line_number":1658,"context_line":"        ]"},{"line_number":1659,"context_line":"        req_range_hdrs \u003d []"},{"line_number":1660,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"4e401ec7_5c96e8eb","line":1657,"range":{"start_line":1657,"start_character":30,"end_line":1657,"end_character":39},"in_reply_to":"f10dc568_176f5c5f","updated":"2023-09-21 15:32:21.000000000","message":"Oh, right! I tried to re-use the headers when I was poking at it, and that\u0027s why I got the error I did...","commit_id":"cb4ac71d8993f1b4e09061bfbcf59e03ea6d0f64"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"792006a19152e8a9bf61c1606c47d28746643ff3","unresolved":true,"context_lines":[{"line_number":1670,"context_line":"            actual_body \u003d resp.body"},{"line_number":1671,"context_line":""},{"line_number":1672,"context_line":"        # read resumes on second source before any object bytes were yielded..."},{"line_number":1673,"context_line":"        self.assertEqual([\u0027bytes\u003d0-49,100-104\u0027] * 2, req_range_hdrs)"},{"line_number":1674,"context_line":"        self.assertEqual(resp.status_int, 206)"},{"line_number":1675,"context_line":"        self.assertEqual(2, len(log))"},{"line_number":1676,"context_line":"        self.assertEqual(resp_body, actual_body)"}],"source_content_type":"text/x-python","patch_set":2,"id":"4ddc115e_341c7f42","line":1673,"updated":"2023-09-19 22:48:54.000000000","message":"This is a little surprising -- it\u0027s not clear to me where we\u0027re dropping bytes on the floor in `test_GET_with_multirange_slow_body_resumes_after_read_started`.","commit_id":"cb4ac71d8993f1b4e09061bfbcf59e03ea6d0f64"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"746ff3cb0ff0a3775d09f894e9f8a06b35d1d858","unresolved":false,"context_lines":[{"line_number":1670,"context_line":"            actual_body \u003d resp.body"},{"line_number":1671,"context_line":""},{"line_number":1672,"context_line":"        # read resumes on second source before any object bytes were yielded..."},{"line_number":1673,"context_line":"        self.assertEqual([\u0027bytes\u003d0-49,100-104\u0027] * 2, req_range_hdrs)"},{"line_number":1674,"context_line":"        self.assertEqual(resp.status_int, 206)"},{"line_number":1675,"context_line":"        self.assertEqual(2, len(log))"},{"line_number":1676,"context_line":"        self.assertEqual(resp_body, actual_body)"}],"source_content_type":"text/x-python","patch_set":2,"id":"e715bb2b_a5d5ffce","line":1673,"in_reply_to":"0e584999_a41c1c18","updated":"2023-09-21 15:32:21.000000000","message":"Got it -- that preamble was longer than I expected; even when I bumped it up to 70 (thinking, \"Surely *this* will get me somewhere in the body, but not get all the way through that first part!\") I only actually added `b\u0027ication/octet-stream\\r\\nContent-\u0027`","commit_id":"cb4ac71d8993f1b4e09061bfbcf59e03ea6d0f64"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7e21ff67e9a6441412f04f6d35fc310f722d1bb5","unresolved":true,"context_lines":[{"line_number":1670,"context_line":"            actual_body \u003d resp.body"},{"line_number":1671,"context_line":""},{"line_number":1672,"context_line":"        # read resumes on second source before any object bytes were yielded..."},{"line_number":1673,"context_line":"        self.assertEqual([\u0027bytes\u003d0-49,100-104\u0027] * 2, req_range_hdrs)"},{"line_number":1674,"context_line":"        self.assertEqual(resp.status_int, 206)"},{"line_number":1675,"context_line":"        self.assertEqual(2, len(log))"},{"line_number":1676,"context_line":"        self.assertEqual(resp_body, actual_body)"}],"source_content_type":"text/x-python","patch_set":2,"id":"0e584999_a41c1c18","line":1673,"in_reply_to":"4ddc115e_341c7f42","updated":"2023-09-21 13:25:57.000000000","message":"only 40 bytes were read from the backend which doesn\u0027t get past the per-part boundary/preamble, so IIUC the ranges don\u0027t get adjusted: we still need the full ranges from the second backend.","commit_id":"cb4ac71d8993f1b4e09061bfbcf59e03ea6d0f64"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"792006a19152e8a9bf61c1606c47d28746643ff3","unresolved":true,"context_lines":[{"line_number":1727,"context_line":"            self.assertEqual(resp.status_int, 206)"},{"line_number":1728,"context_line":"            actual_body \u003d resp.body"},{"line_number":1729,"context_line":""},{"line_number":1730,"context_line":"        self.assertEqual(resp.status_int, 206)"},{"line_number":1731,"context_line":"        self.assertEqual(6, len(log))"},{"line_number":1732,"context_line":"        resp_boundary \u003d resp.headers[\u0027content-type\u0027].rsplit(\u0027\u003d\u0027, 1)[1].encode()"},{"line_number":1733,"context_line":"        self.assertEqual(b\u0027--%s--\u0027 % resp_boundary, actual_body)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5adaab71_696ac714","line":1730,"updated":"2023-09-19 22:48:54.000000000","message":"I\u0027m kinda surprised this doesn\u0027t 503 -- but at least the `Content-Length` indicates that we didn\u0027t get what all we were expecting (right?)","commit_id":"cb4ac71d8993f1b4e09061bfbcf59e03ea6d0f64"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"746ff3cb0ff0a3775d09f894e9f8a06b35d1d858","unresolved":true,"context_lines":[{"line_number":1727,"context_line":"            self.assertEqual(resp.status_int, 206)"},{"line_number":1728,"context_line":"            actual_body \u003d resp.body"},{"line_number":1729,"context_line":""},{"line_number":1730,"context_line":"        self.assertEqual(resp.status_int, 206)"},{"line_number":1731,"context_line":"        self.assertEqual(6, len(log))"},{"line_number":1732,"context_line":"        resp_boundary \u003d resp.headers[\u0027content-type\u0027].rsplit(\u0027\u003d\u0027, 1)[1].encode()"},{"line_number":1733,"context_line":"        self.assertEqual(b\u0027--%s--\u0027 % resp_boundary, actual_body)"}],"source_content_type":"text/x-python","patch_set":2,"id":"6b26e17d_db171aa6","line":1730,"in_reply_to":"373e8928_89dbaab4","updated":"2023-09-21 15:32:21.000000000","message":"Nope! Looks like in `document_iters_to_multipart_byteranges`, our `ranges_iter` fails to yield *any* ranges, causing us to *only* [yield the terminator](https://github.com/openstack/swift/blob/2.32.0/swift/common/utils/__init__.py#L4396)! It really looks like we\u0027ve got all the context we need to be able to respond with a 503 and not emit an invalid multipart MIME doc...","commit_id":"cb4ac71d8993f1b4e09061bfbcf59e03ea6d0f64"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"7e21ff67e9a6441412f04f6d35fc310f722d1bb5","unresolved":true,"context_lines":[{"line_number":1727,"context_line":"            self.assertEqual(resp.status_int, 206)"},{"line_number":1728,"context_line":"            actual_body \u003d resp.body"},{"line_number":1729,"context_line":""},{"line_number":1730,"context_line":"        self.assertEqual(resp.status_int, 206)"},{"line_number":1731,"context_line":"        self.assertEqual(6, len(log))"},{"line_number":1732,"context_line":"        resp_boundary \u003d resp.headers[\u0027content-type\u0027].rsplit(\u0027\u003d\u0027, 1)[1].encode()"},{"line_number":1733,"context_line":"        self.assertEqual(b\u0027--%s--\u0027 % resp_boundary, actual_body)"}],"source_content_type":"text/x-python","patch_set":2,"id":"373e8928_89dbaab4","line":1730,"in_reply_to":"5adaab71_696ac714","updated":"2023-09-21 13:25:57.000000000","message":"hmmm, IIRC the proxy is yielding the boundary (based on having the backend headers) before trying to read the backend iter?? it\u0027s possible the test is not true to real life behaviour w.r.t. when the slowdown is applied??","commit_id":"cb4ac71d8993f1b4e09061bfbcf59e03ea6d0f64"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"d15db6b202e1a799dee0ecd9be1731c1dc767aab","unresolved":true,"context_lines":[{"line_number":1727,"context_line":"            self.assertEqual(resp.status_int, 206)"},{"line_number":1728,"context_line":"            actual_body \u003d resp.body"},{"line_number":1729,"context_line":""},{"line_number":1730,"context_line":"        self.assertEqual(resp.status_int, 206)"},{"line_number":1731,"context_line":"        self.assertEqual(6, len(log))"},{"line_number":1732,"context_line":"        resp_boundary \u003d resp.headers[\u0027content-type\u0027].rsplit(\u0027\u003d\u0027, 1)[1].encode()"},{"line_number":1733,"context_line":"        self.assertEqual(b\u0027--%s--\u0027 % resp_boundary, actual_body)"}],"source_content_type":"text/x-python","patch_set":2,"id":"6af720b0_9478ac24","line":1730,"in_reply_to":"6b26e17d_db171aa6","updated":"2023-09-22 14:18:16.000000000","message":"Isn\u0027t it too late though? We only try to iterate once the body is read. There are reiterate calls in the path, but they don\u0027t seem to be triggering sufficient IO to have an exception before the response starts.\n\nThis doesn\u0027t help for example:\n```\ndiff --git a/swift/common/utils/__init__.py b/swift/common/utils/__init__.py\nindex 90fd5ecf6..73fab3204 100644\n--- a/swift/common/utils/__init__.py\n+++ b/swift/common/utils/__init__.py\n@@ -4223,6 +4223,7 @@ def document_iters_to_multipart_byteranges(ranges_iter, boundary):\n     divider \u003d b\"--\" + boundary + b\"\\r\\n\"\n     terminator \u003d b\"--\" + boundary + b\"--\"\n\n+    parts \u003d 0\n     for range_spec in ranges_iter:\n         start_byte \u003d range_spec[\"start_byte\"]\n         end_byte \u003d range_spec[\"end_byte\"]\n@@ -4246,6 +4247,9 @@ def document_iters_to_multipart_byteranges(ranges_iter, boundary):\n         for chunk in part_iter:\n             yield chunk\n         yield b\"\\r\\n\"\n+        parts +\u003d 1\n+    if not parts:\n+        raise swift.common.swob.HTTPInternalServerError()\n     yield terminator\n     ```\n\nInteresting that in the EC case we do return 500 : test.unit.proxy.controllers.test_obj.TestECObjController.test_GET_with_multirange_unable_to_resume - but that\u0027s different because the proxy presumably fails to assemble enough ECFragGetters and hasn\u0027t yet started the response???","commit_id":"cb4ac71d8993f1b4e09061bfbcf59e03ea6d0f64"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"792006a19152e8a9bf61c1606c47d28746643ff3","unresolved":true,"context_lines":[{"line_number":1730,"context_line":"        self.assertEqual(resp.status_int, 206)"},{"line_number":1731,"context_line":"        self.assertEqual(6, len(log))"},{"line_number":1732,"context_line":"        resp_boundary \u003d resp.headers[\u0027content-type\u0027].rsplit(\u0027\u003d\u0027, 1)[1].encode()"},{"line_number":1733,"context_line":"        self.assertEqual(b\u0027--%s--\u0027 % resp_boundary, actual_body)"},{"line_number":1734,"context_line":"        error_lines \u003d self.app.logger.get_lines_for_level(\u0027error\u0027)"},{"line_number":1735,"context_line":"        self.assertEqual(3, len(error_lines))"},{"line_number":1736,"context_line":"        for line in error_lines:"}],"source_content_type":"text/x-python","patch_set":2,"id":"05007c83_331acd24","line":1733,"updated":"2023-09-19 22:48:54.000000000","message":"wat\n\nMy read on https://datatracker.ietf.org/doc/html/rfc1341 is that multipart MIME docs require at least *one* part...","commit_id":"cb4ac71d8993f1b4e09061bfbcf59e03ea6d0f64"}]}
