Hi Mark,
Thank you for your contribution.
I noticed that the indentation in your patch is not multiple of four.
You can check this by running `./setup.py pylint`.
On 31/03/18 17:12, Mark Hamzy wrote:
> If --location is an ftp url with a username and password
> then virt-install fails to install with an error:
> ERROR Error validating install location: Opening URL u failed: 530 Login
> incorrect..
> ---
> virtinst/urlfetcher.py | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/virtinst/urlfetcher.py b/virtinst/urlfetcher.py
> index da59a476..8bf24e08 100644
> --- a/virtinst/urlfetcher.py
> +++ b/virtinst/urlfetcher.py
> @@ -207,13 +207,20 @@ class _FTPURLFetcher(_URLFetcher):
> return
>
> try:
> - parsed = urllib.parse.urlparse(self.location)
> - self._ftp = ftplib.FTP()
> - self._ftp.connect(parsed.hostname, parsed.port or 0)
> - self._ftp.login()
> - # Force binary mode
> - self._ftp.voidcmd("TYPE I")
> - except Exception as e:
> + parsed = urllib.parse.urlparse(self.location)
> + self._ftp = ftplib.FTP()
> + from urllib2 import unquote
The |urllib2| module has been split across several modules in Python 3
named |urllib.request| and |urllib.error|
In this case, `urllib.request` has been imported in the beginning of the
file, so we can just use it with `urllib.request.unquote()`
Note that from the next release virt-manager will be Python 3 only.
> + parsed = urlparse.urlparse(self.location)
`parsed` has already been assigned value above, no need to do it again here.
Also the |urlparse| module was renamed to |urllib.parse| in Python 3.
> + username = parsed.username or ''
> + username = unquote(username).decode('utf8')
Here we can keep only the 2nd assignment and there is no need to use decode.
(See
https://docs.python.org/3/library/urllib.parse.html#urllib.parse.unquote)
For example:
username = urllib.request.unquote(parsed.username)
> + password = parsed.password or ''
> + password = unquote(password).decode('utf8')
Same here.
> + self._ftp = ftplib.FTP(parsed.hostname, username, password)
> + self._ftp.connect(parsed.hostname, parsed.port or 0)
There is no need to create a new instance of the FTP class here with
`ftplib.FTP(parsed.hostname, username, password)`.
The connect() call is sufficient to provide the values for username and
password.
> + self._ftp.login(username, password)
> + # Force binary mode
> + self._ftp.voidcmd("TYPE I")
> + except Exception as e:
> raise ValueError(_("Opening URL %s failed: %s.") %
> (self.location, str(e)))
>
>
Radostin
_______________________________________________
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
