Added threading to search methods #194
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
removed artist.num_songs for thread safety
during sorting len(tracks) will return 1 instead of the actual length, so I used a variable instead
When using threads, songs are not guaranteed to be inserted in order, so they need to be sorted
sublass therading.Thread, run the thread and if it resulted in an exception, put that error in an errors_queue. Then check the queue after a page is done and raise if there are any errors
- cleared thread_pool list for next pages - moved print statement in search_artist to after songs have been fetched. When threads are used, this statement would get printed before songs are done being fetched
Set thread.daemon to True so that they're killed when an error is raised
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
search_albumandsearch_artistmethods could definitely benefit from threading. I added thenum_workersparameter for the user to define the number of threads used. If the user just used the defaultnum_workers=1, no threading is used and the songs are downloaded like they were before.As for thread safety:
artist.songs.artist.num_songsattribute isn't thread-safe and we'd need to set setter/getters and athreading.lockif we wanted to keep that attribute. Instead, I removed it and substituted it withlen(artist.songs).requests.Sessionobject is not thread-safe and it's recommended to use one Session per thread. But considering that we don't modify the Session at all in this solution, I don't think that'll be a problem. Although it needs a little more investigation.Checklist