Stop cleanly on Ctrl-C, and ask which albums to take from an artist - #1025
Open
sjbrownrigg wants to merge 2 commits into
Open
Stop cleanly on Ctrl-C, and ask which albums to take from an artist#1025sjbrownrigg wants to merge 2 commits into
sjbrownrigg wants to merge 2 commits into
Conversation
Ctrl-C did not stop a download. Measured on a 16-track album: SIGINT was ignored, the process was still running ten seconds later and had to be killed outright -- which skips Main.__aexit__, so the __artwork directories it would have removed were left behind. That is where large numbers of stray __artwork directories in a library come from. Cancel the running task from a SIGINT handler instead, so the `async with Main(...)` blocks unwind normally and clean up after themselves. The process now exits promptly, __artwork is removed, and the message says what happened rather than showing a KeyboardInterrupt traceback. Partly-downloaded tracks are left behind for the next run to continue, so interrupting mid-album loses nothing: re-running the same command finishes it. Verified end to end -- interrupt a 16-track album, re-run, finish with 16 valid FLACs and no leftovers.
sjbrownrigg
force-pushed
the
contrib/graceful-exit-and-artist-select
branch
6 times, most recently
from
August 17, 2026 18:12
78d53c7 to
c97f78a
Compare
An artist url downloaded the entire discography with no chance to look
at it first. Artist search is unreliable on both Qobuz and Tidal, so the
url can easily be the wrong person entirely, and even the right one
brings a lot nobody asked for -- the only way to stop it was Ctrl-C.
Show the discography in the same scrolling multi-select menu `rip
search` already uses, with the same SPACE/ENTER/ESC keys and the same
preview pane:
The Beatles - 88 releases
SPACE - select, ENTER - download, ESC - exit
1. 1963 Please Please Me (Remastered) [14 trk] lossless
2. 1963 With The Beatles (Remastered) [14 trk] lossless
...
86. 2025 In My Life (Take 1) (single) [1 trk] hi-res
To do that the menu moves into utils/menu.py and both callers use it, so
the Windows `pick` fallback is no longer implemented only for search.
The listing costs no extra requests: the artist response already carries
the full album objects and ArtistMetadata was keeping only their ids. It
now keeps the AlbumSummary that search_results already builds, which
reads every source's field names and folds `version` into the name.
Sorted, since sources return a discography in their own order: by
release type then date, or as one chronological list. Set with
cli.artist_album_sort, overridden for a run by `rip url --sort`. The
config field is defaulted so older configs still load.
Release type is only used where the source reports it. Tidal does;
Qobuz leaves release_type null on every album, and guessing from the
track count would put wrong labels on screen, since a four-track release
is not reliably an EP. Unknown entries are unlabelled and sort with the
albums, so "type" degrades to chronological rather than inventing
groupings.
AlbumSummary also gains an optional `quality`, defaulting to empty so
existing construction is unaffected. Tidal lists the same release twice,
once lossless and once hi-res, which is otherwise impossible to tell
apart in a list. Search results show it in their preview pane too.
And stop doubling the edition in the name: the version was appended even
when the title already contained it, giving "Please Please Me
(Remastered) (Remastered)". That affected `rip search` results as well.
Only prompts when stdin is a terminal, so scripts and cron are
unaffected, and `rip url --no-confirm` skips it.
sjbrownrigg
force-pushed
the
contrib/graceful-exit-and-artist-select
branch
from
August 17, 2026 18:13
c97f78a to
43c2966
Compare
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.
Two related fixes for the same complaint: an artist url downloads the whole discography, and there is no good way to stop it.
Ctrl-C did not stop a download
Measured on a 16-track album: SIGINT was ignored, the process was still running ten seconds later, and it had to be killed outright.
That matters beyond the annoyance, because force-killing skips
Main.__aexit__and thereforeremove_artwork_tempdirs(). Every interrupted run leaves an__artworkdirectory behind in the album folder. I found 448 of them in one library, and interrupted artist downloads are where they came from.Cancelling the running task from a SIGINT handler lets the
async with Main(...)blocks unwind normally, so cleanup runs:and the user gets a message rather than a
KeyboardInterrupttraceback.Artist downloads are all-or-nothing
Artist search is unreliable on both Qobuz and Tidal, so an artist url is easily the wrong person entirely — and even the right one brings a lot nobody asked for. There was no way to look before committing to it.
This shows the discography in the same scrolling multi-select menu
rip searchalready uses, with the same SPACE/ENTER/ESC keys and the same preview pane:To do that the menu moves into
utils/menu.pyand both callers use it, so the Windowspickfallback is no longer implemented only for search.The listing costs no extra requests. The artist response already carries the full album objects and
ArtistMetadatawas keeping only their ids. It now keeps theAlbumSummarythatsearch_resultsalready builds, which reads every source's field names (tracks_count/nb_tracks/numberOfTracks, and the various date fields) and foldsversioninto the name.Sorting
Sources return a discography in their own order, which is not much use for eighty-odd releases. Sorted either by release type then date — albums, then EPs, then singles, each oldest first — or as one chronological list. Set with
cli.artist_album_sort, overridden for a single run byrip url --sort. The config field is defaulted, so configs written before this still load.Release type is only used where the source reports it. Tidal does; Qobuz leaves
release_typenull on every album, and guessing from the track count would put wrong labels on screen — a four-track release is not reliably an EP. Unknown entries are unlabelled and sort with the albums, sotypedegrades to plain chronological order rather than inventing groupings.Two fixes to AlbumSummary along the way
It gains an optional
quality, defaulting to empty so existing construction is unaffected. Tidal lists the same release twice, once lossless and once hi-res, which is otherwise impossible to tell apart in a list. Search results show it in their preview pane for the same reason.And it no longer doubles the edition in the name. The version was appended even when the title already contained it:
That affected
rip searchresults too, not just this menu.The artist menu builds its own entry lines rather than using
summarize(), since every row there has the same artist and the useful columns are year, track count, kind and quality.It only prompts when
stdinis a terminal, so scripts and cron are unaffected, andrip url --no-confirmskips it for unattended runs.Testing
Interrupt behaviour verified end to end on a real 16-track album: interrupt, confirm the
__artworkdirectory was cleaned up, re-run, finish with 16 files that all decode.Album listing checked against a real 88-release artist — entries and preview pane built from live metadata, with no additional requests beyond the artist call that was already being made.
Both gates verified: no prompt when
stdinis not a terminal, and none with--no-confirm.AlbumSummaryback-compatibility checked: five-argument construction still works and yields an emptyquality; Qobuz items give24bit/96kHz, Tidal items givehi-res/lossless.Both commits compile standalone.
pytest tests/is unchanged at 59 passed / 1 failed, and that failure (test_meta.py::test_album_metadata_qobuz, a genre assertion) also fails on an unmodifieddev.I have not driven the menu itself programmatically —
simple_term_menuneeds a real tty — so the interactive behaviour is the part most worth a second pair of eyes.Note
While investigating the interrupt I also hit
KeyError: 'body'fromlatest_streamrip_version()surfacing as "Task exception was never retrieved". #995 already fixes that, and more thoroughly than I had, so I have left it out of this branch.