<html>
<head>
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<font face="Helvetica, Arial, sans-serif">Hi Mark,<br>
<br>
Thank you for your contribution.<br>
<br>
I noticed that the indentation in your patch is not multiple of
four. You can check this by running `./setup.py pylint`.</font><br>
<br>
<div class="moz-cite-prefix">On 31/03/18 17:12, Mark Hamzy wrote:<br>
</div>
<blockquote type="cite"
cite="mid:OF726D15CF.E5762E68-ON00258261.0058FDF0-86258261.00590AE6@notes.na.collabserv.com">
<pre wrap="">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()</pre>
</blockquote>
<blockquote type="cite"
cite="mid:OF726D15CF.E5762E68-ON00258261.0058FDF0-86258261.00590AE6@notes.na.collabserv.com">
<pre wrap="">+ from urllib2 import unquote</pre>
</blockquote>
The <code>urllib2</code> module has been split across several
modules in Python 3 named <code>urllib.request</code> and <code>urllib.error</code><br>
In this case, `urllib.request` has been imported in the beginning of
the file, so we can just use it with `urllib.request.unquote()`<br>
<br>
Note that from the next release virt-manager will be Python 3 only.
<blockquote type="cite"
cite="mid:OF726D15CF.E5762E68-ON00258261.0058FDF0-86258261.00590AE6@notes.na.collabserv.com">
<pre wrap="">
+ parsed = urlparse.urlparse(self.location)</pre>
</blockquote>
`parsed` has already been assigned value above, no need to do it
again here.<br>
Also the <code>urlparse</code> module was renamed to <code>urllib.parse</code>
in Python 3.
<blockquote type="cite"
cite="mid:OF726D15CF.E5762E68-ON00258261.0058FDF0-86258261.00590AE6@notes.na.collabserv.com">
<pre wrap="">
+ username = parsed.username or ''
+ username = unquote(username).decode('utf8')</pre>
</blockquote>
Here we can keep only the 2nd assignment and there is no need to use
decode.<br>
(See
<a class="moz-txt-link-freetext" href="https://docs.python.org/3/library/urllib.parse.html#urllib.parse.unquote">https://docs.python.org/3/library/urllib.parse.html#urllib.parse.unquote</a>)<br>
<br>
For example:<br>
<pre wrap=""> username = urllib.request.unquote(parsed.username)</pre>
<blockquote type="cite"
cite="mid:OF726D15CF.E5762E68-ON00258261.0058FDF0-86258261.00590AE6@notes.na.collabserv.com">
<pre wrap="">
+ password = parsed.password or ''
+ password = unquote(password).decode('utf8')</pre>
</blockquote>
Same here.<br>
<blockquote type="cite"
cite="mid:OF726D15CF.E5762E68-ON00258261.0058FDF0-86258261.00590AE6@notes.na.collabserv.com">
<pre wrap="">
+ self._ftp = ftplib.FTP(parsed.hostname, username, password)
+ self._ftp.connect(parsed.hostname, parsed.port or 0)</pre>
</blockquote>
There is no need to create a new instance of the FTP class here with
`ftplib.FTP(parsed.hostname, username, password)`.<br>
The connect() call is sufficient to provide the values for username
and password.<br>
<blockquote type="cite"
cite="mid:OF726D15CF.E5762E68-ON00258261.0058FDF0-86258261.00590AE6@notes.na.collabserv.com">
<pre wrap="">
+ 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)))
</pre>
<br>
</blockquote>
<br>
Radostin<br>
</body>
</html>