10:08 p.m.
When user provides an invalid donwload URL, the error message
"Unexpected exception" was displayed without any more information.
On Kimchi logs, there is:
Error in async_task 1
Traceback (most recent call last):
File "/home/alinefm/kimchi/src/kimchi/asynctask.py", line 72, in _run_helper
self.fn(cb, opaque)
File "/home/alinefm/kimchi/src/kimchi/model/storagevolumes.py", line 192, in
_create_volume_with_url
with contextlib.closing(urllib2.urlopen(url)) as response:
File "/usr/lib64/python2.7/urllib2.py", line 127, in urlopen
return _opener.open(url, data, timeout)
File "/usr/lib64/python2.7/urllib2.py", line 404, in open
response = self._open(req, data)
File "/usr/lib64/python2.7/urllib2.py", line 422, in _open
'_open', req)
File "/usr/lib64/python2.7/urllib2.py", line 382, in _call_chain
result = func(*args)
File "/usr/lib64/python2.7/urllib2.py", line 1224, in https_open
return self.do_open(httplib.HTTPSConnection, req)
File "/usr/lib64/python2.7/urllib2.py", line 1186, in do_open
raise URLError(err)
URLError: <urlopen error [Errno -2] Name or service not known>
It is because the download URL was not being checked prior to start the
Task and this case was not checked on Task handler.
To solver it, check if the download URL is accessible prior to start the
Task so user can change it accordingly.
Signed-off-by: Aline Manera <alinefm(a)linux.vnet.ibm.com>
---
src/kimchi/i18n.py | 1 +
src/kimchi/model/storagevolumes.py | 8 ++++++++
2 files changed, 9 insertions(+)
diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
index 635d6b9..67dfc11 100644
--- a/src/kimchi/i18n.py
+++ b/src/kimchi/i18n.py
@@ -190,6 +190,7 @@ messages = {
"KCHVOL0019E": _("Create volume from %(param)s is not
supported"),
"KCHVOL0020E": _("Storage volume capacity must be an integer
number."),
"KCHVOL0021E": _("Storage volume URL must be http://, https://, ftp://
or ftps://."),
+ "KCHVOL0022E": _("Unable to access file %(url)s. Please, check
it."),
"KCHIFACE0001E": _("Interface %(name)s does not exist"),
diff --git a/src/kimchi/model/storagevolumes.py b/src/kimchi/model/storagevolumes.py
index ac6887a..58730ea 100644
--- a/src/kimchi/model/storagevolumes.py
+++ b/src/kimchi/model/storagevolumes.py
@@ -67,6 +67,14 @@ class StorageVolumesModel(object):
create_param = vol_source[index_list[0]]
+ # Verify if the URL is valid
+ if create_param == 'url':
+ url = params['url']
+ try:
+ urllib2.urlopen(url)
+ except:
+ raise InvalidParameter('KCHVOL0022E', {'url': url})
+
if name is None:
# the methods listed in 'REQUIRE_NAME_PARAMS' cannot have
# 'name' == None
--
1.9.3
10:43 a.m.
On 24-09-2014 23:08, Aline Manera wrote:
+ # Verify if the URL is valid
+ if create_param == 'url':
+ url = params['url']
+ try:
+ urllib2.urlopen(url)
We should validate the specific parameters for the 'url' case inside the
function '_create_volume_with_url', just like we check if the upload
file exists before reading it inside the function
'_create_volume_with_file'.
Actually, I think the best way to fix this problem is by enlarging the
try/except block to include the existing urllib2.urlopen call. We don't
need to make two requests. That call will raise the same exception
there. The problem now is that it's not included inside the try/except
block, hence the "Unexpected exception".
But if we do keep this approach, you still need to close the return
variable of "urllib2.urlopen".
11:03 a.m.
On 09/25/2014 11:43 AM, Crístian Viana wrote:
On 24-09-2014 23:08, Aline Manera wrote:
> + # Verify if the URL is valid
> + if create_param == 'url':
> + url = params['url']
> + try:
> + urllib2.urlopen(url)
We should validate the specific parameters for the 'url' case inside
the function '_create_volume_with_url', just like we check if the
upload file exists before reading it inside the function
'_create_volume_with_file'.
I have done it in my first implementation but it does not work well on
UI perspective.
Let me explain.
If we do this verification on _create_volume_with_url means the POST
/storagepools/<pool>/storagevolumes will return 202 (success)
On UI side, the "Add volume" dialog will be closed and a new "volume"
is
displayed in the pool indicating it is being downloaded.
But as far as the new volume is added, it appears as "Failed" as the
Task finished with a failed status as the URL is not valid.
Doing the verification prior to start the Task means POST
/storagepools/<pool>/storagevolumes will return 500 (failed)
On UI side, the "Add volume" dialog will keep open, so user can modify
the URL and only see a "in progress volume" when it is really in progress.
Actually, I think the best way to fix this problem is by enlarging the
try/except block to include the existing urllib2.urlopen call. We
don't need to make two requests. That call will raise the same
exception there. The problem now is that it's not included inside the
try/except block, hence the "Unexpected exception".
But if we do keep this approach, you still need to close the return
variable of "urllib2.urlopen".
Good point. I will do that on V2.
3744
days inactive
3744
days old
2 comments
2 participants
participants (2)
-
Aline Manera -
Crístian Viana