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

S3: really call version 2 in list_objects_v2 #317

Merged
merged 5 commits into from
Mar 17, 2021
Merged

S3: really call version 2 in list_objects_v2 #317

merged 5 commits into from
Mar 17, 2021

Conversation

laborg
Copy link
Contributor

@laborg laborg commented Mar 16, 2021

Fixes #316 by only using S3 escape rules for the path.

  • With this I can properly use the continuation token for paginated results.
  • I don't know enough about minio to know if the added tests are suitable to detect a regression.
  • URIs.jl has been added as an dependency, but I feel that this will sooner or later be the case anyway.

@mattBrzezinski
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Mar 16, 2021
@bors
Copy link
Contributor

bors bot commented Mar 16, 2021

try

Build failed:

@laborg
Copy link
Contributor Author

laborg commented Mar 16, 2021

bors try
(okay, that was somehow expected)

@bors
Copy link
Contributor

bors bot commented Mar 16, 2021

🔒 Permission denied

Existing reviewers: click here to make laborg a reviewer

@mattBrzezinski
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Mar 16, 2021
@bors
Copy link
Contributor

bors bot commented Mar 16, 2021

Copy link
Member

@mattBrzezinski mattBrzezinski left a comment

Choose a reason for hiding this comment

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

LGTM, just a few minor things, thank you for making this PR!

Project.toml Outdated Show resolved Hide resolved
src/AWS.jl Outdated Show resolved Hide resolved
Copy link
Member

@mattBrzezinski mattBrzezinski left a comment

Choose a reason for hiding this comment

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

Two minor changes and it's good to go!

test/AWS.jl Outdated Show resolved Hide resolved
test/AWS.jl Outdated Show resolved Hide resolved
laborg and others added 2 commits March 17, 2021 16:09
Co-authored-by: mattBrzezinski <matt.brzezinski@invenia.ca>
Co-authored-by: mattBrzezinski <matt.brzezinski@invenia.ca>
@mattBrzezinski
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Mar 17, 2021
317: S3: really call version 2 in `list_objects_v2` r=mattBrzezinski a=laborg

Fixes #316 by only using S3 escape rules for the path.

- With this I can properly use the continuation token for paginated results.
- I don't know enough about minio to know if the added tests are suitable to detect a regression. 
- URIs.jl has been added as an dependency, but I feel that this will sooner or later be the case anyway.

Co-authored-by: Gerhard Aigner <gerhard.aigner@gmail.com>
@laborg
Copy link
Contributor Author

laborg commented Mar 17, 2021

Thanks for steering this PR. I really like your work on AWS.jl.

@mattBrzezinski
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 17, 2021

@bors bors bot merged commit 2b7758f into JuliaCloud:master Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

S3.list_objects_v2 doesn't contain a continuation token
2 participants