[Kimchi-devel] [PATCH 4/6] Move ISO path validation to IsoImage()
Aline Manera
alinefm at linux.vnet.ibm.com
Mon Jan 6 14:21:05 UTC 2014
On 01/06/2014 02:49 AM, Sheldon wrote:
> Just a minor comment below
>
> On 01/04/2014 03:18 AM, Aline Manera wrote:
>> From: Aline Manera <alinefm at br.ibm.com>
>>
>> That way we will have a single point to check ISO path validation.
>> And the user doesn't need to care about it as IsoImage() will check
>> if the ISO
>> path is local, remote or invalid.
>>
>> Signed-off-by: Aline Manera <alinefm at br.ibm.com>
>> ---
>> src/kimchi/isoinfo.py | 44
>> +++++++++++++++++++-------------------------
>> src/kimchi/utils.py | 12 ++++++++++++
>> 2 files changed, 31 insertions(+), 25 deletions(-)
>>
>> diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py
>> index 7d919a0..83e4b95 100644
>> --- a/src/kimchi/isoinfo.py
>> +++ b/src/kimchi/isoinfo.py
>> @@ -29,7 +29,7 @@ import urllib2
>>
>>
>> from kimchi.exception import IsoFormatError
>> -from kimchi.utils import kimchi_log
>> +from kimchi.utils import check_url_path, kimchi_log
>>
>> iso_dir = [
>> ##
>> @@ -134,13 +134,26 @@ class IsoImage(object):
>> EL_TORITO_VALIDATION_ENTRY = struct.Struct("=BBH24sHBB")
>> EL_TORITO_BOOT_ENTRY = struct.Struct("=BBHBBHL20x")
>>
>> - def __init__(self, path, remote = False):
>> + def __init__(self, path, remote = None):
>> self.path = path
>> self.volume_id = None
>> self.bootable = False
>> +
>> self.remote = remote
> why change "remote = False " to "remote = None"
"remote = None" means the IsoImage will verify if it is a remote ISO or not
> Sometimes, you want to check the iso is remote by caller outer of the c
> And Sometimes, you want to check the iso is remote by IsoImage
>
>
> why not we remove "remote" argument, and always
OK
> class IsoImage(object):
> + def __init__(self, path, remote = False ):
> + self.remote = self._is_iso_remote()
> +
> + def _is_iso_remote(self):
> + if os.path.isfile(self.path):
> + return False
> .....
>
> or just let the caller of IsoImage to check it?
> class IsoImage(object):
> + def __init__(self, path, remote = False ):
> then
> isoimg = IsoImage(path, remote = is_iso_remote(path) ):
>
> + def _is_iso_remote(path):
> + if os.path.isfile(path):
> + return False
> +
> + if check_url_path(path):
> + return True
> +
> + raise IsoFormatError('ISO %s does not exist' % path)
>> + if self.remote is None:
>> + self.remote = self._is_iso_remote()
>> +
>> self._scan()
>>
>> + def _is_iso_remote(self):
>> + if os.path.isfile(self.path):
>> + return False
>> +
>> + if check_url_path(self.path):
>> + return True
>> +
>> + raise IsoFormatError('ISO %s does not exist' % self.path)
>> +
>> def _unpack(self, s, data):
>> return s.unpack(data[:s.size])
>>
>> @@ -246,7 +259,7 @@ class Matcher(object):
>> return self.lastmatch.group(num)
>>
>>
>> -def _probe_iso(fname, remote = False):
>> +def _probe_iso(fname, remote = None):
>> try:
>> iso = IsoImage(fname, remote)
>> except Exception, e:
>> @@ -298,39 +311,20 @@ def probe_iso(status_helper, params):
>> continue
>> iso = os.path.join(root, name)
>> try:
>> - ret = _probe_iso(iso)
>> + ret = _probe_iso(iso, False)
> the same question of IsoImage definition
>> update_result(iso, ret)
>> except:
>> continue
>> - elif os.path.isfile(loc):
>> - ret = _probe_iso(loc, False)
>> - update_result(loc, ret)
>> else:
>> - ret = _probe_iso(loc, True)
>> + ret = _probe_iso(loc)
>> update_result(loc, ret)
>>
>> if status_helper != None:
>> status_helper('', True)
>>
>> -def _check_url_path(path):
>> - try:
>> - code = urllib2.urlopen(path).getcode()
>> - if code != 200:
>> - return False
>> - except (urllib2.HTTPError, ValueError):
>> - return False
>> -
>> - return True
>>
>> def probe_one(iso):
>> - if os.path.isfile(iso):
>> - remote = False
>> - elif _check_url_path(iso):
>> - remote = True
>> - else:
>> - raise IsoFormatError('ISO %s does not exist' % iso)
>> -
>> - return _probe_iso(iso, remote)
>> + return _probe_iso(iso)
>>
>>
>> if __name__ == '__main__':
>> diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py
>> index b6d84fd..efd693c 100644
>> --- a/src/kimchi/utils.py
>> +++ b/src/kimchi/utils.py
>> @@ -23,6 +23,7 @@
>>
>> import cherrypy
>> import os
>> +import urllib2
>>
>>
>> from cherrypy.lib.reprconf import Parser
>> @@ -84,3 +85,14 @@ def import_class(class_path):
>>
>> def import_module(module_name):
>> return __import__(module_name, globals(), locals(), [''])
>> +
>> +
>> +def check_url_path(path):
>> + try:
>> + code = urllib2.urlopen(path).getcode()
>> + if code != 200:
>> + return False
>> + except (urllib2.HTTPError, ValueError):
>> + return False
>> +
>> + return True
>
> +def check_url_path(path):
> + try:
> + code = urllib2.urlopen(path).getcode()
> + return code != 200
> + except (urllib2.HTTPError, ValueError):
> + return False
>
>
> when urllib2.HTTPError, ValueError occur?
In fact, urllib2.HTTPError should be urllib2.URLError to be more generic
It happens when an url isn't accessible.
>>> urllib2.urlopen("https://foo").getcode()
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/lib/python2.7/urllib2.py", line 127, in urlopen
return _opener.open(url, data, timeout)
File "/usr/lib/python2.7/urllib2.py", line 401, in open
response = self._open(req, data)
File "/usr/lib/python2.7/urllib2.py", line 419, in _open
'_open', req)
File "/usr/lib/python2.7/urllib2.py", line 379, in _call_chain
result = func(*args)
File "/usr/lib/python2.7/urllib2.py", line 1219, in https_open
return self.do_open(httplib.HTTPSConnection, req)
File "/usr/lib/python2.7/urllib2.py", line 1181, in do_open
raise URLError(err)
urllib2.URLError: <urlopen error [Errno -2] Name or service not known>
The ValueError occurs when using a non well-formatted url
>>> urllib2.urlopen("foo").getcode()
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/lib/python2.7/urllib2.py", line 127, in urlopen
return _opener.open(url, data, timeout)
File "/usr/lib/python2.7/urllib2.py", line 393, in open
protocol = req.get_type()
File "/usr/lib/python2.7/urllib2.py", line 255, in get_type
raise ValueError, "unknown url type: %s" % self.__original
ValueError: unknown url type: foo
> not sure urllib2.HTTPError, ValueError can cover all exception
> so other exception still throw?
>
I will change HTTPError for URLError in next version.
More information about the Kimchi-devel
mailing list