----- Original Message -----
From: "Brian C. Lane" <bcl(a)redhat.com>
To: anaconda-patches(a)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(a)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(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches