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

Fix paging-related bugs by reworking the strategy #36

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sveniu
Copy link

@sveniu sveniu commented Oct 6, 2023

Responses from the S3 ListObjectVersions API may have pages where the Versions list is not present because the DeleteMarkers list takes up the entire page.

Fix that bug and rework the strategy to make it clearer and easier to reason about.

Responses from the S3 ListObjectVersions API may have pages where the
Versions list is not present because the DeleteMarkers list takes up the
entire page.

Fix that bug and rework the strategy to make it clearer and easier to
reason about.
@angeloc
Copy link
Owner

angeloc commented Jan 17, 2024

@sveniu Any news on this? It looks promising!

@sveniu
Copy link
Author

sveniu commented Jan 17, 2024

@angeloc It ended up being hard to test, so I worked on getmoto/moto#6896 for a while to be able to use moto in the s3-pit-restore tests. I'm not actually 100% certain that the moto change implements the exact behaviour of the real S3 server, but it should be pretty close and quite useful for testing.

Timing wise, this has fallen off my radar for the moment, but it is still relevant to me and I'll likely come back to work on this soon-ish.

@angeloc
Copy link
Owner

angeloc commented Jan 17, 2024

@angeloc It ended up being hard to test, so I worked on getmoto/moto#6896 for a while to be able to use moto in the s3-pit-restore tests

Very nice indeed!

Timing wise, this has fallen off my radar for the moment, but it is still relevant to me and I'll likely come back to work on this soon-ish.

No problem, you do it when you can, like the rest of us!

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.

2 participants