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

Ensure consistent use of versions during pagination #591

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

rlamy
Copy link
Contributor

@rlamy rlamy commented Nov 27, 2023

Fixes #590.

I didn't add a test since I don't know how to write a sensible unit-test for paginated results.

@martindurant
Copy link
Member

martindurant commented Nov 28, 2023

So that I understand what's going on here: listing of files should either be versioned or not, because it's not anticipated to be able to "list" a specific file generation? We usually do allow list of a specific path, though (would return a list of one item, essentially the same as info() ), so we need to make sure that that still works. Of course, such a listing with a specified generation would never be paginated.

To test, the call accepts max-keys, so that could be set to a small number for the sake of a test. I'm not immediately sure if that would need to be a monkeypatch. (having said that, pushing >1000 files to a local moto server shouldn't take that long at all)

@rlamy
Copy link
Contributor Author

rlamy commented Nov 28, 2023

The only use of generation is to decide whether to ask for versions. I guess it's indeed to support single-file ls. The issue seems to be that the page token may be rejected (depending on bucket settings, auth method, ...) if versions isn't set in the same way as in the first call.

I tried your suggestion for testing, but it looks like gcs-fake-server (this is GCS, not S3 😉) doesn't ever return paginated results.

@martindurant
Copy link
Member

Got it :)

@martindurant martindurant merged commit 20959bb into fsspec:main Nov 29, 2023
5 checks passed
@rlamy
Copy link
Contributor Author

rlamy commented Nov 29, 2023

Thank you! Any idea of when the next release will be?

@rlamy rlamy deleted the version-pagination branch November 29, 2023 16:52
@rlamy rlamy restored the version-pagination branch November 29, 2023 16:52
@martindurant
Copy link
Member

when the next release will be

As soon as we've sorted out the aiobotocore version pinning issue in s3fs (because they are always released together with fsspec)

@rlamy rlamy deleted the version-pagination branch December 1, 2023 14:50
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.

Error when listing large directory with versions=True
2 participants