Optimize fs.scandir #37
Conversation
Codecov Report
@@ Coverage Diff @@
## master #37 +/- ##
=========================================
+ Coverage 96.96% 97.1% +0.13%
=========================================
Files 5 5
Lines 330 345 +15
=========================================
+ Hits 320 335 +15
Misses 10 10
Continue to review full report at Codecov.
|
|
You can run the tests locally if you have the Docker client installed and running, and the |
|
Thanks! I was able to get the tests running locally. FYI I'll squash the new commits down again once the reviews are complete -- I'm worried that github reviews will not show things clearly otherwise; I don't have much experience with its workflow. (I use https://reviewable.io/ for all of my reviews, and it's astonishingly great.) |
|
No problem, in the end I'll just squash everything in one commit so feel free to make your branch dirty 😉 |
|
@althonos All of my changes are pushed; do you have any additional feedback? |
fs/sshfs/sshfs.py
Outdated
| start, stop = page or (None, None) | ||
| try: | ||
| with convert_sshfs_errors('scandir', path, directory=True): | ||
| stat_iter = self._sftp.listdir_iter(_path) |
There was a problem hiding this comment.
Just use listdir_attr here, this way you won't have to collect the results with list later!
There was a problem hiding this comment.
Done. I was trying to preserve the responsiveness benefits of listdir_iter while paging, but maybe nobody uses paging with sshfs anyway, since we don't have a fast way to skip to the start.
|
Thanks again! I'll merge and then tinker with the code a bit to see if I can resolve the concurrency issues by locking during iteration using a custom iterator. |
|
I released the patch in |
Closes #36.
This is my best cut so far. I didn't immediately see how to run the tests locally, so hopefully Travis will tell me. (I did test as a monkey patch within my application integration.)
When
DirectoryExpectedis raised the backtrace is a bit heavy -- is has the two prior exceptions shown as well. I tried some control flow phrasings to work around this, but found them all a bit too awkward.Happy to take any feedback.