-
Notifications
You must be signed in to change notification settings - Fork 64
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
Reduce network overhead of _refresh_cache #111
Conversation
Make a single network call in _reresh_cache
Codecov Report
@@ Coverage Diff @@
## master #111 +/- ##
========================================
- Coverage 91.4% 91.3% -0.2%
========================================
Files 8 8
Lines 680 691 +11
========================================
+ Hits 622 631 +9
- Misses 58 60 +2
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a reasonable solution to me.
Upload was also slow. Caching stats calls here as well. Bonus: updated comment typo
Use objects filter instead of getting metadata to reduce cloud calls in file/dir check
Bonus: loop to list comprehension
Two updates, and I think this is ready to go if all the tests pass. (1) In testing, found that (2) While looking for speed improvements, found that filtering the objects list made both our |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🎉
We used to do a lot of network calls in _refresh_cache, which slowed down scripts that looked at lots of files. This change makes it so
_refresh_cache
only needs one network call.NoStat
exception if we can't get stats for a particular path (two cases: could be directory or could not exist)Downsides:
On a slow connection, it seems ~4x faster after removing the additional calls. (First
_refresh_cache
is slow because it actually downloads the file on my slow connection)BEFORE:
AFTER:
Closes #110