feat: Add retry failed downloads feature with RETRY_FAILED_DOWNLOADS config (default: false), MAX_RETRY_ATTEMPTS config (default: 3) - #767#867
Conversation
…config (default: false), MAX_RETRY_ATTEMPTS config (default: 3) - alexta69#767
|
@alexta69 I raised new pr with updated master |
There was a problem hiding this comment.
Pull request overview
This PR implements automatic retry functionality for failed downloads with configurable maximum attempts. The feature is opt-in (disabled by default) and can be configured both globally via environment variables and per-download through the UI.
Key Changes
- Added two new configuration options:
RETRY_FAILED_DOWNLOADS(default: false) andMAX_RETRY_ATTEMPTS(default: 3) - Implemented automatic retry logic in the backend that re-queues failed downloads until max attempts are reached
- Added UI controls (checkbox toggle and number input) in the Advanced Options panel for per-download retry configuration
- Retry settings persist in browser cookies and are included in both individual and batch download requests
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| app/main.py | Added retry configuration defaults and parameters to the /add endpoint |
| app/ytdl.py | Extended DownloadInfo class with retry fields and implemented retry logic in _post_download_cleanup() |
| ui/src/app/app.ts | Added retry settings properties, cookie persistence, and validation logic |
| ui/src/app/app.html | Added UI controls for retry toggle and max attempts input field |
| ui/src/app/services/downloads.service.ts | Updated add() method signature to include retry parameters |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
app/main.py
Outdated
| playlist_item_limit = int(playlist_item_limit) | ||
| max_retry_attempts = int(max_retry_attempts) |
There was a problem hiding this comment.
Missing error handling for type conversion. If max_retry_attempts or playlist_item_limit contain values that cannot be converted to integers (e.g., non-numeric strings), the int() conversion will raise a ValueError, causing a 500 error instead of a proper 400 Bad Request response. Consider wrapping these conversions in a try-except block and returning an appropriate error message.
| playlist_item_limit = int(playlist_item_limit) | |
| max_retry_attempts = int(max_retry_attempts) | |
| try: | |
| playlist_item_limit = int(playlist_item_limit) | |
| max_retry_attempts = int(max_retry_attempts) | |
| except (TypeError, ValueError): | |
| log.error("Bad request: 'playlist_item_limit' and 'max_retry_attempts' must be integers") | |
| raise web.HTTPBadRequest(text="'playlist_item_limit' and 'max_retry_attempts' must be integers") |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Refactor download creation and retry logic for better clarity and maintainability.
|
now u can review @alexta69 |
|
Thanks @shiva-sai-824 . I just noticed now that the README isn't updated with the new configuration variables, can you add them under the Download Behavior section? And then we can merge the PR. |
|
I updated it can u check it and approve @alexta69 ? |
|
@alexta69 still any changes needed?? |
|
@shiva-sai-824 Yes, sorry for the delays, but there are so many demands on our time these days. One final thing, I hope. The "max retry attempts" field appears even when the toggle is off, and it appears on the next line after the toggle. Another option we have there is "split by chapters", and it behaves differently -- the input box only appears when the toggle is on, and on the same line. Let's align the behavior. I suggest copying the behavior from the "split by chapters" feature. |
|
Okay, I will try to implement it |
Summary
Implements automatic retry functionality for failed downloads with configurable maximum attempts, as requested in issue #767.
Backend Changes
Files:
app/main.py,app/ytdl.pyRETRY_FAILED_DOWNLOADSconfiguration option (default:'false')MAX_RETRY_ATTEMPTSconfiguration option (default:'3')retry_failed,max_retry_attempts, andretry_countfields toDownloadInfoclass/addendpoint to acceptretry_failedandmax_retry_attemptsparameters_post_download_cleanup()method:Frontend Changes
Files:
ui/src/app/app.ts,ui/src/app/app.html,ui/src/app/interfaces/download.ts,ui/src/app/services/downloads.service.tsmetube_retry_failed,metube_max_retry_attempts)DownloadsService.add()method to pass retry parameters to backendDownloadinterface to includeretry_failed,max_retry_attempts, andretry_countpropertiesKey Features
How It Works
retry_failedis enabled for that downloadTesting
Resolves #767