Skip to content
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

AWS S3: Add S3.listBuckets #2899

Merged
merged 1 commit into from
Sep 5, 2022
Merged

Conversation

mdedetrich
Copy link
Contributor

@mdedetrich mdedetrich commented Sep 5, 2022

This PR adds the AWS S3 list buckets call (see https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListBuckets.html). The call itself is trivial however there are some non conventional differences

  • This seems to be one of the few S3 calls that returns a list of entities that is not paginated (i.e. the list of entities is a XML node list in the response). Due to this its an open question whether the S3.listBuckets method should just return a Future[Seq[ListBucketsResultContents]] or the current version which is a Source[ListBucketsResultContents, NotUsed]. I opted for the Source variant due to consistency with the rest of the S3 API (every single other method in S3 that returns a list of entities is a Source) plus there is a chance that AWS may add a paginated list-buckets call in the future in which case its trivially easy to use the newly updated version. I don't feel strongly about it either way however, so if you want me to change it to Future[Seq[ListBucketsResultContents]] then just let me know
  • The base construction of the URI is also very different to any other S3 call, hence why the listBuckets call in HttpRequests.scala is manually constructing the URI itself, i.e. there isn't any Path/Virtual access style and the authority is hardcoded to s3.amazonaws.com (for AWS) since the call is region agnostic. I have tested this both with Minio and a real AWS S3 account so I can confirm its working

@mdedetrich
Copy link
Contributor Author

Test failure for CI / ScalaDoc, Documentation with Paradox (pull_request) is unrelated (link validator is failing).

Copy link
Member

@ennru ennru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Great work, as always.

@ennru ennru changed the title Add S3.listBuckets AWS S3: Add S3.listBuckets Sep 5, 2022
@ennru ennru merged commit 4ebf24a into akka:master Sep 5, 2022
@mdedetrich mdedetrich deleted the add-s3-listbuckets branch September 5, 2022 18:27
pityka pushed a commit to pityka/alpakka that referenced this pull request Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants