Add ObjectStore::put_opts / Conditional Put (#4879) #4984
Conversation
| /// Stores will use differing combinations of `e_tag` and `version` to provide conditional | ||
| /// updates, and it is therefore recommended applications preserve both |
There was a problem hiding this comment.
This is kind of annoying, but is necessary because GCS doesn't support etag-based preconditions on put, only ifGenerationMatch
tustvold
force-pushed
the
conditional-put
branch
from
deb80bd
to
f88574b
Compare
tustvold
force-pushed
the
conditional-put
branch
from
7617bad
to
be34993
Compare
| @@ -1406,14 +1472,66 @@ mod tests { | |||
| // Can retrieve previous version | |||
| let get_opts = storage.get_opts(&path, options).await.unwrap(); | |||
| let old = get_opts.bytes().await.unwrap(); | |||
| assert_eq!(old, b"foo".as_slice()); | |||
| assert_eq!(old, b"test".as_slice()); | |||
There was a problem hiding this comment.
This was a merge conflict from #4935
| if let Some(version) = &options.version { | ||
| request = request.query(&[("generation", version)]); | ||
| } |
There was a problem hiding this comment.
This was a merge conflict from #4935
| @@ -444,24 +514,3 @@ impl ListClient for GoogleCloudStorageClient { | |||
| Ok((response.try_into()?, token)) | |||
| } | |||
| } | |||
|
|
|||
| #[derive(serde::Deserialize, Debug)] | |||
There was a problem hiding this comment.
These are moved to be shared with S3 (as they are the same)
|
|
||
| let etag = get_etag(result.headers()).context(MetadataSnafu)?; |
There was a problem hiding this comment.
This was a mistake introduced in #4971, this needs to extract the etag from the XML payload
| @@ -147,33 +155,6 @@ impl From<Error> for crate::Error { | |||
| } | |||
| } | |||
|
|
|||
| #[derive(Debug, Deserialize)] | |||
There was a problem hiding this comment.
These are moved to be shared with GCS
| async fn put(&self, location: &Path, bytes: Bytes) -> Result<PutResult> { | ||
| async fn put_opts(&self, location: &Path, bytes: Bytes, opts: PutOptions) -> Result<PutResult> { | ||
| if opts.mode != PutMode::Overwrite { | ||
| // TODO: Add support for If header - https://datatracker.ietf.org/doc/html/rfc2518#section-9.4 |
There was a problem hiding this comment.
I couldn't actually find an easy to run WebDav implementation that respected this header, I don't have a use-case for this currently so punted on this
| async fn put(&self, location: &Path, bytes: Bytes) -> Result<PutResult> { | ||
| async fn put_opts(&self, location: &Path, bytes: Bytes, opts: PutOptions) -> Result<PutResult> { | ||
| if matches!(opts.mode, PutMode::Update(_)) { | ||
| return Err(crate::Error::NotImplemented); |
There was a problem hiding this comment.
I intend to add support for this as a follow up PR
tustvold
changed the title
Which issue does this PR close?
Closes #4879
Relates to #4754
Rationale for this change
Allows optimistic concurrency control based transactions against object storage
What changes are included in this PR?
Are there any user-facing changes?