-
Notifications
You must be signed in to change notification settings - Fork 87
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
Pagination for list bucket #30
Pagination for list bucket #30
Conversation
Codecov Report
@@ Coverage Diff @@
## master #30 +/- ##
==========================================
+ Coverage 65% 66.32% +1.32%
==========================================
Files 26 27 +1
Lines 2260 2355 +95
==========================================
+ Hits 1469 1562 +93
Misses 568 568
- Partials 223 225 +2
Continue to review full report at Codecov.
|
86edf9d
to
6f780a3
Compare
Alright I think I'm done tinkering here; I reckon this one's good to go! (pending any feedback, of course!) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Man, I really like that! Implements a super important part of the S3 interface!
Thanks heaps for merging! I've done a bit more digging and found that there are a lot of fussy edge cases that need to be handled before it'll be possible to work out a convenient way to roll this into the bolt and afero backends. I think the next time I get a block of time to look at stuff I might start cleaning up all the messy tests I've written recently and start looking at the backend tester; that'll make it a lot easier to start dealing with the finnicky bits. |
This implements pagination for both versions of the "list bucket" API:
I'm using a bit of a trick with the ContinuationToken to save backend implementers from having to implement this twice; all the S3-mandated hacks are pushed into GoFakeS3.
Only the memory backend implements the pagination for now; I'm still trying to think of a good way to do it in the bolt backend. I might try and do that as a separate PR.
NOTE - this PR is based on #28 and depends on that merge, so it should stay WIP until that one has made it through review into master.TODO: