[Kimchi-devel] [PATCH 4/6] Move ISO path validation to IsoImage()

[Royce Lv]

On 2013年12月31日 01:09, 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 |   55 +++++++++++++++++++++++++++----------------------
>   1 file changed, 30 insertions(+), 25 deletions(-)
>
> diff --git a/src/kimchi/isoinfo.py b/src/kimchi/isoinfo.py
> index 7b7fa78..dd26204 100644
> --- a/src/kimchi/isoinfo.py
> +++ b/src/kimchi/isoinfo.py
> @@ -134,13 +134,37 @@ 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
> +
> +        if remote is not None:
> +            self.remote = remote
> +        else:
> +            self.remote = self._is_iso_remote()
> +
>           self._scan()
>
> +    def _is_iso_remote(self):
> +        if os.path.isfile(self.path):
> +            return False
> +
> +        if self._check_url_path():
> +            return True
> +
> +        raise IsoFormatError('ISO %s does not exist' % self.path)
> +
> +    def _check_url_path(self):
> +        try:
> +            code = urllib2.urlopen(self.path).getcode()
> +            if code != 200:
> +                return False
> +        except (urllib2.HTTPError, ValueError):
> +            return False
> +
> +        return True
I kind of want this func moved outside for reusage of other module.
> +
>       def _unpack(self, s, data):
>           return s.unpack(data[:s.size])
>
> @@ -246,7 +270,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 +322,20 @@ def probe_iso(status_helper, params):
>                       continue
>                   iso = os.path.join(root, name)
>                   try:
> -                    ret = _probe_iso(iso)
> +                    ret = _probe_iso(iso, False)
>                       update_result(iso, ret)
I think if we want to move remote url probe inside IsoImage class, is 
there any need to pass 'remote' param outside? We can just dump it.
>                   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)
At first we have probe_one and probe_iso originally to serve use cases 
of just probe one iso and probe all isos under a directory, now that you 
are refactor this part, I suggest that we dump this probe_one function, 
and all use probe_iso.
>
>
>   if __name__ == '__main__':