-
Notifications
You must be signed in to change notification settings - Fork 842
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DynamoDB ConditionalPut #5247
DynamoDB ConditionalPut #5247
Conversation
/// * A string partition key named `"path"` | ||
/// * A string sort key named `"etag"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking change, but given #4918 was only merged this morning and hasn't been released I think that is fine.
Before I had planned to encode both the path and the etag in the partition key, as was documented here, but this requires a delimiter, which after #5020 can't be #
. This new approach avoids this, whilst also being I think easier to understand.
@@ -48,7 +48,7 @@ pub enum S3CopyIfNotExists { | |||
HeaderWithStatus(String, String, reqwest::StatusCode), | |||
/// The name of a DynamoDB table to use for coordination | |||
/// | |||
/// Encoded as either `dynamodb:<TABLE_NAME>` or `dynamodb:<TABLE_NAME>:<TIMEOUT_MILLIS>` | |||
/// Encoded as either `dynamo:<TABLE_NAME>` or `dynamo:<TABLE_NAME>:<TIMEOUT_MILLIS>` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drive by docs fix
/// $ aws dynamodb update-time-to-live --table-name <TABLE_NAME> --time-to-live-specification Enabled=true,AttributeName=ttl | ||
/// ``` | ||
/// | ||
/// To perform a conditional operation on an object with a given `path` and `etag` (`*` if creating), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DynamoDB doesn't let you use an empty string for a key attribute, so we use *
, which is an illegal ETAG value (as If-Match wildcards would then be ambiguous)
@roeap when you get a chance, perhaps you could give this a look over |
@roeap sorry to dump this on you, but I just realised this needs to go in the release or it'll be a major breaking change... Could you perhaps take a look? 🙏 Otherwise I can probably back out the other DynamoDb PR, cut the release, and then put this back |
@tustvold - I'll take a look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 - looking good, excited to see this land.
Which issue does this PR close?
Closes #.
Rationale for this change
Builds on #4918 and extends it to conditional put.
What changes are included in this PR?
Are there any user-facing changes?