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

Pagination for list bucket #30

Merged

Conversation

shabbyrobe
Copy link
Collaborator

@shabbyrobe shabbyrobe commented Mar 7, 2019

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:

  • Tests

@shabbyrobe shabbyrobe added the WIP Work in progress label Mar 7, 2019
@codecov-io
Copy link

codecov-io commented Mar 7, 2019

Codecov Report

Merging #30 into master will increase coverage by 1.32%.
The diff coverage is 78.61%.

Impacted file tree graph

@@            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
Impacted Files Coverage Δ
messages.go 70.75% <ø> (-3.2%) ⬇️
error.go 66.66% <0%> (-1.63%) ⬇️
util.go 68.96% <0%> (ø) ⬆️
routing.go 83.33% <100%> (+0.6%) ⬆️
option.go 70% <100%> (+3.33%) ⬆️
backend.go 100% <100%> (ø)
backend/s3mem/bucket.go 79.56% <100%> (+0.45%) ⬆️
range.go 83.33% <100%> (-0.54%) ⬇️
uploader.go 85.31% <100%> (-0.09%) ⬇️
prefix.go 96.96% <100%> (+6.34%) ⬆️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cfb2b0f...efe8c17. Read the comment docs.

@shabbyrobe shabbyrobe force-pushed the feature/list-bucket-page branch from 86edf9d to 6f780a3 Compare March 11, 2019 01:30
@shabbyrobe shabbyrobe removed the WIP Work in progress label Mar 11, 2019
@shabbyrobe
Copy link
Collaborator Author

Alright I think I'm done tinkering here; I reckon this one's good to go! (pending any feedback, of course!)

Copy link
Owner

@johannesboyne johannesboyne left a 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!

@johannesboyne johannesboyne merged commit 292602e into johannesboyne:master Mar 17, 2019
@shabbyrobe
Copy link
Collaborator Author

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.

@shabbyrobe shabbyrobe deleted the feature/list-bucket-page branch March 18, 2019 11:54
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.

3 participants