)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e694e1c24c8dd08dc7ed0462bb035bd7a565bc44","unresolved":true,"context_lines":[{"line_number":18,"context_line":"*deleted* object \"foo\". This would result in a 404 from"},{"line_number":19,"context_line":"object_versioning middleware when it does a HEAD to verify the"},{"line_number":20,"context_line":"existence of the restored version. The client would receive a 404"},{"line_number":21,"context_line":"response when in fact the requested object version has been"},{"line_number":22,"context_line":"successfully deleted."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"This patch fixes the s3api restore-on-delete logic to ignore other"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"81d67752_d8f13458","line":21,"updated":"2026-07-01 00:10:18.000000000","message":"if the HEAD is part of version\u0027d PUT it may be that s3api response translation just translates object-PUT\u003d\u003e404 means \"BucketNotFound\" (since a normal un-versioned swift PUT would normally only fail 404 if the container does not exist)","commit_id":"40fa5395feda255d1474d22267efefa66f986156"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"35bda92ebcb188c1e4ab956c17fde2fd67ebc4e5","unresolved":true,"context_lines":[{"line_number":18,"context_line":"*deleted* object \"foo\". This would result in a 404 from"},{"line_number":19,"context_line":"object_versioning middleware when it does a HEAD to verify the"},{"line_number":20,"context_line":"existence of the restored version. The client would receive a 404"},{"line_number":21,"context_line":"response when in fact the requested object version has been"},{"line_number":22,"context_line":"successfully deleted."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"This patch fixes the s3api restore-on-delete logic to ignore other"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"0384dc99_3ab884c0","line":21,"in_reply_to":"81d67752_d8f13458","updated":"2026-07-01 08:48:56.000000000","message":"right! this is the subject of the follow on patch 995546: s3api: fix erroneous 404 response from version DELETE | https://review.opendev.org/c/openstack/swift/+/995546\n\nThere seems to be some mismatch between the s3api version PUT handling and the object-versioning implementation i.e. s3api restore_on_delete  seems to have been written to gracefully handle a 412 from the PUT?version-id, returning a 204 to the client for the original DELETE. But object-versioning in fact raises a 404 when the PUT?version-id fails to HEAD the new target, which is not caught by restore_on_delete and bubbles up, then the funky error code translation thinks it is a PUT 404 hence NoSuchBucket.","commit_id":"40fa5395feda255d1474d22267efefa66f986156"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":39146,"name":"Nathaniel Martes","display_name":"Nate Martes","email":"nmartes@NVIDIA.com","username":"nmartes"},"change_message_id":"df99ec402b2b3f44a7041429075a0e14801703b0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"571f1112_d6af267b","updated":"2026-06-30 15:52:14.000000000","message":"This test looks good to me, should be good once we sq https://review.opendev.org/c/openstack/swift/+/995512/2","commit_id":"2457f1911665cee519a037ff678543d0dd916bbe"},{"author":{"_account_id":39146,"name":"Nathaniel Martes","display_name":"Nate Martes","email":"nmartes@NVIDIA.com","username":"nmartes"},"change_message_id":"339351b4efa4858f272fa150dba0e5533fc04038","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"2db2d54d_4c8e42b3","updated":"2026-06-30 20:14:33.000000000","message":"LGTM!","commit_id":"40fa5395feda255d1474d22267efefa66f986156"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e694e1c24c8dd08dc7ed0462bb035bd7a565bc44","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"35a377d6_de8e23e9","updated":"2026-07-01 00:10:18.000000000","message":"LGTM!","commit_id":"40fa5395feda255d1474d22267efefa66f986156"}],"swift/common/middleware/s3api/controllers/obj.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"152f29c7a423d8e47273153b24b8349173e2e7e2","unresolved":true,"context_lines":[{"line_number":193,"context_line":"    def _restore_on_delete(self, req):"},{"line_number":194,"context_line":"        resp \u003d req.get_response(self.app, \u0027GET\u0027, req.container_name, \u0027\u0027,"},{"line_number":195,"context_line":"                                query\u003d{\u0027prefix\u0027: req.object_name,"},{"line_number":196,"context_line":"                                       \u0027versions\u0027: True})"},{"line_number":197,"context_line":"        if resp.status_int !\u003d HTTP_OK:"},{"line_number":198,"context_line":"            return resp"},{"line_number":199,"context_line":"        old_versions \u003d json.loads(resp.body)"}],"source_content_type":"text/x-python","patch_set":4,"id":"f198633a_2badc5fb","line":196,"updated":"2026-07-01 22:42:35.000000000","message":"It seems like the real bug might be that we should include `\u0027end_marker\u0027: req.object_name + \u0027\\x00\u0027` here...","commit_id":"40fa5395feda255d1474d22267efefa66f986156"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"78fb76bd310eb73d72973ae58822f7bcf02f1b8d","unresolved":true,"context_lines":[{"line_number":193,"context_line":"    def _restore_on_delete(self, req):"},{"line_number":194,"context_line":"        resp \u003d req.get_response(self.app, \u0027GET\u0027, req.container_name, \u0027\u0027,"},{"line_number":195,"context_line":"                                query\u003d{\u0027prefix\u0027: req.object_name,"},{"line_number":196,"context_line":"                                       \u0027versions\u0027: True})"},{"line_number":197,"context_line":"        if resp.status_int !\u003d HTTP_OK:"},{"line_number":198,"context_line":"            return resp"},{"line_number":199,"context_line":"        old_versions \u003d json.loads(resp.body)"}],"source_content_type":"text/x-python","patch_set":4,"id":"09501e0a_02c01827","line":196,"in_reply_to":"f198633a_2badc5fb","updated":"2026-07-02 09:23:44.000000000","message":"+1 that fix would have saved some listing items","commit_id":"40fa5395feda255d1474d22267efefa66f986156"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a167e4b58d1b8417c28684fb9f0c65699af220e3","unresolved":true,"context_lines":[{"line_number":202,"context_line":"            if item[\u0027name\u0027] !\u003d req.object_name:"},{"line_number":203,"context_line":"                break"},{"line_number":204,"context_line":"            if item[\u0027content_type\u0027] \u003d\u003d DELETE_MARKER_CONTENT_TYPE:"},{"line_number":205,"context_line":"                resp \u003d None"},{"line_number":206,"context_line":"                break"},{"line_number":207,"context_line":"            try:"},{"line_number":208,"context_line":"                resp \u003d req.get_response(self.app, \u0027PUT\u0027, query\u003d{"}],"source_content_type":"text/x-python","patch_set":4,"id":"7fe48110_5864f729","line":205,"updated":"2026-07-01 22:26:30.000000000","message":"Off-topic: well this seems unnecessary -- if we hit the `PreconditionFailed` below, it\u0027ll be *before* the assignment...","commit_id":"40fa5395feda255d1474d22267efefa66f986156"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e694e1c24c8dd08dc7ed0462bb035bd7a565bc44","unresolved":true,"context_lines":[{"line_number":206,"context_line":"                break"},{"line_number":207,"context_line":"            try:"},{"line_number":208,"context_line":"                resp \u003d req.get_response(self.app, \u0027PUT\u0027, query\u003d{"},{"line_number":209,"context_line":"                    \u0027version-id\u0027: item[\u0027version_id\u0027]})"},{"line_number":210,"context_line":"            except PreconditionFailed:"},{"line_number":211,"context_line":"                self.logger.debug(\u0027skipping failed PUT?version-id\u003d%s\u0027 %"},{"line_number":212,"context_line":"                                  item[\u0027version_id\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"f9a20be3_a2dc73b1","line":209,"updated":"2026-07-01 00:10:18.000000000","message":"yeah this sort of looks like a \"normal\" swift-api version\u0027d PUT - which I hadn\u0027t considered as a new way for object-PUT\u003d\u003e404","commit_id":"40fa5395feda255d1474d22267efefa66f986156"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"dc7cccbfec53df167d164797e0685313e29aab23","unresolved":true,"context_lines":[{"line_number":206,"context_line":"                break"},{"line_number":207,"context_line":"            try:"},{"line_number":208,"context_line":"                resp \u003d req.get_response(self.app, \u0027PUT\u0027, query\u003d{"},{"line_number":209,"context_line":"                    \u0027version-id\u0027: item[\u0027version_id\u0027]})"},{"line_number":210,"context_line":"            except PreconditionFailed:"},{"line_number":211,"context_line":"                self.logger.debug(\u0027skipping failed PUT?version-id\u003d%s\u0027 %"},{"line_number":212,"context_line":"                                  item[\u0027version_id\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"ecf4330b_8bdca229","line":209,"in_reply_to":"67dbd3a8_2b035b29","updated":"2026-07-02 16:49:52.000000000","message":"\u003e can you elaborate on the corruption angle? AFAICT the `PUT?version-id` only modifies the symlink.\n\nTrue -- it\u0027s not like we\u0027ve actually **lost data** -- but if a client only has access to the primary container (not the versions -- maybe can\u0027t even do *listings* of versions), it\u0027d sure look like data corruption from their point of view.\n\n\u003e Guaranteed correct restore_on_delete requires an atomically updated history of object versions which we don\u0027t have.\n\nTrue enough 😞","commit_id":"40fa5395feda255d1474d22267efefa66f986156"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"78fb76bd310eb73d72973ae58822f7bcf02f1b8d","unresolved":true,"context_lines":[{"line_number":206,"context_line":"                break"},{"line_number":207,"context_line":"            try:"},{"line_number":208,"context_line":"                resp \u003d req.get_response(self.app, \u0027PUT\u0027, query\u003d{"},{"line_number":209,"context_line":"                    \u0027version-id\u0027: item[\u0027version_id\u0027]})"},{"line_number":210,"context_line":"            except PreconditionFailed:"},{"line_number":211,"context_line":"                self.logger.debug(\u0027skipping failed PUT?version-id\u003d%s\u0027 %"},{"line_number":212,"context_line":"                                  item[\u0027version_id\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"67dbd3a8_2b035b29","line":209,"in_reply_to":"7caed76d_549d1366","updated":"2026-07-02 09:23:44.000000000","message":"\u003e if there\u0027s a timestamp collision... oops. That\u0027s some data silent corruption!\n\nTim - can you elaborate on the corruption angle? AFAICT the ``PUT?version-id`` only modifies the symlink.\n\n\u003e Whereas we ought to have the client retry so we can see the new state.\n\nI\u0027m not 100% sure that retry will provoke a repeated attempt to restore_on_delete. This opinion is based off trying to read the complex response status handling in S3Request (sigh) but I think that if the retried client DELETE request gets a 404 from Swift then a NoSuckKey is raised in the s3api obj controller causing it to skip _restore_on_delete ?!?\n\nIf I\u0027m right, that could be fixed, but the _restore_on_delete is always going to be best-effort based on potentially inconsistent listings. e.g. The client may get a 204 because the middleware doesn\u0027t see a previous version to restore an everything is happy path, except actually there is a previous version in async pending.\n\nGuaranteed correct restore_on_delete requires an atomically updated history of object versions which we don\u0027t have.","commit_id":"40fa5395feda255d1474d22267efefa66f986156"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"a167e4b58d1b8417c28684fb9f0c65699af220e3","unresolved":true,"context_lines":[{"line_number":206,"context_line":"                break"},{"line_number":207,"context_line":"            try:"},{"line_number":208,"context_line":"                resp \u003d req.get_response(self.app, \u0027PUT\u0027, query\u003d{"},{"line_number":209,"context_line":"                    \u0027version-id\u0027: item[\u0027version_id\u0027]})"},{"line_number":210,"context_line":"            except PreconditionFailed:"},{"line_number":211,"context_line":"                self.logger.debug(\u0027skipping failed PUT?version-id\u003d%s\u0027 %"},{"line_number":212,"context_line":"                                  item[\u0027version_id\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"7caed76d_549d1366","line":209,"in_reply_to":"f9a20be3_a2dc73b1","updated":"2026-07-01 22:26:30.000000000","message":"Heh, 404 is the *happy* case -- if there\u0027s a timestamp collision... oops. That\u0027s some data silent corruption!\n\nCome to think of it... maybe we shouldn\u0027t 404 here. Maybe we should translate to a 503? Like, if you\u0027ve got a stack of three versions, from t0, t1, t2... and you issue concurrent deletes to the versions for t1 and t2... couldn\u0027t we have both see t2 as the \"current\" version and the t2 delete sees t1 as the \"previous\" version, t1 delete goes through first, t2 delete tries to restore t1 and... 404s. Whereas we ought to have the client retry so we can see the **new** state.","commit_id":"40fa5395feda255d1474d22267efefa66f986156"}],"test/s3api/test_versioning.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e694e1c24c8dd08dc7ed0462bb035bd7a565bc44","unresolved":true,"context_lines":[{"line_number":362,"context_line":"        resp \u003d self.client.delete_object("},{"line_number":363,"context_line":"            Bucket\u003dself.bucket_name, Key\u003dobj_name,"},{"line_number":364,"context_line":"            VersionId\u003dmarker_version_id)"},{"line_number":365,"context_line":"        self.assertEqual(204, resp[\u0027ResponseMetadata\u0027][\u0027HTTPStatusCode\u0027])"},{"line_number":366,"context_line":""},{"line_number":367,"context_line":"        resp \u003d self.client.list_object_versions("},{"line_number":368,"context_line":"            Bucket\u003dself.bucket_name, Prefix\u003dobj_name)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fd63d374_0882fd6c","line":365,"updated":"2026-07-01 00:10:18.000000000","message":"that\u0027s better!","commit_id":"40fa5395feda255d1474d22267efefa66f986156"}],"test/unit/common/middleware/s3api/test_obj.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e694e1c24c8dd08dc7ed0462bb035bd7a565bc44","unresolved":true,"context_lines":[{"line_number":1561,"context_line":"            \u0027content_type\u0027: \u0027application/older\u0027,"},{"line_number":1562,"context_line":"        }]"},{"line_number":1563,"context_line":"        self.swift.register(\u0027GET\u0027, \u0027/v1/AUTH_test/bucket\u0027, swob.HTTPOk, {},"},{"line_number":1564,"context_line":"                            json.dumps(old_versions))"},{"line_number":1565,"context_line":"        req \u003d Request.blank(\u0027/bucket/object?versionId\u003d1574358170.12293\u0027,"},{"line_number":1566,"context_line":"                            method\u003d\u0027DELETE\u0027, headers\u003d{"},{"line_number":1567,"context_line":"                                \u0027Authorization\u0027: \u0027AWS test:tester:hmac\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"bc2644bf_18ea4a6f","line":1564,"updated":"2026-07-01 00:10:18.000000000","message":"FWIW I think this pattern of registering the literal json response in the `self.swift` is well established:\n\n```\n        self.swift.register(\u0027GET\u0027, \u0027/v1/AUTH_test/bucket\u0027, swob.HTTPOk, {},\n                            json.dumps(prefix_listing))\n```\n\nI think it\u0027s been well adapted to expose the current issue.","commit_id":"40fa5395feda255d1474d22267efefa66f986156"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e694e1c24c8dd08dc7ed0462bb035bd7a565bc44","unresolved":true,"context_lines":[{"line_number":1628,"context_line":"             \u0027?prefix\u003dobject\u0026versions\u003dTrue\u0027, \u00270\u0027),"},{"line_number":1629,"context_line":"        ], ["},{"line_number":1630,"context_line":"            (method, path, headers.get(\u0027content-length\u0027))"},{"line_number":1631,"context_line":"            for method, path, headers in self.swift.calls_with_headers])"},{"line_number":1632,"context_line":""},{"line_number":1633,"context_line":"    def test_object_DELETE_version_id_not_implemented(self):"},{"line_number":1634,"context_line":"        req \u003d Request.blank(\u0027/bucket/object?versionId\u003d1574358170.12293\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa971e30_4c5a5737","line":1631,"updated":"2026-07-01 00:10:18.000000000","message":"right!? yess....\n\n```\nE       AssertionError: Lists differ: [(\u0027HE[231 chars] \u00270\u0027)] !\u003d [(\u0027HE[231 chars] \u00270\u0027), (\u0027PUT\u0027, \u0027/v1/AUTH_test/bucket/object?ve[28 chars]\u00270\u0027)]\nE       \nE       Second list contains 1 additional elements.\nE       First extra element 3:\nE       (\u0027PUT\u0027, \u0027/v1/AUTH_test/bucket/object?version-id\u003d1574341899.21751\u0027, \u00270\u0027)\nE       \nE         [(\u0027HEAD\u0027,\nE           \u0027/v1/AUTH_test/bucket/object?symlink\u003dget\u0026version-id\u003d1574358170.12293\u0027,\nE           None),\nE          (\u0027DELETE\u0027,\nE           \u0027/v1/AUTH_test/bucket/object?symlink\u003dget\u0026version-id\u003d1574358170.12293\u0027,\nE           None),\nE       -  (\u0027GET\u0027, \u0027/v1/AUTH_test/bucket?prefix\u003dobject\u0026versions\u003dTrue\u0027, \u00270\u0027)]\nE       ?                                                                  ^\nE       \nE       +  (\u0027GET\u0027, \u0027/v1/AUTH_test/bucket?prefix\u003dobject\u0026versions\u003dTrue\u0027, \u00270\u0027),\nE       ?                                                                  ^\nE       \nE       +  (\u0027PUT\u0027, \u0027/v1/AUTH_test/bucket/object?version-id\u003d1574341899.21751\u0027, \u00270\u0027)]\n```\n\nthat extra PUT with someone elses version-id don\u0027t make no sense!","commit_id":"40fa5395feda255d1474d22267efefa66f986156"}]}
