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

Allow size= in open() #797

Merged
merged 2 commits into from
Sep 22, 2023
Merged

Allow size= in open() #797

merged 2 commits into from
Sep 22, 2023

Conversation

martindurant
Copy link
Member

Fixes #794

@jrbourbeau , do you want to test? I'll think about how to test this in CI.

@jrbourbeau
Copy link
Contributor

Thanks @martindurant! I'll test this out

Naively, looking at the PR diff, we may still be triggering an s3.info call here https://github.com/fsspec/s3fs/pull/797/files#diff-044e3dd45e47667275a210a6ac0033c31687eaea7d4e33e3b21b34baa1a050daR2083 (I could be wrong though)

@martindurant
Copy link
Member Author

That's only for buckets with versioning. I don't suppose I need to change too many code paths in one go here, but I has better yet add the docstring for size=

@martindurant
Copy link
Member Author

# after any initial call which does SSL and checks credentials. 
In [4]: %time f = fs.open("mymdtemp/icesat2-4.01.json")
CPU times: user 4.07 ms, sys: 1.76 ms, total: 5.83 ms
Wall time: 57.3 ms

In [5]: fs.invalidate_cache()

In [6]: %time f = fs.open("mymdtemp/icesat2-4.01.json", size=100)
CPU times: user 110 µs, sys: 0 ns, total: 110 µs
Wall time: 113 µs

Copy link
Contributor

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this update @martindurant, I'm also seeing performance improvements when opening with size= specified

@martindurant martindurant merged commit 7e05807 into fsspec:main Sep 22, 2023
5 checks passed
@martindurant martindurant deleted the file-size branch September 22, 2023 15:41
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.

Support size= in S3File
2 participants