-
Notifications
You must be signed in to change notification settings - Fork 28
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
Replace os.path in storage module #30
Conversation
Codecov Report
@@ Coverage Diff @@
## master #30 +/- ##
==========================================
+ Coverage 51.04% 51.12% +0.07%
==========================================
Files 9 9
Lines 764 757 -7
==========================================
- Hits 390 387 -3
+ Misses 374 370 -4
Continue to review full report at Codecov.
|
except Exception as e: | ||
shutil.rmtree(self._image_dir) | ||
self._image_dir.mkdir() | ||
except IOError as e: | ||
logger.warning("Error clearing image directory: %s", e) |
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.
f-strings while we are here?
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.
There's lots of modulo formatting in this file, so I opted to not change anything more than the lines I changed anyway.
path = os.path.join(root, name) | ||
logger.info("Deleting file %s", path) | ||
os.remove(path) | ||
for image_path in self._image_dir.glob("**/*"): |
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.
Note Using the “**” pattern in large directory trees may consume an inordinate amount of time.
Should we be using glob.iglob
or are we going to have this problem anyway. Same would go for any other uses of glob.
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.
I guess that note mostly refers to "if you run this on something unknown, you don't know how long it will take". We only use it on dirs we've built ourselves. The size of this directory tree isn't any larger than the number of albums in your local music collection.
PR #29 replaced all the
os.path
usage in the scanner. This finishes the work by replacing allos.path
usage in the storage module, and thus fixes #20.