[anaconda:rhel7/master] Handle --image arguments more thoroughly (#982164, #994488)

Anne Mulhern amulhern at redhat.com
Thu Jan 30 15:25:44 UTC 2014





----- Original Message -----
> From: "Brian C. Lane" <bcl at redhat.com>
> To: anaconda-patches at lists.fedorahosted.org
> Sent: Wednesday, January 29, 2014 12:42:57 PM
> Subject: Re: [anaconda:rhel7/master] Handle --image arguments more thoroughly	(#982164, #994488)
> 
> On Wed, Jan 29, 2014 at 12:15:29PM -0500, mulhern wrote:
> 
> [snip]
> 
> > +    image_count = 0
> > +    try:
> > +        disk_image_counter = 0
> > +        for (name, path) in name_path_pairs(opts.images):
> > +            log.info("naming disk image '%s' '%s'" % (path, name))
> 
> log parameters should be arguments now to keep pylint happy.
> 

Done!

> > +            anaconda.storage.config.diskImages[name] = path
> > +            image_count += 1
> > +            flags.imageInstall = True
> > +    except ValueError as e:
> > +        stdoutLog.error("error specifying image file: %s" % e)
> 
> Same here.

Ok!

> 
> > +        sys.exit(0)
> >  
> >      if image_count:
> >          anaconda.storage.setupDiskImages()
> > diff --git a/pyanaconda/anaconda_optparse.py
> > b/pyanaconda/anaconda_optparse.py
> > index 37f4b86..bcaab81 100644
> > --- a/pyanaconda/anaconda_optparse.py
> > +++ b/pyanaconda/anaconda_optparse.py
> > @@ -19,6 +19,9 @@
> >  # Authors:
> >  #   Will Woods <wwoods at redhat.com>
> >  
> > +import itertools
> > +import os
> > +
> >  from flags import BootArgs
> >  from optparse import OptionParser, OptionConflictError
> >  
> > @@ -124,3 +127,52 @@ class AnacondaOptionParser(OptionParser):
> >              values = self.get_default_values()
> >          v = self.parse_boot_cmdline(cmdline, values)
> >          return OptionParser.parse_args(self, args, v)
> > +
> > +def name_path_pairs(image_specs):
> > +    """Processes and verifies image file specifications. Generates pairs
> > +       of names and paths.
> > +
> > +       If no name given in specification, synthesizes name from basename
> > +       of path. Since two distinct paths may have the same basename,
> > handles
> > +       name collisions by synthesizing a different name for the colliding
> > +       name.
> 
> It would probably be helpful to have a brief summary of the
> <path>[:name] format of image_specs here in the docstring.
> 

Done!

> > +
> > +       :param image_specs: a list of image specifications
> > +       :type image_specs: list of str
> > +
> > +       Raises an exception if:
> > +         * A path is empty
> > +         * A path specifies a non-existant file
> > +         * A path specifies a directory
> > +         * Duplicate paths are specified
> > +         * A name contains a "/"
> 
> Sphinx docs has a :raises: tag that you can use.
> 

I've been avoiding the raises tag since the formatted documentation that results is bizarre.

> > +    """
> > +    image_specs = (spec.rsplit(":", 1) for spec in image_specs)
> > +    path_name_pairs = ((image_spec[0], image_spec[1].strip() if
> > len(image_spec) == 2 else None) for image_spec in image_specs)
> 
> This line is kinda hard to parse on first glance. Maybe break it up into
> multiple lines?
>

It's so much a single action that it's really hard to break it up further, I tried.

> Looks ok other than those small things.
> 
> --
> Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA
> (PST8PDT)
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> 


More information about the anaconda-patches mailing list