Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Whipper gives up even if 5th rip attempt is successful #449

Closed
the-confessor opened this issue Jan 12, 2020 · 3 comments
Closed

Whipper gives up even if 5th rip attempt is successful #449

the-confessor opened this issue Jan 12, 2020 · 3 comments
Labels
Accepted Accepted issue on our roadmap Bug Generic bug: can be used together with more specific labels Priority: low Low priority
Milestone

Comments

@the-confessor
Copy link

I haven't run into this condition yet, but I noticed this looking at the code.

It appears that if the first 4 attempts to rip a track fail, then the 5th attempt succeeds, Whipper would abort the operation anyway.

From cd.py:

                if tries == MAX_TRIES:
                    logger.critical('giving up on track %d after %d times',
                                    number, tries)
                    raise RuntimeError(
                        "track can't be ripped. "
                        "Rip attempts number is equal to 'MAX_TRIES'")

Should the condition actually be tries >= MAX_TRIES?

@ABCbum
Copy link
Contributor

ABCbum commented Jan 12, 2020

Yeah, i think just move the block

if tries == MAX_TRIES:
  ......

in to the while loop before it should solve the problem since if the rip attempt is successful then it will break the loop right away. Right ?

@the-confessor
Copy link
Author

Yes something like that would also work. As long as it doesn't give up when the most recent attempt to rip the track is successful.

@JoeLametta JoeLametta changed the title Will Whipper give up if 5th rip attempt is successful? Whipper gives up even if 5th rip attempt is successful Jan 14, 2020
@JoeLametta JoeLametta added Accepted Accepted issue on our roadmap Bug Generic bug: can be used together with more specific labels Priority: low Low priority labels Jan 14, 2020
@JoeLametta JoeLametta added this to the 1.0 milestone Jan 14, 2020
@JoeLametta JoeLametta removed Accepted Accepted issue on our roadmap Bug Generic bug: can be used together with more specific labels Priority: low Low priority labels Jan 14, 2020
@JoeLametta JoeLametta removed this from the 1.0 milestone Jan 14, 2020
@JoeLametta JoeLametta changed the title Whipper gives up even if 5th rip attempt is successful Will Whipper give up if 5th rip attempt is successful? Jan 14, 2020
@JoeLametta JoeLametta added this to the 1.0 milestone Jan 14, 2020
@JoeLametta JoeLametta added Accepted Accepted issue on our roadmap Bug Generic bug: can be used together with more specific labels Priority: low Low priority labels Jan 14, 2020
@JoeLametta JoeLametta changed the title Will Whipper give up if 5th rip attempt is successful? Whipper gives up even if 5th rip attempt is successful Jan 14, 2020
@JoeLametta
Copy link
Collaborator

@the-confessor Thanks for reporting this issue.
The bug should have been fixed in commit caa2c8b.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Bug Generic bug: can be used together with more specific labels Priority: low Low priority
Projects
None yet
Development

No branches or pull requests

3 participants