Add option to keep unverified tracks. Fixes #612.

[leper]

Add a -u/--keep-unverified option that keeps the most recent attempt of ripping a track that failed verification.

The logging might be inconsistent in that the unverifiable track is mentioned as bein skipped in some log messages.

[github-actions]

💖 Thanks for opening your first pull request here! 💖

[MerlijnWajer]

Apologies for the delay. When you write:

The logging might be inconsistent in that the unverifiable track is mentioned as bein skipped in some log messages.

Can you confirm that the final rip log does contain the track and indicates that it wasn't verified?

[leper]

Apologies for the delay. When you write:

The logging might be inconsistent in that the unverifiable track is mentioned as bein skipped in some log messages.

Can you confirm that the final rip log does contain the track and indicates that it wasn't verified?
It lists the unverified tracks as skipped, mainly because AFAIR passing through another set of "not accurately ripped" files through some layers would have required some larger changes.

Sample log for one disc I used that on (which has some "copy protection", I just found this one quickly)

Tracks:
  1:
    Filename: <snip>/Death on the Road/Iron Maiden - Wildest Dreams.flac
    Peak level: 0.0
    Pre-emphasis:
    AccurateRip v1:
      Result: Track not present in AccurateRip database
    AccurateRip v2:
      Result: Track not present in AccurateRip database
    Status: Track not ripped (skipped)

but the (unverified) track still exists.

[MerlijnWajer]

Thanks for checking. I think we'll probably want the log to indicate something else if the final result is that the track is ripped, otherwise I think the log will be inconsistent with the result.

[leper]

Understandable, took a look and have some WIP (just pushed) that might just handle this better, but I still need to test this (most likely next week).

[leper]

Tested and squashed the previously WIP changes, it works nicely now.

Relevant parts of the log now look like the below

Tracks:
  1:
    Filename: <snip>/Death on the Road/Iron Maiden - Wildest Dreams.flac
    Peak level: 0.0
    Pre-emphasis:
    AccurateRip v1:
      Result: Track not present in AccurateRip database
    AccurateRip v2:
      Result: Track not present in AccurateRip database
    Status: Copy NOT OK (unverified file kept)
[...]
Conclusive status report:
  AccurateRip summary: None of the tracks are present in the AccurateRip database
  Health status: Some tracks could not be verified (but were kept)
  EOF: End of status report

There is a non-zero (6) exit code if any tracks were kept like this, given this is an accurate ripper.

[MerlijnWajer]

Thanks, this looks good.

[MerlijnWajer]

I am wondering about the exit code. If you pass the flag to keep the unverified tracks, I am not sure if exiting with a non-zero exit code makes sense...

[leper]

I am wondering about the exit code. If you pass the flag to keep the unverified tracks, I am not sure if exiting with a non-zero exit code makes sense...

That way you can still distinguish if the rip just worked without checking the logs, one of the discs that pushed me to write this did suddenly manage to get matching checksums for the one scratched track.

If you think that is not useful I can just remove that return.

[MerlijnWajer]

Happy to hear thoughts from others, but my inclination is not to exit with an error if the flag is passed - the user seems to want to accept keeping unverified tracks, so the presence of those shouldn't return in an error, I think.

[dseomn]

Happy to hear thoughts from others, but my inclination is not to exit with an error if the flag is passed - the user seems to want to accept keeping unverified tracks, so the presence of those shouldn't return in an error, I think.

I was a bit surprised by the non-zero exit code with --keep-going, which seems pretty comparable to this. Thinking about it a bit more, I don't have much of an opinion about which way is better. I would prefer that both --keep-going and this --keep-unverified behave similarly though, either both returning 0 or both returning (possibly different) non-zero values. If they return non-zero, it would also be nice to document what specific code they return.