)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ce671f0d11ca07fc5d56d95f0431eae6b774f30c","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We want the correct error from our s3api middleware"},{"line_number":10,"context_line":"when we send incorrect/invalid values for the"},{"line_number":11,"context_line":"aforementioned header. More coverage in cross-compat"},{"line_number":12,"context_line":"tests for the same"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I9874a140027cf143df1ac670e2b8a2d09b398c1e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ac14c513_8e682bb4","line":12,"range":{"start_line":11,"start_character":23,"end_line":12,"end_character":18},"updated":"2024-11-05 19:30:44.000000000","message":"I only see functional tests so far -- did you mean for them to be under `test/s3api/` instead of `test/functional/s3api/`?","commit_id":"310422bbafa7a645dc5628623887415d5ac4ae99"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"75c5d079935dec999d57f8611e7c8d82335a9ffb","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We want the correct error from our s3api middleware"},{"line_number":10,"context_line":"when we send incorrect/invalid values for the"},{"line_number":11,"context_line":"aforementioned header. More coverage in cross-compat"},{"line_number":12,"context_line":"tests for the same"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I9874a140027cf143df1ac670e2b8a2d09b398c1e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9279f667_641e44c8","line":12,"range":{"start_line":11,"start_character":23,"end_line":12,"end_character":18},"in_reply_to":"9afb9a20_64f0c77d","updated":"2024-12-05 16:39:00.000000000","message":"Done","commit_id":"310422bbafa7a645dc5628623887415d5ac4ae99"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"36b96bf063235e24553674a8b229d486f27891a3","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We want the correct error from our s3api middleware"},{"line_number":10,"context_line":"when we send incorrect/invalid values for the"},{"line_number":11,"context_line":"aforementioned header. More coverage in cross-compat"},{"line_number":12,"context_line":"tests for the same"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I9874a140027cf143df1ac670e2b8a2d09b398c1e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9afb9a20_64f0c77d","line":12,"range":{"start_line":11,"start_character":23,"end_line":12,"end_character":18},"in_reply_to":"ac14c513_8e682bb4","updated":"2024-11-21 21:44:24.000000000","message":"I meant to say we need more coverage in this patch in the form of cross-compat tests","commit_id":"310422bbafa7a645dc5628623887415d5ac4ae99"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fd2967ef4b525bad62db7e50bfde9a5fc5681f41","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We want the correct error from our s3api middleware when we send incorrect/invalid values for the aforementioned header."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Drive-By Change: Add and fix other s3api error responses"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I9874a140027cf143df1ac670e2b8a2d09b398c1e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"656546ca_6fbba854","line":11,"updated":"2024-12-17 18:53:16.000000000","message":"this ended up making it harder to merge this change; this is a common outcome with drive-bys - my recommendation is to avoid them.","commit_id":"7433672f2631c1fa08c0f611fb84650fd27cdf01"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"12cc61114216734a457b1910cdb15b7108fb595f","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We want the correct error from our s3api middleware when we send incorrect/invalid values for the aforementioned header."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Drive-By Change: Add and fix other s3api error responses"},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I9874a140027cf143df1ac670e2b8a2d09b398c1e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"a40f8ac7_a169daef","line":11,"in_reply_to":"656546ca_6fbba854","updated":"2025-02-03 15:56:07.000000000","message":"Acknowledged","commit_id":"7433672f2631c1fa08c0f611fb84650fd27cdf01"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"8fc7ab6bd9b6a6b4df0267855514db5b816b7fd4","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"7f4d46ff_8d17e791","updated":"2024-11-22 17:14:38.000000000","message":"functional tests need fixing and the patch needs more cross-compat tests","commit_id":"4552769cbb070a0448a2180ffae17da9aaae062d"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"e8c4e3e7c0d0cd749573e0e8127d46969b235d23","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"bf4e60da_691d57bb","in_reply_to":"7f4d46ff_8d17e791","updated":"2024-12-05 16:39:17.000000000","message":"Done","commit_id":"4552769cbb070a0448a2180ffae17da9aaae062d"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fd2967ef4b525bad62db7e50bfde9a5fc5681f41","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"08720cb1_c146beb8","updated":"2024-12-17 18:53:16.000000000","message":"I think if you just pull the other changes unrelated to copy-directive and copy-source into follow on work this would probably be mergeable.\n\ne.g. the `InvalidVersionID` appears to have been added and not used?\n\nIt would be nice to start a new `test.s3api.test_object` module for the cross-compat tests that don\u0027t have anything to do with mpu\n\nIt would be nice to get some new unittests to cover the changes in the s3api `error_resp[\u0027Message\u0027]` - esp the ones not covered by the new-cross compat test (although really those should just go in a separate change)\n\nIt would be nice to update the existing `test.functional.s3api` tests that already hit this error paths to include assertions on the `Message` (or perhaps even better port them into `test.s3api`)\n\nI did some exploration in a follow-on; some of it, like the s3xml_to_dict, might be worth squashing for use in beefing up your unit/functests - but the s3api cross-compat tests don\u0027t need it because boto is already doing something similar when it crafts the error_resp.\n\n937902: more tests for s3 error message | https://review.opendev.org/c/openstack/swift/+/937902\n\nI think even more valuable than \"fixing\" the err_resp Message strings is writing the cross-compat tests that demonstrate how we can hit these errors and allowing reviewers to easily confirm these changes are correct and justified.  When using most aws sdk tools the Message is probably mostly for humans and having it more strictly match the observed AWS s3 responses is probably only medium value compared to enhancing our test suite so that these errors are reproducible AND correct (esp Code or any other error resp keys like ArgumentName and ArgumentValue)","commit_id":"7433672f2631c1fa08c0f611fb84650fd27cdf01"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0c7dc6134f886bf28b18284cf9856e4952838d3f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"89cfaba9_f5b0edc6","updated":"2024-12-17 20:48:28.000000000","message":"Are there any places where it\u0027s significant that these new subclasses are also `InvalidArgument`s? I.e., do we catch `InvalidArgument` explicitly somewhere and do some special handling?","commit_id":"b12c8849af7f82305bff1108bf63cc9574cfa1ae"}],"swift/common/middleware/s3api/s3request.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"1938cfce1d40e9ec7862eedd322ac93f88a4be54","unresolved":true,"context_lines":[{"line_number":997,"context_line":"        elif len(parsed) \u003d\u003d 1 and parsed[0][0] \u003d\u003d \u0027versionId\u0027:"},{"line_number":998,"context_line":"            query \u003d {\u0027version-id\u0027: parsed[0][1]}"},{"line_number":999,"context_line":"        else:"},{"line_number":1000,"context_line":"            raise InvalidCopySource(value\u003dself.headers[\u0027X-Amz-Copy-Source\u0027])"},{"line_number":1001,"context_line":""},{"line_number":1002,"context_line":"        src_path \u003d unquote(src_path)"},{"line_number":1003,"context_line":"        src_path \u003d src_path if src_path.startswith(\u0027/\u0027) else (\u0027/\u0027 + src_path)"}],"source_content_type":"text/x-python","patch_set":6,"id":"d9c92045_fd4c082c","line":1000,"updated":"2024-12-04 01:38:01.000000000","message":"I\u0027d love it if we got a cross-compat test that hits this.","commit_id":"52421981fd629f987a7039e41fce351803ee812a"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0c7dc6134f886bf28b18284cf9856e4952838d3f","unresolved":true,"context_lines":[{"line_number":997,"context_line":"        elif len(parsed) \u003d\u003d 1 and parsed[0][0] \u003d\u003d \u0027versionId\u0027:"},{"line_number":998,"context_line":"            query \u003d {\u0027version-id\u0027: parsed[0][1]}"},{"line_number":999,"context_line":"        else:"},{"line_number":1000,"context_line":"            raise InvalidCopySource(value\u003dself.headers[\u0027X-Amz-Copy-Source\u0027])"},{"line_number":1001,"context_line":""},{"line_number":1002,"context_line":"        src_path \u003d unquote(src_path)"},{"line_number":1003,"context_line":"        src_path \u003d src_path if src_path.startswith(\u0027/\u0027) else (\u0027/\u0027 + src_path)"}],"source_content_type":"text/x-python","patch_set":6,"id":"0cb643bd_87f6b4e5","line":1000,"in_reply_to":"2e1a4984_b2d1a4bb","updated":"2024-12-17 20:48:28.000000000","message":"Yeah, I think it came down to the `__name__`/`\u003cCode\u003e` coupling that I\u0027ve gotten used to... :-/\n\nI suppose there\u0027s some precedent for using `_code` to override (`VersionedBucketNotEmpty`, `InvalidPartArgument`, `S3NotImplemented`) but those look more like\n\n- base class uses `__name__`\n- subclass uses `_code`\n\n(ignoring `S3NotImplemented`, which had other reasons for being the way it is) while this sets `_code` on a base class and lets the subclass inherit. As a result, `_code` shifted from `InvalidPartArgument` to `InvalidArgument`.\n\nI think I prefer what\u0027s on master -- prefer the `__name__` (since we have to type it anyway), but use `_code` to override *in the class that needs overriding*.\n\nIf we stick with this path (expecting `_code` to be implemented only by direct subclasses of `ErrorResponse`),\n\n- we should shift `VersionedBucketNotEmpty._code` to `BucketNotEmpty` as well, and\n- we might want to get rid of the `__name__` handling entirely -- if we\u0027re embracing the idea of nested subclasses of `ErrorResponse`, the current `__name__` defaulting feels like a foot-gun.","commit_id":"52421981fd629f987a7039e41fce351803ee812a"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"75c5d079935dec999d57f8611e7c8d82335a9ffb","unresolved":true,"context_lines":[{"line_number":997,"context_line":"        elif len(parsed) \u003d\u003d 1 and parsed[0][0] \u003d\u003d \u0027versionId\u0027:"},{"line_number":998,"context_line":"            query \u003d {\u0027version-id\u0027: parsed[0][1]}"},{"line_number":999,"context_line":"        else:"},{"line_number":1000,"context_line":"            raise InvalidCopySource(value\u003dself.headers[\u0027X-Amz-Copy-Source\u0027])"},{"line_number":1001,"context_line":""},{"line_number":1002,"context_line":"        src_path \u003d unquote(src_path)"},{"line_number":1003,"context_line":"        src_path \u003d src_path if src_path.startswith(\u0027/\u0027) else (\u0027/\u0027 + src_path)"}],"source_content_type":"text/x-python","patch_set":6,"id":"2e1a4984_b2d1a4bb","line":1000,"in_reply_to":"d9c92045_fd4c082c","updated":"2024-12-05 16:39:00.000000000","message":"I did here, https://review.opendev.org/c/openstack/swift/+/934153/6/test/s3api/test_mpu.py#865, I guess the class name threw you off there, but the _code for this error is still `InvalidArgument`","commit_id":"52421981fd629f987a7039e41fce351803ee812a"}],"swift/common/middleware/s3api/s3response.py":[{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ce671f0d11ca07fc5d56d95f0431eae6b774f30c","unresolved":true,"context_lines":[{"line_number":301,"context_line":""},{"line_number":302,"context_line":"class AccessKeyDisabled(ErrorResponse):"},{"line_number":303,"context_line":"    _status \u003d \u0027403 Forbidden\u0027"},{"line_number":304,"context_line":"    _msg \u003d \u0027Your account is disabled; please contact your administrator.\u0027"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"class AccountProblem(ErrorResponse):"}],"source_content_type":"text/x-python","patch_set":1,"id":"c81747e0_56af502d","line":304,"updated":"2024-11-05 19:30:44.000000000","message":"I don\u0027t think we\u0027re going to have any way of raising this -- middleware doesn\u0027t really have a means of conveying \"I recognize this account, but it\u0027s disabled.\"\n\nMaybe we could add a header to 403s? But that seems out of scope for this change.","commit_id":"310422bbafa7a645dc5628623887415d5ac4ae99"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"36b96bf063235e24553674a8b229d486f27891a3","unresolved":false,"context_lines":[{"line_number":301,"context_line":""},{"line_number":302,"context_line":"class AccessKeyDisabled(ErrorResponse):"},{"line_number":303,"context_line":"    _status \u003d \u0027403 Forbidden\u0027"},{"line_number":304,"context_line":"    _msg \u003d \u0027Your account is disabled; please contact your administrator.\u0027"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"class AccountProblem(ErrorResponse):"}],"source_content_type":"text/x-python","patch_set":1,"id":"09c37977_77eee96c","line":304,"in_reply_to":"c81747e0_56af502d","updated":"2024-11-21 21:44:24.000000000","message":"Acknowledged","commit_id":"310422bbafa7a645dc5628623887415d5ac4ae99"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ce671f0d11ca07fc5d56d95f0431eae6b774f30c","unresolved":true,"context_lines":[{"line_number":427,"context_line":""},{"line_number":428,"context_line":"class InvalidAccessKeyId(ErrorResponse):"},{"line_number":429,"context_line":"    _status \u003d \u0027403 Forbidden\u0027"},{"line_number":430,"context_line":"    _msg \u003d \u0027The Access Key Id you provided does not exist in our records.\u0027"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":""},{"line_number":433,"context_line":"class InvalidArgument(ErrorResponse):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a42eb64_2e467039","line":430,"updated":"2024-11-05 19:30:44.000000000","message":"Oh, but we *do* define this, even though we don\u0027t have a way of tripping *it*, either...\n\nHonestly, I rather _like_ that I can rely on Swift not returning this -- makes it abundantly clear when a user accidentally makes requests against real AWS instead of my cluster. Maybe it\u0027d be better if we got rid of this, to make it more clear that we **can\u0027t** return this.","commit_id":"310422bbafa7a645dc5628623887415d5ac4ae99"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"36b96bf063235e24553674a8b229d486f27891a3","unresolved":false,"context_lines":[{"line_number":427,"context_line":""},{"line_number":428,"context_line":"class InvalidAccessKeyId(ErrorResponse):"},{"line_number":429,"context_line":"    _status \u003d \u0027403 Forbidden\u0027"},{"line_number":430,"context_line":"    _msg \u003d \u0027The Access Key Id you provided does not exist in our records.\u0027"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":""},{"line_number":433,"context_line":"class InvalidArgument(ErrorResponse):"}],"source_content_type":"text/x-python","patch_set":1,"id":"417df9d4_0bd61ee1","line":430,"in_reply_to":"1a42eb64_2e467039","updated":"2024-11-21 21:44:24.000000000","message":"Acknowledged","commit_id":"310422bbafa7a645dc5628623887415d5ac4ae99"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ce671f0d11ca07fc5d56d95f0431eae6b774f30c","unresolved":true,"context_lines":[{"line_number":442,"context_line":"class InvalidCopySource(InvalidArgument):"},{"line_number":443,"context_line":"    def __init__(self, value):"},{"line_number":444,"context_line":"        err_msg \u003d \u0027Copy Source must mention the source bucket and key: \u0027 \\"},{"line_number":445,"context_line":"                   \u0027sourcebucket/sourcekey.\u0027"},{"line_number":446,"context_line":"        super(InvalidArgument, self).__init__("},{"line_number":447,"context_line":"            err_msg, argument_name\u003d\u0027x-amz-copy-source\u0027, argument_value\u003dvalue)"},{"line_number":448,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"f94b97f6_b2d559bc","line":445,"updated":"2024-11-05 19:30:44.000000000","message":"Why can\u0027t we just use `_msg` at the class level, like the others? I think you\u0027d just need to pass an explicit `msg\u003dNone` to the `super` constructor.","commit_id":"310422bbafa7a645dc5628623887415d5ac4ae99"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"36b96bf063235e24553674a8b229d486f27891a3","unresolved":false,"context_lines":[{"line_number":442,"context_line":"class InvalidCopySource(InvalidArgument):"},{"line_number":443,"context_line":"    def __init__(self, value):"},{"line_number":444,"context_line":"        err_msg \u003d \u0027Copy Source must mention the source bucket and key: \u0027 \\"},{"line_number":445,"context_line":"                   \u0027sourcebucket/sourcekey.\u0027"},{"line_number":446,"context_line":"        super(InvalidArgument, self).__init__("},{"line_number":447,"context_line":"            err_msg, argument_name\u003d\u0027x-amz-copy-source\u0027, argument_value\u003dvalue)"},{"line_number":448,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"0e831ece_44d0acc3","line":445,"in_reply_to":"f94b97f6_b2d559bc","updated":"2024-11-21 21:44:24.000000000","message":"Done","commit_id":"310422bbafa7a645dc5628623887415d5ac4ae99"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"ce671f0d11ca07fc5d56d95f0431eae6b774f30c","unresolved":true,"context_lines":[{"line_number":449,"context_line":""},{"line_number":450,"context_line":"class InvalidMetadataDirective(InvalidArgument):"},{"line_number":451,"context_line":"    def __init__(self):"},{"line_number":452,"context_line":"        err_msg \u003d \u0027Unknown metadata directive.\u0027"},{"line_number":453,"context_line":"        super(InvalidArgument, self).__init__(err_msg)"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"15f7d766_9b2fea6c","line":452,"updated":"2024-11-05 19:30:44.000000000","message":"This one seems all the stranger for not using `_msg`.\n\nThough it starts to make me wonder: what are we gaining with the inheritance? I don\u0027t really see us wanting to catch specifically `InvalidArgument` anywhere... Why not have this inherit directly from `ErrorResponse` and force it to be explicit about the `_status` used?","commit_id":"310422bbafa7a645dc5628623887415d5ac4ae99"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"36b96bf063235e24553674a8b229d486f27891a3","unresolved":true,"context_lines":[{"line_number":449,"context_line":""},{"line_number":450,"context_line":"class InvalidMetadataDirective(InvalidArgument):"},{"line_number":451,"context_line":"    def __init__(self):"},{"line_number":452,"context_line":"        err_msg \u003d \u0027Unknown metadata directive.\u0027"},{"line_number":453,"context_line":"        super(InvalidArgument, self).__init__(err_msg)"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7eaf93f2_3396ba97","line":452,"in_reply_to":"15f7d766_9b2fea6c","updated":"2024-11-21 21:44:24.000000000","message":"This way i know which subclass of errors belong to `InvalidArgument`","commit_id":"310422bbafa7a645dc5628623887415d5ac4ae99"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"12cc61114216734a457b1910cdb15b7108fb595f","unresolved":true,"context_lines":[{"line_number":449,"context_line":""},{"line_number":450,"context_line":"class InvalidMetadataDirective(InvalidArgument):"},{"line_number":451,"context_line":"    def __init__(self):"},{"line_number":452,"context_line":"        err_msg \u003d \u0027Unknown metadata directive.\u0027"},{"line_number":453,"context_line":"        super(InvalidArgument, self).__init__(err_msg)"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"961a3d52_7fcf0281","line":452,"in_reply_to":"7eaf93f2_3396ba97","updated":"2025-02-03 15:56:07.000000000","message":"Acknowledged","commit_id":"310422bbafa7a645dc5628623887415d5ac4ae99"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"1938cfce1d40e9ec7862eedd322ac93f88a4be54","unresolved":true,"context_lines":[{"line_number":428,"context_line":"    def __init__(self, name, value, msg\u003dNone, *args, **kwargs):"},{"line_number":429,"context_line":"        if msg is None:"},{"line_number":430,"context_line":"            msg \u003d self._msg"},{"line_number":431,"context_line":"        super().__init__(msg, argument_name\u003dname, argument_value\u003dvalue,"},{"line_number":432,"context_line":"                         *args, **kwargs)"},{"line_number":433,"context_line":""},{"line_number":434,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"17d88028_015b9e3d","line":431,"range":{"start_line":431,"start_character":8,"end_line":431,"end_character":15},"updated":"2024-12-04 01:38:01.000000000","message":"Stupid py2 😞\n\nYou\u0027ll want something like `super(InvalidArgument, self).__init__(...)` here and elsewhere.","commit_id":"52421981fd629f987a7039e41fce351803ee812a"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"75c5d079935dec999d57f8611e7c8d82335a9ffb","unresolved":false,"context_lines":[{"line_number":428,"context_line":"    def __init__(self, name, value, msg\u003dNone, *args, **kwargs):"},{"line_number":429,"context_line":"        if msg is None:"},{"line_number":430,"context_line":"            msg \u003d self._msg"},{"line_number":431,"context_line":"        super().__init__(msg, argument_name\u003dname, argument_value\u003dvalue,"},{"line_number":432,"context_line":"                         *args, **kwargs)"},{"line_number":433,"context_line":""},{"line_number":434,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"732e2b2f_c0c30af3","line":431,"range":{"start_line":431,"start_character":8,"end_line":431,"end_character":15},"in_reply_to":"17d88028_015b9e3d","updated":"2024-12-05 16:39:00.000000000","message":"Done","commit_id":"52421981fd629f987a7039e41fce351803ee812a"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fd2967ef4b525bad62db7e50bfde9a5fc5681f41","unresolved":true,"context_lines":[{"line_number":421,"context_line":""},{"line_number":422,"context_line":""},{"line_number":423,"context_line":"class InvalidAccessKeyId(ErrorResponse):"},{"line_number":424,"context_line":"    _status \u003d \u0027403 Forbidden\u0027"},{"line_number":425,"context_line":"    _msg \u003d \u0027The AWS Access Key Id you provided does not exist in our records.\u0027"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"316c80b4_bd526914","side":"PARENT","line":424,"updated":"2024-12-17 18:53:16.000000000","message":"was this just never used?","commit_id":"fe7928ea8aa45b15fde3154cd8ccdfabc43053a0"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fd2967ef4b525bad62db7e50bfde9a5fc5681f41","unresolved":true,"context_lines":[{"line_number":421,"context_line":""},{"line_number":422,"context_line":""},{"line_number":423,"context_line":"class InvalidArgument(ErrorResponse):"},{"line_number":424,"context_line":"    _code \u003d \u0027InvalidArgument\u0027"},{"line_number":425,"context_line":"    _status \u003d \u0027400 Bad Request\u0027"},{"line_number":426,"context_line":"    _msg \u003d \u0027Invalid Argument.\u0027"},{"line_number":427,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"d465ab84_6c4efdaf","line":424,"updated":"2024-12-17 18:53:16.000000000","message":"ok, so this ensures that everything that inherits from `InvalidArgument` set as the `[\u0027Error\u0027][\u0027Code\u0027]`\n\nit looks like `code` is also used for metrics in conjunction with `reason` via \"summary\"\n\n... but I\u0027m not sure how well we cover those assertions on all errors; we don\u0027t want to change any of ops metrics.","commit_id":"7433672f2631c1fa08c0f611fb84650fd27cdf01"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fd2967ef4b525bad62db7e50bfde9a5fc5681f41","unresolved":true,"context_lines":[{"line_number":427,"context_line":""},{"line_number":428,"context_line":"    def __init__(self, name, value, msg\u003dNone, *args, **kwargs):"},{"line_number":429,"context_line":"        if msg is None:"},{"line_number":430,"context_line":"            msg \u003d self._msg"},{"line_number":431,"context_line":"        super(InvalidArgument, self).__init__(msg, argument_name\u003dname,"},{"line_number":432,"context_line":"                                              argument_value\u003dvalue,"},{"line_number":433,"context_line":"                                              *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":9,"id":"a8cac0c2_4a64ec4d","line":430,"updated":"2024-12-17 18:53:16.000000000","message":"ErrorResponse already does this.  Test pass when I remove it.","commit_id":"7433672f2631c1fa08c0f611fb84650fd27cdf01"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fd2967ef4b525bad62db7e50bfde9a5fc5681f41","unresolved":true,"context_lines":[{"line_number":429,"context_line":"        if msg is None:"},{"line_number":430,"context_line":"            msg \u003d self._msg"},{"line_number":431,"context_line":"        super(InvalidArgument, self).__init__(msg, argument_name\u003dname,"},{"line_number":432,"context_line":"                                              argument_value\u003dvalue,"},{"line_number":433,"context_line":"                                              *args, **kwargs)"},{"line_number":434,"context_line":""},{"line_number":435,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"2e5285c6_462cf7e6","line":432,"updated":"2024-12-17 18:53:16.000000000","message":"I looks like `_dict_to_etree` in ErrorResponse will copy `argument_name` and `argument_value` kwargs into self.info and then insert them into the body:\n\n I got back an XML body like:\n\n```\nb\"\u003c?xml version\u003d\u00271.0\u0027 encoding\u003d\u0027UTF-8\u0027?\u003e\\n\u003cError\u003e\u003cCode\u003eInvalidArgument\u003c/Code\u003e\u003cMessage\u003eUnknown metadata directive.\u003c/Message\u003e\u003cRequestId\u003etx15976fdbad274e71ad1e0-006760a540\u003c/RequestId\u003e\u003cArgumentName\u003ex-amz-metadata-directive\u003c/ArgumentName\u003e\u003cArgumentValue\u003eBAD\u003c/ArgumentValue\u003e\u003c/Error\u003e\"\n```\n\nAre we asserting anywhere that an error response like this is what we\u0027d get back from AWS?","commit_id":"7433672f2631c1fa08c0f611fb84650fd27cdf01"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fd2967ef4b525bad62db7e50bfde9a5fc5681f41","unresolved":true,"context_lines":[{"line_number":456,"context_line":""},{"line_number":457,"context_line":"    def __init__(self, bucket, msg\u003dNone, *args, **kwargs):"},{"line_number":458,"context_line":"        ErrorResponse.__init__(self, msg, bucket_name\u003dbucket,"},{"line_number":459,"context_line":"                               *args, **kwargs)"},{"line_number":460,"context_line":""},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"class InvalidBucketState(ErrorResponse):"}],"source_content_type":"text/x-python","patch_set":9,"id":"3418f1b1_6563880e","line":459,"updated":"2024-12-17 18:53:16.000000000","message":"what was wrong with the original formatting?","commit_id":"7433672f2631c1fa08c0f611fb84650fd27cdf01"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fd2967ef4b525bad62db7e50bfde9a5fc5681f41","unresolved":true,"context_lines":[{"line_number":540,"context_line":""},{"line_number":541,"context_line":"class InvalidStorageClass(ErrorResponse):"},{"line_number":542,"context_line":"    _status \u003d \u0027400 Bad Request\u0027"},{"line_number":543,"context_line":"    _msg \u003d \u0027Invalid storage class.\u0027"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":""},{"line_number":546,"context_line":"class InvalidTargetBucketForLogging(ErrorResponse):"}],"source_content_type":"text/x-python","patch_set":9,"id":"206f2d49_3b0fc39f","line":543,"updated":"2024-12-17 18:53:16.000000000","message":"can we add some assertions around some of these other changes?\n\nSince we\u0027re changing the message the ideal test would be a cross compat; but it doesn\u0027t look like test.s3api currently tries to mess with StorageClass, which I guess AWS s3 does per-object (!!)\n\nhttps://docs.aws.amazon.com/AmazonS3/latest/userguide/sc-howtoset.html","commit_id":"7433672f2631c1fa08c0f611fb84650fd27cdf01"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fd2967ef4b525bad62db7e50bfde9a5fc5681f41","unresolved":true,"context_lines":[{"line_number":567,"context_line":"        ErrorResponse.__init__(self, msg, uri\u003duri, *args, **kwargs)"},{"line_number":568,"context_line":""},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"class InvalidVersionID(InvalidArgument):"},{"line_number":571,"context_line":"    def __init__(self, name, value):"},{"line_number":572,"context_line":"        err_msg \u003d \u0027Invalid version id specified.\u0027"},{"line_number":573,"context_line":"        super(InvalidArgument, self).__init__(err_msg,"}],"source_content_type":"text/x-python","patch_set":9,"id":"c9805a6c_287efe07","line":570,"updated":"2024-12-17 18:53:16.000000000","message":"Is this new?  I don\u0027t see where it\u0027s used...\n\n```\n(.venv) clayg@ThinkStation:~/Workspace/vagrant-swift-all-in-one/swift$ ag InvalidVersionID\nswift/common/middleware/s3api/s3response.py\n568:class InvalidVersionID(InvalidArgument):\n(.venv) clayg@ThinkStation:~/Workspace/vagrant-swift-all-in-one/swift$ \n```","commit_id":"7433672f2631c1fa08c0f611fb84650fd27cdf01"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fd2967ef4b525bad62db7e50bfde9a5fc5681f41","unresolved":true,"context_lines":[{"line_number":704,"context_line":"class S3NotImplemented(ErrorResponse):"},{"line_number":705,"context_line":"    _status \u003d \u0027501 Not Implemented\u0027"},{"line_number":706,"context_line":"    _msg \u003d \u0027A header you provided implies functionality that is \u0027 \\"},{"line_number":707,"context_line":"           \u0027not implemented\u0027"},{"line_number":708,"context_line":"    _code \u003d \u0027NotImplemented\u0027"},{"line_number":709,"context_line":""},{"line_number":710,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"96414b38_0de6fa78","line":707,"updated":"2024-12-17 18:53:16.000000000","message":"is it reasonable that this is the ONLY error s3api returns for 501?  What about unsupported request METHOD?\n\nI couldn\u0027t find any tests for not implemented method; but there\u0027s a bunch of unittests that assert on 501 and manage to get a *different* \"Message\" in the response body - so I guess we mostly override this value and this is just some kind of \"default\" (in which case I wonder if this is the BEST default?)","commit_id":"7433672f2631c1fa08c0f611fb84650fd27cdf01"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"0c7dc6134f886bf28b18284cf9856e4952838d3f","unresolved":true,"context_lines":[{"line_number":704,"context_line":"class S3NotImplemented(ErrorResponse):"},{"line_number":705,"context_line":"    _status \u003d \u0027501 Not Implemented\u0027"},{"line_number":706,"context_line":"    _msg \u003d \u0027A header you provided implies functionality that is \u0027 \\"},{"line_number":707,"context_line":"           \u0027not implemented\u0027"},{"line_number":708,"context_line":"    _code \u003d \u0027NotImplemented\u0027"},{"line_number":709,"context_line":""},{"line_number":710,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"ea2970e8_2f8285fe","line":707,"in_reply_to":"96414b38_0de6fa78","updated":"2024-12-17 20:48:28.000000000","message":"FWIW, it\u0027s the only version of a 501 I\u0027ve ever gotten out of AWS -- see https://review.opendev.org/c/openstack/swift/+/927177","commit_id":"7433672f2631c1fa08c0f611fb84650fd27cdf01"}],"test/s3api/test_mpu.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fd2967ef4b525bad62db7e50bfde9a5fc5681f41","unresolved":true,"context_lines":[{"line_number":855,"context_line":"        self.assertIn(complete_mpu_resp[\u0027Error\u0027][\u0027PartNumber\u0027], (\u00271\u0027, \u00272\u0027))"},{"line_number":856,"context_line":"        self.assertEqual(complete_mpu_resp[\u0027Error\u0027][\u0027ETag\u0027], None)"},{"line_number":857,"context_line":""},{"line_number":858,"context_line":"    def test_copy_object_with_errors(self):"},{"line_number":859,"context_line":"        dst_bucket \u003d self.create_name(\u0027dst-buk\u0027)"},{"line_number":860,"context_line":"        src_key \u003d self.create_name(\u0027src-key\u0027)"},{"line_number":861,"context_line":"        dst_key \u003d self.create_name(\u0027dst-key\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"af361497_f226897b","line":858,"updated":"2024-12-17 18:53:16.000000000","message":"is this an MPU test?  we have `test.functional.s3api.test_object`\n\nIt would probably be better to spin up a new `test.s3api.test_object` and start to mirror/port some of the `functiona.s3api` tests into the cross-compat suite.","commit_id":"7433672f2631c1fa08c0f611fb84650fd27cdf01"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"12cc61114216734a457b1910cdb15b7108fb595f","unresolved":true,"context_lines":[{"line_number":855,"context_line":"        self.assertIn(complete_mpu_resp[\u0027Error\u0027][\u0027PartNumber\u0027], (\u00271\u0027, \u00272\u0027))"},{"line_number":856,"context_line":"        self.assertEqual(complete_mpu_resp[\u0027Error\u0027][\u0027ETag\u0027], None)"},{"line_number":857,"context_line":""},{"line_number":858,"context_line":"    def test_copy_object_with_errors(self):"},{"line_number":859,"context_line":"        dst_bucket \u003d self.create_name(\u0027dst-buk\u0027)"},{"line_number":860,"context_line":"        src_key \u003d self.create_name(\u0027src-key\u0027)"},{"line_number":861,"context_line":"        dst_key \u003d self.create_name(\u0027dst-key\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"b67a6086_99255fef","line":858,"in_reply_to":"af361497_f226897b","updated":"2025-02-03 15:56:07.000000000","message":"Acknowledged","commit_id":"7433672f2631c1fa08c0f611fb84650fd27cdf01"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"fd2967ef4b525bad62db7e50bfde9a5fc5681f41","unresolved":true,"context_lines":[{"line_number":884,"context_line":"            self.client.copy_object(Bucket\u003ddst_bucket, Key\u003ddst_key,"},{"line_number":885,"context_line":"                                    CopySource\u003d{\u0027Bucket\u0027: self.bucket_name,"},{"line_number":886,"context_line":"                                                \u0027Key\u0027: src_key},"},{"line_number":887,"context_line":"                                    MetadataDirective\u003d\u0027INVALID_DIRECTIVE\u0027)"},{"line_number":888,"context_line":"            err_resp \u003d caught.exception.response"},{"line_number":889,"context_line":"            self.assertEqual(err_resp[\u0027ResponseMetadata\u0027][\u0027HTTPStatusCode\u0027],"},{"line_number":890,"context_line":"                             400)"}],"source_content_type":"text/x-python","patch_set":9,"id":"a2bb4f60_55729778","line":887,"updated":"2024-12-17 18:53:16.000000000","message":"it seems like valid options for `--metadata-directive` are `COPY` and `REPLACE`\n\nthose are both tested in `test_put_object_copy_metadata_directive` over in `test.functional.s3api.test_object` along with the value of `BAD` (although it doesn\u0027t assert on the `Message` and doesn\u0027t support running against AWS)","commit_id":"7433672f2631c1fa08c0f611fb84650fd27cdf01"},{"author":{"_account_id":34892,"name":"ASHWIN A NAIR","display_name":"indianwhocodes","email":"nairashwin952013@gmail.com","username":"indianwhocodes","status":"Nvidia"},"change_message_id":"12cc61114216734a457b1910cdb15b7108fb595f","unresolved":true,"context_lines":[{"line_number":884,"context_line":"            self.client.copy_object(Bucket\u003ddst_bucket, Key\u003ddst_key,"},{"line_number":885,"context_line":"                                    CopySource\u003d{\u0027Bucket\u0027: self.bucket_name,"},{"line_number":886,"context_line":"                                                \u0027Key\u0027: src_key},"},{"line_number":887,"context_line":"                                    MetadataDirective\u003d\u0027INVALID_DIRECTIVE\u0027)"},{"line_number":888,"context_line":"            err_resp \u003d caught.exception.response"},{"line_number":889,"context_line":"            self.assertEqual(err_resp[\u0027ResponseMetadata\u0027][\u0027HTTPStatusCode\u0027],"},{"line_number":890,"context_line":"                             400)"}],"source_content_type":"text/x-python","patch_set":9,"id":"88392eff_57585856","line":887,"in_reply_to":"a2bb4f60_55729778","updated":"2025-02-03 15:56:07.000000000","message":"Acknowledged","commit_id":"7433672f2631c1fa08c0f611fb84650fd27cdf01"}]}
