Submitting Infinite Retries on 0 option #507

Closed
wants to merge 7 commits into
from

Conversation

Projects
None yet
3 participants

Sepero commented Oct 31, 2012

Code and Documentation were modified accordingly. Anything neglected is unintentional.

Sepero added some commits Oct 31, 2012

@Sepero Sepero retries cannot be None, because a default was set
retries cannot be None, because a default of 10 was set in add_option
76d7235
@Sepero Sepero retries cannot be None, because a default was set c4cb885
@Sepero Sepero Allow for infinite retries with retries option 0
Allow for infinite retries if the -R retries option was set as 0
f01e218
@Sepero Sepero Change print output for infinite retries 148749e
@Sepero Sepero Edited code and documentation for infinite retries
Changed code to accept "0" or "inf" for infinite retries (like wget), and modified OptParse help documentation to reflect changes.
147054c
@Sepero Sepero Updated more documentation e6b2b3e

@FiloSottile FiloSottile commented on the diff Oct 31, 2012

youtube_dl/__init__.py
@@ -441,11 +441,11 @@ def _real_main():
if numeric_limit is None:
parser.error(u'invalid rate limit specified')
opts.ratelimit = numeric_limit
- if opts.retries is not None:
@FiloSottile

FiloSottile Oct 31, 2012

Collaborator

So, what's the behavior on opts.retries == None? At least it is not clear...

@Sepero

Sepero Oct 31, 2012

Sorry if that was not clear. I wrote in the commit for this edit that opt.retries should never be None because a default was set in the add_option().

@FiloSottile FiloSottile commented on the diff Oct 31, 2012

youtube_dl/__init__.py
@@ -188,7 +188,7 @@ def _find_term_columns():
general.add_option('-r', '--rate-limit',
dest='ratelimit', metavar='LIMIT', help='download rate limit (e.g. 50k or 44.6m)')
general.add_option('-R', '--retries',
- dest='retries', metavar='RETRIES', help='number of retries (default is %default)', default=10)
+ dest='retries', metavar='RETRIES', help='number of retries (default is %default). specify 0 or inf for infinite retries', default=10)
@FiloSottile

FiloSottile Oct 31, 2012

Collaborator

Capitalize S and add a anding dot. (Yes, I'm a pedant perfectionist ^^)

@Sepero

Sepero Oct 31, 2012

Very good. I wasn't sure what the preference would be on this.

@FiloSottile FiloSottile commented on the diff Oct 31, 2012

README.md
@@ -19,7 +19,8 @@ which means you can modify it, redistribute it or use it however you like.
-U, --update update this program to latest version
-i, --ignore-errors continue on download errors
-r, --rate-limit LIMIT download rate limit (e.g. 50k or 44.6m)
- -R, --retries RETRIES number of retries (default is 10)
+ -R, --retries RETRIES number of retries (default is 10). Specify 0 or inf
+ for infinite retries
@FiloSottile

FiloSottile Oct 31, 2012

Collaborator

README.md and relatives (bash completion and man page) are auto-generated by make

@Sepero

Sepero Oct 31, 2012

I have learned something new :)

@FiloSottile FiloSottile commented on the diff Oct 31, 2012

youtube_dl/FileDownloader.py
- if count > retries:
+ if retries != 0 and count > retries:
@FiloSottile

FiloSottile Oct 31, 2012

Collaborator

This should be

if count == retries:

since you changed the while condition to count < retries and since this will never be reached if the retries are infinite.

@Sepero

Sepero Oct 31, 2012

I should have seen that :)

@Sepero

Sepero Oct 31, 2012

Line 583 in FileDownloader.py could also be changed to:
while retries == 0 or count != retries:

@FiloSottile FiloSottile commented on the diff Oct 31, 2012

youtube_dl/FileDownloader.py
@@ -621,10 +621,9 @@ def _do_download(self, filename, info_dict):
break
# Retry
count += 1
- if count <= retries:
- self.report_retry(count, retries)
+ self.report_retry(count, retries if retries else "infinite")
@FiloSottile

FiloSottile Oct 31, 2012

Collaborator

A

if count == retries:

is needed here, otherwise on the last retry the program will say Retrying... and then exit.

@Sepero

Sepero Oct 31, 2012

Ah, good catch. I did not see that.

Collaborator

FiloSottile commented Oct 31, 2012

I'm +1 on this, LGTM with the annotated corrections.

Sorry for the number of comments ^^

Thanks!

Sepero commented Oct 31, 2012

Sorry for the number of comments ^^

No worries. Thanks for the correspondence. Maybe you have some ideas about making -R retries also apply to the video download process (instead of just the connection process).

phihag referenced this pull request Oct 31, 2012

Open

Retry on ERROR #506

@Sepero Sepero Modularization of login code
An attempt to modularize the method of logging in across different websites.
71bcf9e

phihag closed this in baeaeff Jan 25, 2015

Collaborator

phihag commented Jan 25, 2015

We can simply set retries to inf ;) - fixed in baeaeff

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