)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b5e1c1204c3ec076df7bce9fd8de50b4f49333e9","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Ask client to close on RequestTimeout"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"It\u0027s possible on ChunkReadTimeout that the client has simply gotten"},{"line_number":10,"context_line":"confused about the size of the body we\u0027re expecting and thinks it\u0027s"},{"line_number":11,"context_line":"already waiting on a server reply; we handle this failure with a Timeout"},{"line_number":12,"context_line":"- we won\u0027t wait on the client to send the bytes we expect forever."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"However, if the client actually gets this reponse, they may realize they"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"d6e1f953_4b18be92","line":11,"range":{"start_line":9,"start_character":39,"end_line":11,"end_character":33},"updated":"2026-03-24 15:59:43.000000000","message":"I\u0027d love more details about this -- like, no, we shouldn\u0027t allow the HTTP framing issue to *continue* -- but how\u0027d it start in the first place? Where did comms go off the rails?","commit_id":"71c40bb08645c8316af276dc0069ec2d9480fe2d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4185fd684c80df825743fd6a3ad74ae4391cbff4","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Ask client to close on RequestTimeout"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"It\u0027s possible on ChunkReadTimeout that the client has simply gotten"},{"line_number":10,"context_line":"confused about the size of the body we\u0027re expecting and thinks it\u0027s"},{"line_number":11,"context_line":"already waiting on a server reply; we handle this failure with a Timeout"},{"line_number":12,"context_line":"- we won\u0027t wait on the client to send the bytes we expect forever."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"However, if the client actually gets this reponse, they may realize they"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"580584cb_bfa2362a","line":11,"range":{"start_line":9,"start_character":39,"end_line":11,"end_character":33},"in_reply_to":"d6e1f953_4b18be92","updated":"2026-03-24 23:05:56.000000000","message":"Yeah, honestly we don\u0027t know - kind of throwing spagetti at the wall, step 1 was just to try and see if a retry helps (since boto3 already has explicit support to retry 400 w/ RequestTimeout) - but w/o the `Connection: close` header boto3 just re-uses the connection and blows up with a 400 Bad Request on the retry - with `Connection: close` it does the right thing.\n\nI was able to repro the error message we were seeing in prod using a hacked up BodyLike that when passed to boto3 would provide all the bytes for checksumming but when it got the upload it\u0027d just return too few bytes.  boto3 didn\u0027t notice the final short read and would wait on server to 201 the PUT.  w/ Connection: close (and no more sneaky/bad short reads from BodyLike) they retry works a treat!\n\nFWIW I\u0027m not sure our client is having this problem with PUT - it might actually be a POST (like a CompleteMultipartUpload?) - I\u0027m not really sure yet.\n\nBut after adding 400 w/ RequestTimeout to their retry handler the ran into the same 400 Bad Request responses and can\u0027t figure out how to get their net/http to discard the connection w/o the hint from the server.","commit_id":"71c40bb08645c8316af276dc0069ec2d9480fe2d"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b5e1c1204c3ec076df7bce9fd8de50b4f49333e9","unresolved":true,"context_lines":[{"line_number":17,"context_line":"discarded as part of the original request body."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Since we can\u0027t reliably ask our http server to close connections on"},{"line_number":20,"context_line":"RequestTimeout errors we\u0027ll ask the client to do it for us - it\u0027s got a"},{"line_number":21,"context_line":"better shot at working out than not!"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Ib4dfa0930e8a100fc25689d3e571a16c9ec481c6"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"56112883_581a0107","line":20,"range":{"start_line":20,"start_character":22,"end_line":20,"end_character":58},"updated":"2026-03-24 15:59:43.000000000","message":"We may want to call out that this violates https://peps.python.org/pep-3333/ -- `Connection` header-setting is squarely in the WSGI *server*\u0027s hands, not the app\u0027s. Though, y\u0027know, I\u0027ve certainly [done that sort of thing before](https://gist.github.com/tipabu/7c8ed7a713a7f51f20c2).\n\nMakes me wonder whether other WSGI servers might quietly drop the header on us, though...","commit_id":"71c40bb08645c8316af276dc0069ec2d9480fe2d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4185fd684c80df825743fd6a3ad74ae4391cbff4","unresolved":true,"context_lines":[{"line_number":17,"context_line":"discarded as part of the original request body."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Since we can\u0027t reliably ask our http server to close connections on"},{"line_number":20,"context_line":"RequestTimeout errors we\u0027ll ask the client to do it for us - it\u0027s got a"},{"line_number":21,"context_line":"better shot at working out than not!"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Ib4dfa0930e8a100fc25689d3e571a16c9ec481c6"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"a889ead1_429b7f31","line":20,"range":{"start_line":20,"start_character":22,"end_line":20,"end_character":58},"in_reply_to":"56112883_581a0107","updated":"2026-03-24 23:05:56.000000000","message":"\u003e We may want to call out that this violates pep-3333\n\nthat\u0027s a good idea!\n\n\u003e Makes me wonder whether other WSGI servers might quietly drop the header on us, though...\n\nprobably 😭  we\u0027re going to end up stuck on swventlet forever 😄","commit_id":"71c40bb08645c8316af276dc0069ec2d9480fe2d"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4185fd684c80df825743fd6a3ad74ae4391cbff4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c51207f0_fd8adc24","updated":"2026-03-24 23:05:56.000000000","message":"@tburke@nvidia.com - these are great questions and I don\u0027t have the answers, but I do have clients that would be a lot happier if s3api returned `Connection: close` on RequestTimeout - can we just do THAT and figure out the rest as next step?","commit_id":"aa3d4b45a8b7648b1477df3c966829efe22386ea"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"dc5d77cf81e7749731efa3fb2d523c64e303744b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"0ceaa575_1a989bcf","updated":"2026-06-22 20:22:36.000000000","message":"I understand this tactically, but... I still don\u0027t particularly like it. Why only s3api? Why only `RequestTimeout` responses, and not `EntityTooLarge` or `PreconditionFailed` or `MissingContentLength`?\n\nStarting to think *strategically*...\n\n- Are we saying the WSGI spec got the hop-by-hop responsibilities wrong? If so, why *wouldn\u0027t* we do this for Swift, too, and then just make sure s3api preserves the hop-by-hop header?\n- Are we going to do anything to try to make this more than a hint and actually close the connection? I think that\u0027ll need something akin to https://review.opendev.org/c/openstack/swift/+/799866 -- but maybe the `Connection` header could be the flag?\n- So we know we can get the `Connection` header through eventlet and out to the client -- have we looked at what happens with any other WSGI servers? Gunicorn seems most relevant, given `feature/threaded`.","commit_id":"aa3d4b45a8b7648b1477df3c966829efe22386ea"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"e3eaf94721ef6d64a59816d27cd4cd92c4d6be47","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"8c17f6e5_aa18af09","updated":"2026-06-25 09:42:16.000000000","message":"Let\u0027s merge this now - it\u0027s a somewhat isolated fix. We should keep Tim\u0027s questions in mind to improve other responses as well.","commit_id":"aa3d4b45a8b7648b1477df3c966829efe22386ea"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"4c0af73642442b17e0796cb34036356ac21ec4fa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"e88e38f9_43d856ff","updated":"2026-03-25 20:42:19.000000000","message":"The shortfilelike is only used by the 1 test and works for that test, so don\u0027t want it to hold up a good fix. And it\u0027s something can can be refactored in future patches esp when we need to use it again.","commit_id":"aa3d4b45a8b7648b1477df3c966829efe22386ea"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"25c5b09f0e027786574b6b3fdc1f6fec4edf5456","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c89d5fd8_599ce488","updated":"2026-06-10 18:15:12.000000000","message":"```\n• Reviewed Gerrit change 974015 in slot vsaio-1.\n\n  Gerrit: https://review.opendev.org/c/openstack/swift/+/974015\n  Title: s3api: Ask client to close on RequestTimeout\n  Patch set: 4, ref refs/changes/15/974015/4\n  Commit: aa3d4b45a8b7648b1477df3c966829efe22386ea\n  Parent: 22fe124ebf7fa8d87f64352cf47064c5294f6623\n\n  Recommendation: +1\n\n  No blocking findings. The behavior change is narrowly scoped to the S3 object PUT timeout mapping, preserves the existing RequestTimeout S3 error, and adds the operator/client-visible Connection: close signal that the new unit and in-process\n  functional tests prove.\n\n  Non-blocking notes: the commit message has a few typos worth cleaning before merge: reponse, reciving, duplicated try to do, duplicated mixed all mixed up, and some how. Also, instances/vsaio-1/swift/test/functional/s3api/test_object.py:53 uses a\n  “magic” boto3 read-call count; it passed here, but a less version-sensitive helper would be easier to maintain.\n\n  Changed Swift files reviewed:\n  swift/common/middleware/s3api/s3request.py, test/functional/s3api/test_object.py, test/unit/common/middleware/s3api/test_obj.py.\n\n  Validation:\n  poetry run -- vsaio-pool --slot vsaio-1 vagrant ssh -c \u0027cd /vagrant/swift \u0026\u0026 pytest test/unit/common/middleware/s3api/test_obj.py::TestS3ApiObj::test_object_PUT_error test/unit/common/middleware/s3api/\n  test_obj.py::TestS3ApiObj::test_object_PUT_request_timeout_connection_close\u0027 passed, 2 tests.\n\n  poetry run -- vsaio-pool --slot vsaio-1 vagrant ssh -c \u0027cd /vagrant/swift \u0026\u0026 SWIFT_TEST_IN_PROCESS\u003d1 pytest test/functional/s3api/test_object.py::TestS3ApiObjectBoto3::test_put_object_request_timeout\u0027 passed, 1 test.\n```\n\nFWIW our user recently reported they were able to find and fix their client side bug after we introduced this change.  We should respond with `Connection: close` on s3api 400 RequestTimeout errors.","commit_id":"aa3d4b45a8b7648b1477df3c966829efe22386ea"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"e3eaf94721ef6d64a59816d27cd4cd92c4d6be47","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"664db94d_0ae20970","in_reply_to":"0ceaa575_1a989bcf","updated":"2026-06-25 09:42:16.000000000","message":"Good questions, I like the strategic approach.\n\nRegarding the other wsgi servers - gunicorn already has support to enforce this: https://github.com/benoitc/gunicorn/blob/25.3.0/gunicorn/http/wsgi.py#L286-L287\n\nThat said, I think it\u0027s ok to merge this one now (solving the auto-retry issue from botocore), and follow up to improve this in general.","commit_id":"aa3d4b45a8b7648b1477df3c966829efe22386ea"}],"swift/common/middleware/s3api/s3response.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"b5e1c1204c3ec076df7bce9fd8de50b4f49333e9","unresolved":true,"context_lines":[{"line_number":754,"context_line":""},{"line_number":755,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":756,"context_line":"        kwargs.setdefault(\u0027headers\u0027, {}).update({\u0027Connection\u0027: \u0027close\u0027})"},{"line_number":757,"context_line":"        super().__init__(*args, **kwargs)"},{"line_number":758,"context_line":""},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"class RequestTimeTooSkewed(ErrorResponse):"}],"source_content_type":"text/x-python","patch_set":3,"id":"f1be47a4_5dee2ec9","line":757,"updated":"2026-03-24 15:59:43.000000000","message":"Do we really need this if we\u0027re updating swob?\n\nIs it that we\u0027re not passing the app\u0027s response headers on to the S3 response headers? Should we fix *that*? Do we really want to fix it in two places *again* when we do this for 413s?","commit_id":"71c40bb08645c8316af276dc0069ec2d9480fe2d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"4185fd684c80df825743fd6a3ad74ae4391cbff4","unresolved":true,"context_lines":[{"line_number":754,"context_line":""},{"line_number":755,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":756,"context_line":"        kwargs.setdefault(\u0027headers\u0027, {}).update({\u0027Connection\u0027: \u0027close\u0027})"},{"line_number":757,"context_line":"        super().__init__(*args, **kwargs)"},{"line_number":758,"context_line":""},{"line_number":759,"context_line":""},{"line_number":760,"context_line":"class RequestTimeTooSkewed(ErrorResponse):"}],"source_content_type":"text/x-python","patch_set":3,"id":"22d6b711_e73ac7de","line":757,"in_reply_to":"f1be47a4_5dee2ec9","updated":"2026-03-24 23:05:56.000000000","message":"\u003e Do we really need this if we\u0027re updating swob?\n\nIn my testing, yes.\n\n\u003e we\u0027re not passing the app\u0027s response headers on to the S3 response headers?\n\nnot that I could see\n\n\u003e Should we fix that? \n\nidk, I judged that as too much of a drive-by to bite off\n\nFrankly, I would be satisfied with JUST adding `Connection: close` to the s3api 400 RequestTimeout for now - that\u0027s what patchset 1 did - this was my attempt at making a little more \"consistent\"\n\n\u003e Do we really want to fix it in two places again when we do this for 413s?\n\nno, we could try it as a pre-factor?\n\n1) add `connection: close` to s3api 400 RequestTimeout (with a test)\n2) make s3api forward on (all?) swift (4XX?) error response headers\n3) make swift return \u0027Connection: close` on 408, 499 and 413","commit_id":"71c40bb08645c8316af276dc0069ec2d9480fe2d"}],"test/functional/s3api/test_object.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"38599885d1603fa1726ab2cccb3f2d3c66e0b761","unresolved":false,"context_lines":[{"line_number":58,"context_line":"    \"\"\""},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    # Found through trial and error, the magic number for the start of the full"},{"line_number":61,"context_line":"    # read which happens to be the actual upload."},{"line_number":62,"context_line":"    MAGIC_NUMBER \u003d 4"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def __init__(self, size):"}],"source_content_type":"text/x-python","patch_set":4,"id":"83d59ae9_dc9b87ad","line":61,"updated":"2026-03-25 05:20:54.000000000","message":"Seems boto 4 is because:\n  1 - we read the body for pos 0 to generate the crc, then seek back.\n  2 and 3 - Boto reads the body until it gets a b\u0027\u0027 back, which is 2 times so it can calculate the crc, then seeks back\n  4 - boto reads the data and boom only gets the b\u0027bbb\u0027.\n\nSo I wonder if this is really test dependant, and because we go and create a \"ShortFileLike\" directly, I wonder if MAGIC_NUMBER should be passed in the constructor?","commit_id":"aa3d4b45a8b7648b1477df3c966829efe22386ea"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"38599885d1603fa1726ab2cccb3f2d3c66e0b761","unresolved":false,"context_lines":[{"line_number":84,"context_line":"        num_reads \u003d len([c for c in self.calls if c[0] \u003d\u003d \u0027read\u0027])"},{"line_number":85,"context_line":"        if num_reads \u003d\u003d self.MAGIC_NUMBER:"},{"line_number":86,"context_line":"            return b\u0027bbb\u0027"},{"line_number":87,"context_line":"        return buff"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"class TestS3ApiObjectBoto3(S3ApiBaseBoto3):"}],"source_content_type":"text/x-python","patch_set":4,"id":"9b98887f_41f3c92b","line":87,"updated":"2026-03-25 05:20:54.000000000","message":"So this class will all ways return the end of the file depending on where you seek to. So this heavily relies on seek to work. From my debuggin boto is calling read with a buffer size of 1048576, so I guess this works better the other options. Just not as intuitive in my opinion.\n\nBut should we at least only limit what we send back to the size they pass in (if passed in and is smaller then self._size?)\n\n```\nread_size \u003d args[0] if args else 0\n...\nif read_size and read_size \u003c\u003d (self._size - self._pos):\n   buff \u003d b\u0027a\u0027 * read_size\n```\nOr something?","commit_id":"aa3d4b45a8b7648b1477df3c966829efe22386ea"}]}