Skip to content

Feature/download client media eligibility - #2633

Open
niddelicious wants to merge 3 commits into
vavallee:mainfrom
niddelicious:feature/download-client-media-eligibility
Open

niddelicious wants to merge 3 commits into
vavallee:mainfrom
niddelicious:feature/download-client-media-eligibility

Conversation

@niddelicious

Copy link
Copy Markdown

Summary

Added functionality to make download clients eligible/non-eligible for different media type (determined by index search), making any grab filter out any clients not enabled for the grabbed type.
Priority field for download clients is enabled in frontend and after filtering any eligible client is processed in priority order.

Origin for this feature was my personal need to pass books to one Transmission instance and audiobooks to a different Transmission instance.

Checklist

  • Commits signed off with git commit -s — see Sign your work
  • Tests added or updated
  • docs/DEPLOYMENT.md updated if env vars, config, or upgrade path changed
  • Added a changelog fragment under changelog.d/ (not an edit to CHANGELOG.md) — see changelog.d/README.md
  • Wiki pages updated if user-facing behaviour changed

Test plan

  • make check (or the individual go test ./cmd/... ./internal/... and cd web && npm run build steps)

@github-actions github-actions Bot added the bindery-notified Discord notification already sent for this PR label Sep 16, 2026
@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

@niddelicious
niddelicious force-pushed the feature/download-client-media-eligibility branch from 25dd717 to ea77ce7 Compare September 16, 2026 11:30
Signed-off-by: niddelicious <github@nidde.nu>
Signed-off-by: niddelicious <github@nidde.nu>
Signed-off-by: niddelicious <github@nidde.nu>
@niddelicious
niddelicious force-pushed the feature/download-client-media-eligibility branch from ea77ce7 to 6fae8c9 Compare September 16, 2026 11:31

@vavallee vavallee left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, it's a nice feature and the routing itself is solid: both send sites filter, polling and removal still go by client id, the migration is safe, and the tests pass. A few things turned up in review that hit exactly the setups this is for, so I'd like to get them sorted before merging.

Needs fixing

  1. Readarr import creates clients that are eligible for nothing. internal/migrate/readarr.go around line 268 builds a models.DownloadClient without the two new flags, so they go in as false and every grab afterwards fails with "no enabled download client is eligible". Setting both to true there (or defaulting in repo.Create) fixes it.
  2. Approving a pending release sends audiobooks to the ebook client. internal/api/pending.go around line 180 uses the book's media type, which is both for a dual format book, and FilterEligibleForMediaType treats anything that isn't audiobook as books. The pending release row has its own MediaType; using that routes it correctly. That bug predates your PR, but the eligibility filter is what turns it into a misroute.
  3. SendWithFallback can send the same release to two clients. It moves on after any error, but several errors happen after the first client already accepted the download: Transmission returning id 0, qBittorrent with no hash, SABnzbd with no NZO id, or a timeout reading the qBittorrent response. The torrent then lands in both clients and the first copy is never tracked. I'd either drop the fallback (use the top ranked eligible client and report its error) or only fall through on errors that prove nothing was sent, like a connection refused before the request.

Worth doing here or as a follow up

  1. Auto-grab doesn't look at the other protocol. scheduler.go around line 1031 picks the best release across both protocols first, then gives up if that protocol has no eligible client. With SABnzbd set to ebooks only and qBittorrent to audiobooks only, a usenet audiobook that ranks best means the sweep logs "no eligible download client" and ignores the approved torrent, every sweep. Filtering candidates to protocols that have an eligible client before choosing would fix it. The manual grab error also tells you to "enable a download client for audiobooks" when one already exists for the other protocol.
  2. Grabs from the free-text Search page always count as books. Results there have no media type, so an m4b torrent found on /search goes to the ebook client. Book detail and Wanted are fine because they fall back to the book's type.
  3. Priority now sorts ahead of the category hint in RankClientsForMediaType. That's reasonable, but it can change which client an existing install uses, so a line in the changelog fragment would help.

Housekeeping

  • The red CodeQL check isn't you. Those three alerts are already open on main at the same lines; CodeQL blames this PR because the client URL input moved in download_clients.go.
  • 086 collides with other open PRs (#2607, #2626, #2628). Whichever lands later renumbers, so no action yet.
  • This will go into v1.37 rather than a patch release.

Happy to help with any of these if you get stuck.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bindery-notified Discord notification already sent for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants