[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