
Just a minor comment below On 01/04/2014 03:18 AM, Aline Manera wrote:
From: Aline Manera <alinefm@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@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" 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
+ 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)
why not we remove "remote" argument, and always 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) 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? not sure urllib2.HTTPError, ValueError can cover all exception so other exception still throw? -- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center