[libvirt] [sandbox PATCH v4 07/21] Image: Add download function

[Daniel P. Berrange]

On Fri, Aug 28, 2015 at 01:47:35PM +0000, Eren Yagdiran wrote:
> Refactor download function from virt-sandbox-image to use
> the newly introduced Source abstract class. The docker-specific
> download code is moved to a new DockerSource class.
> ---
>  virt-sandbox-image/Makefile.am             |   1 +
>  virt-sandbox-image/sources/DockerSource.py | 214 +++++++++++++++++++++++++++++
>  virt-sandbox-image/sources/Source.py       |   4 +
>  virt-sandbox-image/virt-sandbox-image.py   | 204 +++++----------------------
>  4 files changed, 251 insertions(+), 172 deletions(-)
>  create mode 100644 virt-sandbox-image/sources/DockerSource.py
> diff --git a/virt-sandbox-image/sources/DockerSource.py b/virt-sandbox-image/sources/DockerSource.py
> new file mode 100644
> index 0000000..f3cf5f3
> --- /dev/null
> +++ b/virt-sandbox-image/sources/DockerSource.py
> +
> +    def download_template(self,**args):
> +        name = args['name']
> +        registry = args['registry'] if args['registry'] is not None else self.default_index_server
> +        username = args['username']
> +        password = args['password']
> +        templatedir = args['templatedir']
> +        self._download_template(name,registry,username,password,templatedir)

Using '**args' is a bit of a wierd way todo named parameters in python. We should
jsut list the named parameters explicitly as you've done in the folloiwing
_download_template method. In fact we don't need separate download_template
and _download_template methods at all.

> +    def _download_template(self,name, server,username,password,destdir):
> +

> diff --git a/virt-sandbox-image/sources/Source.py b/virt-sandbox-image/sources/Source.py
> index 508ca80..8751689 100644
> --- a/virt-sandbox-image/sources/Source.py
> +++ b/virt-sandbox-image/sources/Source.py
> @@ -25,3 +25,7 @@ class Source():
>      __metaclass__ = ABCMeta
>      def __init__(self):
>          pass
> +
> +    @abstractmethod
> +    def download_template(self,**args):
> +        pass

We actually want to list the named parameters here, as the base class
is defining the API contract that the subclass must satisfy. It is
also worth having python docs inline.



Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|