-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Fixes: #2212 ; Feature: Skip Duplicate Song from Sub-Folders ; Fixed #2214
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
Fixes: #2212 ; Feature: Skip Duplicate Song from Sub-Folders ; Fixed #2214
Conversation
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.
Good way of handling errors in general.
Thanks @kiruthikpurpose for your review... |
|
||
# Update output path using song.album_name if valid; otherwise, use song.artist. | ||
output_file = ( | ||
Path("output") | ||
/ ( | ||
Path(song.album_name) | ||
if Path(song.album_name).is_dir() | ||
else Path(song.artist) | ||
) | ||
/ output_file | ||
) | ||
|
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.
What's this for?
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.
@himanshumahajan138
I saw you added output/
into the .gitignore
as well. Can you elaborate on this?
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.
Actually although you have added mp3
files in the .gitignore
but when i was testing it i just added output folder for myself we can remove it, it was just my testing part
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.
Can you make a PR removing both of these please? Seems they slipped through the cracks in the haste of approving the PR :)
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.
Ya sure i will do this by today positively...
# Checking if file already exists in all subfolders of output directory | ||
file_exists = ( | ||
next(Path(output_file.parts[0]).rglob(output_file.name), None) | ||
or dup_song_paths | ||
) |
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 don't think it's good idea to perform rglob
every time we check if a file exists. This should be done once in the init function to construct a dict (?)
Also I think it would be better to add tis functionality as a command line argument, to not break the current behavior for other users. |
I can merge this one, and improve the code later if you need PR for hacktoberfest ;) |
So nice of you man, ok then merge this one and i will raise a new pr in morning with next updates Thanks for your understanding 😊 |
Hi @himanshumahajan138 @kiruthikpurpose Great to see you around the spotDL project. We are always on the lookout for more volunteers to contribute to spotDL. Thanks for your contributions thus far. |
I am interested in joining spotDL developement Team and help the community with contributions |
Thanks @himanshumahajan138. Please join the spotDL Discord and mention me ( |
This was inadvertently committed to spotDL.
Title
Fixes: #2212 ; Feature: Skip Duplicate Song from Sub-Folders ; Fixed
Description
This PR Fixes the issue of re-downloading duplicate songs if they exist in subfolders and fixed the output structure for downloaded files
Related Issue
Fixes: #2212
Motivation and Context
User faces problem in skipping duplicate files present in subfolders and a generalized folder structure, so this PR fixed this problem
How Has This Been Tested?
Performed download testing with different overwrite options to check working of the updated code
Types of Changes
Checklist