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

Brian C. Lane bcl at redhat.com
Wed Jan 29 17:42:57 UTC 2014


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.

> +            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.

> +        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.

> +
> +       :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.

> +    """
> +    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?

Looks ok other than those small things.

-- 
Brian C. Lane | Anaconda Team | IRC: bcl #anaconda | Port Orchard, WA (PST8PDT)


More information about the anaconda-patches mailing list