Consistently use GCP XML API #4207
Conversation
|
There appear to be some emulator quirks... Will fix tomorrow... |
| @@ -118,69 +118,6 @@ impl From<Error> for crate::Error { | |||
| } | |||
| } | |||
|
|
|||
| #[derive(Debug, Deserialize)] | |||
There was a problem hiding this comment.
This is just moved into list.rs so it can be shared with the gcs implementation
|
It would appear that fake-gcs-server doesn't properly support the XML APIs... - fsouza/fake-gcs-server#331 |
| @@ -96,7 +96,7 @@ jobs: | |||
|
|
|||
| - name: Configure Fake GCS Server (GCP emulation) | |||
| run: | | |||
| docker run -d -p 4443:4443 fsouza/fake-gcs-server -scheme http | |||
| docker run -d -p 4443:4443 tustvold/fake-gcs-server -scheme http -backend memory -public-host localhost:4443 | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Can we please leave a link to that PR as a comment (so we now we can update it back when it is accepted upstream)?
| @@ -639,12 +614,6 @@ impl ObjectStore for GoogleCloudStorage { | |||
| } | |||
|
|
|||
| async fn get_opts(&self, location: &Path, options: GetOptions) -> Result<GetResult> { | |||
| if options.if_modified_since.is_some() || options.if_unmodified_since.is_some() { | |||
There was a problem hiding this comment.
We can now support this 🎉
There was a problem hiding this comment.
Looks like a nice cleanup to me -- I read the PR and it all made sense to me.
I am not an expert in the various object store APIs so I probably wouldn't catch some subtle API misuse, but all in all it looks like a nice improvement to me
Thank you @tustvold
| @@ -96,7 +96,7 @@ jobs: | |||
|
|
|||
| - name: Configure Fake GCS Server (GCP emulation) | |||
| run: | | |||
| docker run -d -p 4443:4443 fsouza/fake-gcs-server -scheme http | |||
| docker run -d -p 4443:4443 tustvold/fake-gcs-server -scheme http -backend memory -public-host localhost:4443 | |||
There was a problem hiding this comment.
Can we please leave a link to that PR as a comment (so we now we can update it back when it is accepted upstream)?
There was a problem hiding this comment.
Looks like a nice cleanup to me -- I read the PR and it all made sense to me.
I am not an expert in the various object store APIs so I probably wouldn't catch some subtle API misuse, but all in all it looks like a nice improvement to me
Thank you @tustvold
Which issue does this PR close?
Relates to #2241
Relates to #3027
Closes #4209
Rationale for this change
We currently use a mixture of the XML and JSON APIs.
The XML API is more complete, supporting the full set of request preconditions, list offsets, etc... and so we should just switch over. As an added bonus this allows for code sharing between the implementations.
What changes are included in this PR?
Are there any user-facing changes?