)]}'
{"/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":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"}],"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"}]}
