[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH] Add --root-device to upgrade command



> I don't fully understand the reasoning behind this because the new 
> features are backwards compatible. Anyway I've moved them into a separate 
> class and updated the control map.

The reason you want a completely new class is because FC3-F10 did not
support this option.  We don't want to recognize it as valid when we
parse kickstart versions for those releases.  If someone's doing a RHEL5
install test and uses a kickstart file they just copied from their
rawhide system, we want to make sure we give them an error.

> I only don't understand what's the purpose of:
>
> removedKeywords = KickstartCommand.removedKeywords
> removedAttrs = KickstartCommand.removedAttrs
>
>
> in FC3_Upgrade (and other classes) and not sure if I need it in the new class.

James explained these already.  While you don't need them since we've
never removed any attributes or options from the upgrade command, I like
to see them in each class so that all objects have these attributes.
That way the BaseCommand and BaseData objects can just assume they're
present.

>  pykickstart/commands/upgrade.py |   33 +++++++++++++++++++++++++++++++++
>  pykickstart/handlers/control.py |    4 ++--
>  2 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/pykickstart/commands/upgrade.py b/pykickstart/commands/upgrade.py
> index b7dba86..5fbf93a 100644
> --- a/pykickstart/commands/upgrade.py
> +++ b/pykickstart/commands/upgrade.py
> @@ -49,3 +49,36 @@ class FC3_Upgrade(KickstartCommand):
>             self.upgrade = True
>          else:
>             self.upgrade = False
> +
> +class F11_Upgrade(FC3_Upgrade):

Please do add:

removedKeywords = FC3_Upgrade.removedKeywords
removedAttrs = FC3_Upgrade.removedAttrs

here as class attributes.  Subclasses do not inherit class attributes of
their superclass (at least, they didn't in my testing) so we have to set
these explicitly.

> +    def __init__(self, writePriority=0, *args, **kwargs):
> +        FC3_Upgrade.__init__(self, writePriority, *args, **kwargs)
> +
> +        self.op = self._getParser()
> +        self.root_device = kwargs.get("root_device", None)
> +
> +    def __str__(self):
> +        if self.upgrade and (self.root_device is not None):
> +            retval="# Upgrade existing installation\nupgrade --root-device=%s\n" % self.root_device
> +        else:
> +            retval=FC3_Upgrade.__str(self)__

You meant retval = FC3_Upgrade.__str__(self) here, I'm sure.

> +
> +        return retval
> +
> +    def _getParser(self):
> +        op = KSOptionParser(lineno=self.lineno)
> +        op.add_option("--root-device", dest="root_device")
> +        return op

Instead of calling KSOptionParser, call FC3_Upgrade._getParser(self)
here to pull in whatever arguments FC3_Upgrade's KSOptionParser object
may support.  I know it's none in this case, but that's how it works in
every other class in pykickstart and I really like things to be
consistent.

Everything else looks fine to me.

- Chris


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]