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

[Aline Manera]

On 12/31/2013 04:44 AM, Royce Lv wrote:
> 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.

It isn't used in other place because that I moved it to to IsoImage()

>> +
>>       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.

In this case, I passed 'remote' parameter as we have already checked if 
it is local

if os.path.isdir(loc):
    ....


>>                   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__':
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel