HDDS-13919. S3 Conditional Writes (PutObject)#9815
Draft
peterxcli wants to merge 7 commits intoapache:masterfrom
Draft
HDDS-13919. S3 Conditional Writes (PutObject)#9815peterxcli wants to merge 7 commits intoapache:masterfrom
peterxcli wants to merge 7 commits intoapache:masterfrom
Conversation
…not exists" (If-None-Match) semantics.
…nt tests and protocol definition.
…ion handling - Implemented tests for `createKey` and `rewriteKey` methods to validate behavior when using the `EXPECTED_GEN_CREATE_IF_NOT_EXISTS` constant. - Added scenarios for key creation when the key is absent and when it already exists. - Enhanced the `rewriteFailsWhenKeyExists` test to cover cases for both committed and uncommitted keys. - Updated error handling to ensure correct responses for key existence checks.
peterxcli
changed the title
jojochuang
reviewed
Mar 7, 2026
Contributor
jojochuang
left a comment
jojochuang
left a comment
There was a problem hiding this comment.
from a quick glance it looks mostly reasonable.
| .addAllMetadataGdpr(metadata) | ||
| .addAllTags(tags) | ||
| .setLatestVersionLocation(getLatestVersionLocation) | ||
| .setExpectedETag(expectedETag); |
Contributor
There was a problem hiding this comment.
does it set setExpectedDataGeneration here?
Comment on lines
211
to
+270
| @@ -267,13 +267,17 @@ public enum ResultCodes { | |||
| UNAUTHORIZED, | |||
|
|
|||
| S3_SECRET_ALREADY_EXISTS, | |||
|
|
|||
Contributor
There was a problem hiding this comment.
please do not commit changes that unrelated.
| @@ -187,8 +188,18 @@ public Response put( | |||
| throw newError(S3ErrorTable.NO_SUCH_BUCKET, bucketName, ex); | |||
| } else if (ex.getResult() == ResultCodes.FILE_ALREADY_EXISTS) { | |||
| throw newError(S3ErrorTable.NO_OVERWRITE, keyPath, ex); | |||
| } else if (ex.getResult() == ResultCodes.KEY_ALREADY_EXISTS) { | |||
| throw newError(PRECOND_FAILED, keyPath, ex); | |||
Contributor
There was a problem hiding this comment.
would you like to consider having different error messages for different cases?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
ozone/hadoop-hdds/docs/content/design/s3-conditional-requests.md
Lines 59 to 82 in 6c8a08e
Should be merged after #9332
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13919
How was this patch tested?