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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list