AWS S3: versioning support #951
Conversation
* populate download rquest with `versionId` query parameter, if provide
* make sure existing test cases does not add any query parameter
* add test for versionId when provided
* versionId function paarmeters for each of "GET" methods
* functions to "GET" an object with version id in each of S3Client for scala and jav dsl
* populate versionId in ObjectMetadata
* function to get entity from HttpResponse with headers (entityForSuccessWithHeaders)
* function to prase entity and populate it values from header
* update S3Client for Java and Scala
* fix test cases
* ignoring one test case for now, to figure out later how to run
docs/src/main/paradox/s3.md
Outdated
| @@ -93,9 +101,15 @@ Copy an S3 object from source bucket to target bucket using multi part copy uplo | |||
| Scala | |||
| : @@snip ($alpakka$/s3/src/test/scala/akka/stream/alpakka/s3/scaladsl/S3SinkSpec.scala) { #multipart-copy } | |||
|
|
|||
| Scala | |||
| : @@snip ($alpakka$/s3/src/test/scala/akka/stream/alpakka/s3/scaladsl/S3SinkSpec.scala) { #multipart-copy-with-source-version } | |||
|
|
|||
There was a problem hiding this comment.
You should make this two snippet sections, and add some text explaining what it does.
| @@ -195,10 +223,14 @@ final class S3Client(s3Settings: S3Settings, system: ActorSystem, mat: Materiali | |||
| */ | |||
| def request(bucket: String, | |||
| key: String, | |||
| versionId: String, | |||
There was a problem hiding this comment.
Wouldn't it be better to go with Optional instead of a nullable String?
We might deprecate the request without the parameter.
s3/docker-compose.yaml
Outdated
| version: '3.1' | ||
|
|
||
| services: | ||
| minio1: |
There was a problem hiding this comment.
Did you implement integration tests against these minio servers?
I so, please send a separate PR with those and remove this section for now.
There was a problem hiding this comment.
Yes I ran against integration tests these minio servers. I noticed that you have an issue, #938, open to run integration tests in CI, in that case there should be a way to create buckets on startup and delete once done.
I'll remove and will send new PR for this.
|
As for creating a PR for docker compose file, as I mentioned in my comments if you are planning to run in CI/CD pipeline then functions need to be added to create and delete buckets at the start and end respectively, let me know about your thoughts. |
|
Thank you for growing the S3 connector even more. |
Support for download, upload, copy, and delete for a specific version, when the bucket is enabled to support versioning.
#944