[Kimchi-devel] [PATCH 3/4] Host's software update: Update backend.

[Crístian Viana]

Am 27-01-2014 14:34, schrieb Paulo Vital:
> +    def swupdate_get_list(self):
> +        return [pkg for pkg in self.host_swupdate.getUpdates().keys()]
> +
> +    def swupdate_lookup(self, name):
> +        return self.host_swupdate.getUpdate(name)
> +
> +    def swupdate_update(self):
> +        kimchi_log.info('Host is going to be updated.')
> +        self.host_swupdate.doUpdate()
> +
>   class MockVMTemplate(VMTemplate):
Please use two blank lines between top-level elements.
> +    def softwareupdate_lookup(self, *name):
> +        return self.host_swupdate.getUpdates()
What's the point of having a function "lookup(name)" with an unused 
variable? If that's a lookup, I'd expect the function to iterate over 
the loop and return the software update if it's been found. That looks 
more like a "get_list" implementation.

And aren't the functions in "model.py" and "mockmodel.py" supposed to 
have the same name? I see a sequence of "softwareupdate_*" and 
"swupdate_*" in both files.
> +
> +    def softwareupdate_update(self, args=None):
> +        kimchi_log.info('Host is going to be updated.')
> +        self.host_swupdate.doUpdate()
>
>   class LibvirtVMTemplate(VMTemplate):
Please use two blank lines between top-level elements.
> +from kimchi.exception import *
According to the PEP8, we should avoid using wildcard imports like that.
> +from kimchi.utils import kimchi_log
> +
Please use two blank lines between top-level elements.
> +YUM_DISTROS = [ 'fedora', 'red hat enterprise linux',
> +                'red hat enterprise linux server']
> +APT_DISTROS = [ 'debian', 'ubuntu']
> +ZYPPER_DISTROS = [ 'opensuse ' ]
> +
> +class SoftwareUpdate(object):
Please use two blank lines between top-level elements.
> +            kimchi_log.info("There is no compatible package manager for \
> +                             this system.")
The string above will be displayed as:

"There is no compatible package manager for                              
this system."

Wrap long strings like this:

kimchi_log.info("There is no compatible package manager for "
                 "this system.")


> +        return self._packages[name]
> +
> +
> +    def getNumOfUpdates(self):
Please use only one blank line here, as they're not top-level elements.
> +        return self._pkg_mnger.update()
> +
> +class YumUpdate(object):
Please use two blank lines between top-level elements.
> +            raise OperationFailed("ERROR when executing command: %s" % stderr)
> +
> +class AptUpdate(object):
Please use two blank lines between top-level elements.
> +            raise OperationFailed("ERROR when executing command: %s" % e)
> +
> +class ZypperUpdate(object):
Please use two blank lines between top-level elements.