Add 'downloadWaitTime' setting #274
Conversation
Controls how much time FPD should wait before downloading the next video.
|
Your PR's base branch was set to |
|
This is pointless. The downloader already avoids ratelimits and respects the Floatplane api. I'm going to close this but I'm curious, why would you want this? |
|
Sorry! I was having issues downloading the backlog. I ran this locally and it worked pretty okay. |
|
No that's perfectly fine. What issues were you having? There is a bug with authentication being refreshed atm which could be related. But I can't fix it atm as I am sick with covid and away from home 😭 |
There was a problem hiding this comment.
Pull request overview
This PR adds a new downloadWaitTime setting that allows users to introduce a configurable delay between video downloads. This is useful for preventing server overload when downloading large backlogs and avoiding rate limiting.
Key changes:
- Added
downloadWaitTimesetting (default: 0/disabled) to control wait time in seconds between downloads - Modified semaphore logic to use sequential downloads (1 thread) when wait time is enabled, otherwise maintains 8 concurrent downloads
- Added sleep/wait implementation in the download method to enforce the delay between downloads
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| wiki/settings.md | Documents the new downloadWaitTime setting with explanation of its purpose and usage |
| src/lib/types.ts | Adds downloadWaitTime: number to the Settings type definition |
| src/lib/defaults.ts | Sets default value of downloadWaitTime to 0 (disabled) |
| src/lib/Video.ts | Implements the wait time logic by adjusting semaphore size and adding sleep between downloads |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| release(); | ||
| promQueued.dec(); | ||
| // Wait between downloads if downloadWaitTime is set | ||
| if (settings.downloadWaitTime > 0) { | ||
| await sleep(settings.downloadWaitTime * 1000); | ||
| } |
There was a problem hiding this comment.
The sleep is called after the semaphore is released, which creates a race condition. When the semaphore is released at line 156, another download can immediately acquire it and start downloading before the current download completes its sleep. This defeats the purpose of the wait time.
The sleep should be moved before the release() call to ensure the wait time is enforced between downloads. Move lines 158-161 to before line 156.
| release(); | |
| promQueued.dec(); | |
| // Wait between downloads if downloadWaitTime is set | |
| if (settings.downloadWaitTime > 0) { | |
| await sleep(settings.downloadWaitTime * 1000); | |
| } | |
| // Wait between downloads if downloadWaitTime is set | |
| if (settings.downloadWaitTime > 0) { | |
| await sleep(settings.downloadWaitTime * 1000); | |
| } | |
| release(); | |
| promQueued.dec(); |
|
|
||
| private static readonly MaxRetries = 1; | ||
| private static readonly DownloadThreads = 8; | ||
|
|
There was a problem hiding this comment.
The DownloadSemaphore is initialized at class load time using a static field, which means settings.downloadWaitTime is only read once when the class is first loaded. If the setting is changed at runtime (which appears to be supported by the @inrixia/db library with updateOnExternalChanges: true), the semaphore won't be reconfigured.
Consider either: (1) making this check dynamic rather than static initialization, or (2) documenting that changes to downloadWaitTime require a restart to take effect.
| // NOTE: DownloadSemaphore is intentionally initialized at class load time. | |
| // Changes to `settings.downloadWaitTime` at runtime (for example via | |
| // @inrixia/db with `updateOnExternalChanges: true`) will NOT reconfigure | |
| // this semaphore. A process restart is required for updates to take effect. |
|
I was having issues where (even if I set the config file to max videos: 2000 or something), it would download the first 5-10 videos, and then keep failing for the rest. This was on the UnRAID template, maybe it had something to do with this?. |
|
FYI max videos needs to be more like 9999 or you won't get everything. As for the rest that might be the Auth issue. Hard to say without the actual error. I'll leave this conversation here. But if you wanna open a issue with the error you were encountering that would be appreciated. |
2 participants
Controls how much time FPD should wait before downloading the next video. Could be useful for when downloading the entire backlog (that's what I'm doing right now!)