[virt-tools-list] [virt-manager PATCH] virtinst: early detect ftp connection errors

[Cole Robinson]

On 04/14/2014 09:11 AM, Giuseppe Scrivano wrote:
> It fixes two problems:
> 
> i) "ftp://" was accepted as valid URL but then it causes this
> exception:
> 
> Traceback (most recent call last):
>   File "<stdin>", line 1, in <module>
>   File "/usr/lib64/python2.7/ftplib.py", line 387, in login
>     resp = self.sendcmd('USER ' + user)
>   File "/usr/lib64/python2.7/ftplib.py", line 243, in sendcmd
>     self.putcmd(cmd)
>   File "/usr/lib64/python2.7/ftplib.py", line 178, in putcmd
>     self.putline(line)
>   File "/usr/lib64/python2.7/ftplib.py", line 173, in putline
>     self.sock.sendall(line)
> AttributeError: 'NoneType' object has no attribute 'sendall'
> 
> ii) only a cryptic error message "Unable to complete install: '[Errno
> -2] Name or service not known'" was showed to users when the DNS
> lookup failed.  The exception is now intercepted and decorated with
> more information.
> 
> Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1086554
> 
> Signed-off-by: Giuseppe Scrivano <gscrivan at redhat.com>
> ---
>  virtinst/urlfetcher.py | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/virtinst/urlfetcher.py b/virtinst/urlfetcher.py
> index 7075929..3f2744b 100644
> --- a/virtinst/urlfetcher.py
> +++ b/virtinst/urlfetcher.py
> @@ -151,9 +151,16 @@ class _FTPImageFetcher(_URIImageFetcher):
>          self.ftp = None
>  
>      def prepareLocation(self):
> -        url = urlparse.urlparse(self._make_path(""))
> -        self.ftp = ftplib.FTP(url[1])
> -        self.ftp.login()
> +        try:
> +            url = urlparse.urlparse(self._make_path(""))
> +            if not url[1]:
> +                raise ValueError(_("Invalid install location"))
> +            self.ftp = ftplib.FTP(url[1])
> +            self.ftp.login()
> +        except Exception, e:
> +            raise ValueError(_("Opening URL %s failed: %s.") %
> +                              (self.location, str(e)))
> +
>  
>      def hasFile(self, filename):
>          path = self._make_path(filename)
> 

ACK

- Cole