justin025 / onthespot

OnTheSpot is an easy-to-use music downloader built with Python and Qt. Search for songs, albums, or playlists and download them directly to your device.
GNU General Public License v2.0
69 stars 7 forks source link

Account round robin doesn't seem to spread across accounts. #15

Closed medmondson-spot closed 2 weeks ago

medmondson-spot commented 2 weeks ago

Seemingly given multiple accounts I can hit a throttle using 15 seconds and 2 workers. While at first I thought I was being throttled by maybe IP I figured I'd look at the round robin setting and make sure. I added logging into the function call for the fetch_account_uuid and found that regardless of the calls it seems like the account number isn't ever incremented (particularly the increased parsing index log isn't ever found I think). I could be misunderstanding but it seems unlikely to me that round robin with 3 accounts would still hit rate limiting if used in this manner.

Some contextual info to help hopefully!

Function after modification:

def fetch_account_uuid(download):
    if config.get("rotate_acc_sn") == True:
         try:
             parsing_index = config.get("parsing_acc_sn")
             logger.info(parsing_index)
             return config.get('accounts')[parsing_index][3]
         except IndexError as e:
             parsing_index = 0
             logger.info(parsing_index)
             return config.get('accounts')[parsing_index][3]
         if download == True:
             config.set_('parsing_acc_sn', parsing_index + 1)
             logger.info('Increased parsing index')
    else:
        return config.get('accounts')[ config.get('parsing_acc_sn') - 1 ][3]
Settings used for triggering the timeout ![rotate](https://github.com/user-attachments/assets/b67e3299-6428-4f63-9f04-2d3552716c6f) ![activenumber](https://github.com/user-attachments/assets/7397c2b4-5e25-4afe-be78-0b190b62d086) ![accounts](https://github.com/user-attachments/assets/1da399e7-8d8f-43d3-b5c3-7c6c17292860)
Logs with 3 accounts ``` [2024-10-03 20:07:23,214 :: utils :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/utils/utils.py -> 309: fetch_account_uuid() :: INFO] -> 0 [2024-10-03 20:07:23,214 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 40: download_track() :: INFO] -> The media : 4rDW0EP8ujtDZeQfF788HH was cancelled ! [2024-10-03 20:07:23,214 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 417: run() :: ERROR] -> Download process returned false: 4rDW0EP8ujtDZeQfF788HH [2024-10-03 20:07:23,214 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 373: run() :: INFO] -> Processing download for track by id '25KcWyLhCwLDdkaMoVcaBR', Attempt: 0/3 [2024-10-03 20:07:23,214 :: utils :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/utils/utils.py -> 309: fetch_account_uuid() :: INFO] -> 0 [2024-10-03 20:07:23,214 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 40: download_track() :: INFO] -> The media : 25KcWyLhCwLDdkaMoVcaBR was cancelled ! [2024-10-03 20:07:23,214 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 417: run() :: ERROR] -> Download process returned false: 25KcWyLhCwLDdkaMoVcaBR [2024-10-03 20:07:23,214 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 373: run() :: INFO] -> Processing download for track by id '4vfsNaQif7Q5gBQrQtj7DT', Attempt: 0/3 [2024-10-03 20:07:23,214 :: utils :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/utils/utils.py -> 309: fetch_account_uuid() :: INFO] -> 0 [2024-10-03 20:07:23,214 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 40: download_track() :: INFO] -> The media : 4vfsNaQif7Q5gBQrQtj7DT was cancelled ! [2024-10-03 20:07:23,214 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 417: run() :: ERROR] -> Download process returned false: 4vfsNaQif7Q5gBQrQtj7DT [2024-10-03 20:07:23,215 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 373: run() :: INFO] -> Processing download for track by id '0a1yJGOJgadNoSayiMf0UF', Attempt: 0/3 [2024-10-03 20:07:23,215 :: utils :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/utils/utils.py -> 309: fetch_account_uuid() :: INFO] -> 0 [2024-10-03 20:07:23,215 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 40: download_track() :: INFO] -> The media : 0a1yJGOJgadNoSayiMf0UF was cancelled ! [2024-10-03 20:07:23,215 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 417: run() :: ERROR] -> Download process returned false: 0a1yJGOJgadNoSayiMf0UF [2024-10-03 20:07:23,215 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 373: run() :: INFO] -> Processing download for track by id '2HwQQIoQ998Qlpbd0mRiTi', Attempt: 0/3 [2024-10-03 20:07:23,215 :: utils :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/utils/utils.py -> 309: fetch_account_uuid() :: INFO] -> 0 [2024-10-03 20:07:23,215 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 40: download_track() :: INFO] -> The media : 2HwQQIoQ998Qlpbd0mRiTi was cancelled ! [2024-10-03 20:07:23,215 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 417: run() :: ERROR] -> Download process returned false: 2HwQQIoQ998Qlpbd0mRiTi [2024-10-03 20:07:23,215 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 373: run() :: INFO] -> Processing download for track by id '2trCzhDpJ2sOCeXT8l7guC', Attempt: 0/3 [2024-10-03 20:07:23,215 :: utils :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/utils/utils.py -> 309: fetch_account_uuid() :: INFO] -> 0 [2024-10-03 20:07:23,215 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 40: download_track() :: INFO] -> The media : 2trCzhDpJ2sOCeXT8l7guC was cancelled ! [2024-10-03 20:07:23,215 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 417: run() :: ERROR] -> Download process returned false: 2trCzhDpJ2sOCeXT8l7guC [2024-10-03 20:07:23,216 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 373: run() :: INFO] -> Processing download for track by id '1ooMHphgtk3QpFYuPcmNrW', Attempt: 0/3 [2024-10-03 20:07:23,216 :: utils :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/utils/utils.py -> 309: fetch_account_uuid() :: INFO] -> 0 [2024-10-03 20:07:23,216 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 40: download_track() :: INFO] -> The media : 1ooMHphgtk3QpFYuPcmNrW was cancelled ! [2024-10-03 20:07:23,216 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 417: run() :: ERROR] -> Download process returned false: 1ooMHphgtk3QpFYuPcmNrW [2024-10-03 20:07:23,216 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 373: run() :: INFO] -> Processing download for track by id '0FtPwHVWDn4iYvPTLw1xvP', Attempt: 0/3 [2024-10-03 20:07:23,216 :: utils :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/utils/utils.py -> 309: fetch_account_uuid() :: INFO] -> 0 [2024-10-03 20:07:23,216 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 40: download_track() :: INFO] -> The media : 0FtPwHVWDn4iYvPTLw1xvP was cancelled ! [2024-10-03 20:07:23,216 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 417: run() :: ERROR] -> Download process returned false: 0FtPwHVWDn4iYvPTLw1xvP [2024-10-03 20:07:23,216 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 373: run() :: INFO] -> Processing download for track by id '4Gx56JzSN43XrXgQ6PdY3W', Attempt: 0/3 [2024-10-03 20:07:23,216 :: utils :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/utils/utils.py -> 309: fetch_account_uuid() :: INFO] -> 0 [2024-10-03 20:07:23,216 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 40: download_track() :: INFO] -> The media : 4Gx56JzSN43XrXgQ6PdY3W was cancelled ! [2024-10-03 20:07:23,216 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 417: run() :: ERROR] -> Download process returned false: 4Gx56JzSN43XrXgQ6PdY3W [2024-10-03 20:07:23,216 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 373: run() :: INFO] -> Processing download for track by id '1fLQ1XuxGRRljHlso6x1vO', Attempt: 0/3 [2024-10-03 20:07:23,217 :: utils :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/utils/utils.py -> 309: fetch_account_uuid() :: INFO] -> 0 [2024-10-03 20:07:23,217 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 40: download_track() :: INFO] -> The media : 1fLQ1XuxGRRljHlso6x1vO was cancelled ! [2024-10-03 20:07:23,217 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 417: run() :: ERROR] -> Download process returned false: 1fLQ1XuxGRRljHlso6x1vO [2024-10-03 20:07:23,217 :: worker.downloader.SESSION_DL_TH-241ea88f-dc34-4688-bd66-d4e89dc25c30 :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/worker/downloader.py -> 373: run() :: INFO] -> Processing download for track by id '4xmG7CPVAauy9KbBG5J9uv', Attempt: 0/3 [2024-10-03 20:07:23,217 :: utils :: /home/tyler/Music/OnTheSpot/onthespot/src/onthespot/utils/utils.py -> 309: fetch_account_uuid() :: INFO] -> 0 ```

Putting this here mainly to log this issue incase someone runs into this, however if I get time I'll try to fix it myself but no promises on timeline!

medmondson-spot commented 2 weeks ago

Root cause seems to be the fact that the increment doesn't happen if the try except returns early. Moving it up above the try except seems to fix this. However I rewrote the function since tossing the error seems a bit extreme. Refactored function looks like

def fetch_account_uuid(download):
    if config.get("rotate_acc_sn") == True:
         parsing_index = config.get("parsing_acc_sn")
         if download == True and parsing_index < (len(config.get('accounts'))-1):
             config.set_('parsing_acc_sn', parsing_index + 1)
         else:
             config.set_('parsing_acc_sn', 0)
         return config.get('accounts')[parsing_index][3]
    else:
        return config.get('accounts')[ config.get('parsing_acc_sn') - 1 ][3]

I'll battle test it, I don't use github so I have to setup my keys to do the pull request however putting this here for bus factor.

justin025 commented 2 weeks ago

I'm dumb thanks for fixing this, reading the code now I am honestly embarrassed I wrote the previous function. Open a pull request whenever and I'd be happy to merge it.

medmondson-spot commented 2 weeks ago

I'm dumb thanks for fixing this, reading the code now I am honestly embarrassed I wrote the previous function. Open a pull request whenever and I'd be happy to merge it.

Nah man, thanks for forking this over and working on it! Glad I could help out and contribute some :) I think there's a weird interaction with this fix that I'm still looking at. Mainly seeing it in URL downloading. I don't know if maybe if it's round robin on the search and popping them onto the queue multiple times (Instead of taking let's say the first 4 results it round robins and takes 4*AccountNumber results?) I'll keep you posted though!

medmondson-spot commented 2 weeks ago

Nope, it seems like the search for an artist is greedy. If there's an artist (Let's say Royal Deluxe for an example) and we download based on them as artists. It seems like the artist Tsutro has done a collab with them which means onthespot will default to pulling all of the album that the Royal Deluxe has shown up on even if it's a single song. This can lead into some fun interactions in situations like when you pull a mixed CD (thinking like the Wow! That's what I call music throwbacks) it'll pull quite a contemporary mixture! I'll leave that nugget up to you (@justin025) to decide if that's unplanned or planned behavior :D

justin025 commented 2 weeks ago

What version are you using? If dev / git its this specific api call that is possibly pulling collabs

https://github.com/justin025/onthespot/blob/4ec3cca5de302d8f9d0b0bb3c48944a69fe8a168/src/onthespot/utils/spotify.py#L95-L99

Initially it would only pull the artists albums but users requested that the app also pull singles and other missing songs. As a result the api returns the whole album for a collab instead of just the song.

It may be overly complex to parse the album json for the current artist and only download songs that match some criteria but if you suggest to go in this direction it shouldn't be that hard to write out.

While in this file please do not look at my spaghetti code metadata function, I plan refactoring much of the codebase after I get 0.7.1 out

medmondson-spot commented 2 weeks ago

I personally think that would be the "right" route but I can also say that in the list of things to do it'd be pretty low on the totem pole imo. I think for now it's sufficient to document and then keep on keeping on. If there's anything I can help on though just give me a holler!