[PATCH] Properly retry package downloads (#924860)
Martin Kolman
mkolman at redhat.com
Tue Feb 11 09:50:13 UTC 2014
On Mon, 2014-02-10 at 16:15 -0800, Brian C. Lane wrote:
> On Thu, Feb 06, 2014 at 06:09:22PM +0100, Martin Kolman wrote:
> > There are basically two cases where package download can fail:
> > - when YUM is populating the transaction
> > (this actually just checks the file exists)
> > - when the transaction is started and packages are downloaded and
> > installed
> >
> > Previously Anaconda did not attempt to retry the first one at all
> > and made an unlimited number of attempts in the second one.
> >
> > This has been changed and Anaconda now does 3 retries separated
> > by 3 seconds of waiting in both cases.
> >
> > Signed-off-by: Martin Kolman <mkolman at redhat.com>
> > ---
> > scripts/anaconda-yum | 63 ++++++++++++++++++++++++++++++++++++++++++----------
> > 1 file changed, 51 insertions(+), 12 deletions(-)
> >
> > diff --git a/scripts/anaconda-yum b/scripts/anaconda-yum
> > index 12a562f..e714d98 100755
> > --- a/scripts/anaconda-yum
> > +++ b/scripts/anaconda-yum
> > @@ -22,6 +22,7 @@ import logging
> > import os
> > import sys
> > import argparse
> > +import time
> > import rpm
> > import rpmUtils
> > import yum
> > @@ -29,6 +30,9 @@ from urlgrabber.grabber import URLGrabError
> >
> > YUM_PLUGINS = ["fastestmirror", "langpacks"]
> >
> > +MAX_DOWNLOAD_RETRIES = 3
> > +DOWNLOAD_RETRY_DELAY = 3 # in seconds
> > +
> > def setup_parser():
> > """ Setup argparse with supported arguments
> >
> > @@ -109,12 +113,28 @@ def run_yum_transaction(release, arch, yum_conf, install_root, ts_file, script_l
> > yb.ts.ts.setColor(3)
> >
> > print("DEBUG: populate transaction set")
> > - try:
> > - # uses dsCallback.transactionPopulation
> > - yb.populateTs(keepold=0)
> > - except RepoError as e:
> > - print("ERROR: error populating transaction: %s" % e)
> > - print("QUIT:")
> > + retry_count = 0
> > + populated_successfully = False
> > + while retry_count <= MAX_DOWNLOAD_RETRIES:
> > + try:
> > + if retry_count:
> > + print("DEBUG: error populating transaction, retrying (%d/%d)"
> > + % (retry_count, MAX_DOWNLOAD_RETRIES))
> > + # uses dsCallback.transactionPopulation
> > + yb.populateTs(keepold=0)
> > + populated_successfully = True
> > + break
> > + except RepoError as e:
> > + # retry after waiting a bit
> > + time.sleep(DOWNLOAD_RETRY_DELAY)
> > + finally:
> > + retry_count += 1
> > +
> > + if not populated_successfully:
> > + print("ERROR: error populating transaction after %d retries: %s"
> > + % (retry_count-1, e))
> > + # we don't need to print "QUIT:" there, the finally clause
> > + # of the toplevel try-block will do that for us
>
>
> I think a for/else loop could make this easier to read:
>
> for retry_count in xrange(1, MAX_DOWNLOAD_RETRIES+1):
> try:
> yb.populateTs(keepold=0)
> break
> except RepoError as e:
> time.sleep(DOWNLOAD_RETRY_DELAY)
> print("DEBUG: error populating transaction, retrying (%d/%d)"
> % (retry_count, MAX_DOWNLOAD_RETRIES))
> else:
> print("ERROR: error populating transaction after %d retries: %s"
> % (MAX_DOWNLOAD_RETRIES, e))
>
Oh right, that indeed looks nicer! :) I've been also kinda worried not
to make the while loop go forever (even though the counter is increased
in a finally block), but using a for loop gives it an explicit upper
bound, solving this issue.
More information about the anaconda-patches
mailing list