feat(downloaders): Add WebUI API key support for qBittorrent - #2860
Open
hymccord wants to merge 1 commit into
Open
feat(downloaders): Add WebUI API key support for qBittorrent#2860hymccord wants to merge 1 commit into
hymccord wants to merge 1 commit into
Conversation
Contributor
Reviewer's GuideAdd optional per-instance qBittorrent WebUI API key support to the mover tuning start/end scripts and configuration, with validation and argument wiring to pass --api-key to mover.py when configured, while preserving legacy username/password authentication and updating script/config version metadata. Sequence diagram for mover.py authentication selection with optional WebUI API keysequenceDiagram
actor Admin
participant mover_tuning_start_sh as mover-tuning-start.sh
participant process_qbit_instance
participant mover_py as mover.py
Admin->>mover_tuning_start_sh: main()
mover_tuning_start_sh->>process_qbit_instance: process_qbit_instance(INSTANCE_NAME, INSTANCE_HOST, INSTANCE_USER, INSTANCE_PASSWORD, INSTANCE_API_KEY, INSTANCE_CA_BUNDLE)
alt [INSTANCE_API_KEY non-empty]
process_qbit_instance->>mover_py: python_cmd MOVER_SCRIPT --api-key INSTANCE_API_KEY
else [INSTANCE_API_KEY empty]
process_qbit_instance->>mover_py: python_cmd MOVER_SCRIPT --user INSTANCE_USER --password INSTANCE_PASSWORD
end
mover_py-->>process_qbit_instance: stdout/stderr
process_qbit_instance-->>mover_tuning_start_sh: log()/notify()
Flow diagram for qBittorrent authentication precedence in mover-tuning scriptsflowchart TD
A["process_qbit_instance start"] --> B{api_key non-empty?}
B -->|Yes| C["mover_args += --api-key api_key"]
B -->|No| D["mover_args += --user user --password password"]
C --> E["optional: mover_args += --ca-bundle ca_bundle"]
D --> E
E --> F["python_cmd MOVER_SCRIPT mover_args"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
mover-tuning-end.sh, the refactor tomover_argsdropped the explicit--resumeflag that was previously passed tomover.py; if this flag is still required to resume torrents, it should be added tomover_argsto preserve behavior. - The config comment says
API_KEYS"override username/password authentication if provided", but both scripts still always pass--user/--passwordalongside--api-key; consider either omitting credentials when an API key is set or adjusting the comment to reflect the actual behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `mover-tuning-end.sh`, the refactor to `mover_args` dropped the explicit `--resume` flag that was previously passed to `mover.py`; if this flag is still required to resume torrents, it should be added to `mover_args` to preserve behavior.
- The config comment says `API_KEYS` "override username/password authentication if provided", but both scripts still always pass `--user`/`--password` alongside `--api-key`; consider either omitting credentials when an API key is set or adjusting the comment to reflect the actual behavior.
## Individual Comments
### Comment 1
<location path="includes/downloaders/mover-tuning-end.sh" line_range="458-465" />
<code_context>
fi
- local ca_bundle_args=()
+ local mover_args=(
+ --pause
+ --host "$host"
+ --user "$user"
+ --password "$password"
+ --cache-mount "$CACHE_MOUNT"
+ --days_from "$DAYS_FROM"
+ --days_to "$DAYS_TO"
+ )
+
</code_context>
<issue_to_address>
**issue (bug_risk):** End script is still using `--pause` and `--cache-mount`, and never passes `--resume` to `mover.py`.
Here you’re still passing `--pause` and `--cache-mount` and never `--resume`. The end script used to call `mover.py` with `--resume` (and without `--cache-mount`), which matches its role of resuming torrents. This looks like a copy from the start script and will likely invert the behavior. Please switch `--pause` to `--resume` here and drop any flags not needed for the resume flow.
</issue_to_address>
### Comment 2
<location path="includes/downloaders/mover-tuning.cfg" line_range="22-27" />
<code_context>
# qBittorrent instances
# Supports unlimited qBittorrent instances
-# Arrays NAMES, HOSTS, USERS and PASSWORDS must all have the same length
+# Arrays NAMES, HOSTS, USERS, PASSWORDS and API_KEYS must all have the same length
# NAMES array is optional - if omitted, instances will be named "qBit-Instance-1", "qBit-Instance-2", etc.
+# API_KEYS array is optional but will override username/password authentication if provided
readonly NAMES=("qBit-Movies" "qBit-TV" "qBit-Music") # qBittorrent instance names
readonly HOSTS=("192.168.2.200:8088" "192.168.2.200:8811" "192.168.2.200:8822") # qBittorrent host:port
</code_context>
<issue_to_address>
**suggestion:** Config comments about API_KEYS length are slightly misleading versus the actual validation logic.
The updated comment says all arrays, including `API_KEYS`, must have the same length, but the scripts treat `API_KEYS` as optional and only validate its length when it’s present and non-empty. Please adjust the comment to state that `API_KEYS` is optional and only needs to match the other arrays when defined, to better reflect the actual behavior and avoid config confusion.
```suggestion
# qBittorrent instances
# Supports unlimited qBittorrent instances
# Arrays NAMES, HOSTS, USERS and PASSWORDS must all have the same length; API_KEYS is optional and only needs to match when defined
# NAMES array is optional - if omitted, instances will be named "qBit-Instance-1", "qBit-Instance-2", etc.
# API_KEYS array is optional but will override username/password authentication if provided
readonly NAMES=("qBit-Movies" "qBit-TV" "qBit-Music") # qBittorrent instance names
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
Deploying with ⚡ Cloudflare Pages
|
There was a problem hiding this comment.
Pull request overview
Adds optional per-instance qBittorrent WebUI API key support to the mover-tuning configuration and start/end scripts, enabling API-key authentication (qBittorrent 5.2.0+) alongside the existing username/password flow.
Changes:
- Add
API_KEYSarray support inmover-tuning.cfg, and legacyQBIT_API_KEY_1/2support in the scripts. - Validate
API_KEYSlength (when present) matchesHOSTS. - Pass
--api-keythrough tomover.pywhen configured.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| includes/downloaders/mover-tuning.cfg | Bumps config version and documents/adds the API_KEYS array example. |
| includes/downloaders/mover-tuning-start.sh | Adds API key handling, validation, and forwards --api-key to mover.py for pause phase. |
| includes/downloaders/mover-tuning-end.sh | Adds API key handling, validation, and forwards --api-key to mover.py for resume phase. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Author
|
I did change the Feedback appreciated. Tested on my instance of unRaid with the updated |
hymccord
force-pushed
the
feat/downloaders-qbit-allow-webapi-key
branch
from
August 18, 2026 01:55
8dfae7e to
079a88d
Compare
Contributor
|
Deploying with ⚡ Cloudflare Pages
|
hymccord
force-pushed
the
feat/downloaders-qbit-allow-webapi-key
branch
from
August 18, 2026 14:25
079a88d to
acb512c
Compare
Author
|
@sourcery-ai review |
Contributor
|
Deploying with ⚡ Cloudflare Pages
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
process_qbit_instancefor both start and end scripts, you now rely onapi_keybeing non-empty to decide auth mode; consider explicitly handling the case where both API key and user/password are unset to fail fast with a clear error rather than relying on qBittorrent/mover.py behavior. - The config header comments now state that NAMES, API_KEYS and CA_BUNDLES must match HOSTS length, but
validate_configonly enforces that for API_KEYS and CA_BUNDLES; if NAMES mismatch should be allowed, reword the comment for clarity, otherwise add a length check for NAMES in start.sh to mirror end.sh.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `process_qbit_instance` for both start and end scripts, you now rely on `api_key` being non-empty to decide auth mode; consider explicitly handling the case where both API key and user/password are unset to fail fast with a clear error rather than relying on qBittorrent/mover.py behavior.
- The config header comments now state that NAMES, API_KEYS and CA_BUNDLES must match HOSTS length, but `validate_config` only enforces that for API_KEYS and CA_BUNDLES; if NAMES mismatch should be allowed, reword the comment for clarity, otherwise add a length check for NAMES in start.sh to mirror end.sh.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
@bz could you take a look at this one? |
This file contains hidden or 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
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.
Pull Request
Purpose
Adds optional WebUI API key support for qBittorrent mover tuning
Approach
API_KEYSarray support alongside existingNAMES,HOSTS,USERS,PASSWORDSandCA_BUNDLESarrays.API_KEYS, when configured, matches the number ofHOSTS.--api-keytomover.pyonly when an API key is configured for that instanceThis depends on
mover.pysupporting the new--api-keyoption.Open Questions and Pre-Merge TODOs
Requirements
Summary by Sourcery
Support qBittorrent WebUI API key authentication across the cache mover start and end workflows.
New Features:
Enhancements:
Documentation:
Chores: