-
Notifications
You must be signed in to change notification settings - Fork 90
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
[DNM] Allow for opening with known file sizes #315
base: main
Are you sure you want to change the base?
Conversation
Surprising to still be seeing new-to-me acronyms for PR statuses 😆 What do you think of setting PRs not ready for merge to draft status? I'm fine with whatever, and I like the idea of standardized PR labels. Just curious what everyone else prefers. Are there automations that recognize the Do Not Merge label? |
It looks good but I'm not sure where these sizes will be coming from? granules have irregular sizes based on what they contain and if we are relaying only on what CMR has, there will be discrepancies. Will it be an issue with fsspec if we use a wrong size? @jrbourbeau |
Good point. We get the correct sizes when we create |
@jrbourbeau, any progress here? I'm marking this PR as Draft for now to help us know which PRs we should be actively reviewing. |
The latest release of
s3fs
allows you to specify the file size ahead of time (if known) when opening an S3 file (this already existed for HTTPS files). This allowss3fs
to skip some calls to S3 which can be expensive (especially when opening lots of files). Marking as DNM for now as I'm still experimenting with what performance impacts this has in practice.