Change in vdsm[master]: v2v: ova should support zip and extracted directory formats
fromani at redhat.com
fromani at redhat.com
Fri Nov 6 09:31:57 UTC 2015
Francesco Romani has posted comments on this change.
Change subject: v2v: ova should support zip and extracted directory formats
......................................................................
Patch Set 2: Code-Review-1
(2 comments)
nice, a couple more of possible improvements.
https://gerrit.ovirt.org/#/c/48129/2/vdsm/v2v.py
File vdsm/v2v.py:
Line 751:
Line 752:
Line 753: def _read_ovf_from_ova_dir(ova_path):
Line 754: files = os.listdir(ova_path)
Line 755: for ova_file in files:
please see also comment at line 766 which may make this irrelevant
minor: here you use ova_file; below you use ova_file. The difference is just one char. Maybe using
for entry in files:
if '.ovf' == os.path.splitext(entry)[1].lower():
with open(os.path.join(ova_path, entry), 'r') as ovf_file:
return ovf_file.read()
leads to better code.
Your call, as you feels is most fit.
Line 756: if '.ovf' == os.path.splitext(ova_file)[1].lower():
Line 757: with open(os.path.join(ova_path, ova_file), 'r') as ovf_file:
Line 758: return ovf_file.read()
Line 759: raise ClientError('OVA directory %s does not contain ovf file' % ova_path)
Line 762: def _read_ovf_from_zip_ova(ova_path):
Line 763: with open(ova_path, 'rb') as fh:
Line 764: zf = zipfile.ZipFile(fh)
Line 765: for name in zf.namelist():
Line 766: if '.ovf' == os.path.splitext(name)[1].lower():
we can extract one helper here:
def _find_ovf(entries):
for entry in entries:
if '.ovf' == os.path.splitext(entry)[1].lower():
return entry
return None
use it like:
def _read_ovf_from_zip_ova(ova_path):
with open(ova_path, 'rb') as fh:
zf = zipfile.ZipFile(fh)
name = _find_ovf(zf.namelist())
if name is not None:
return zf.read(name)
raise ClientError('OVA does not contains file with .ovf suffix')
and, above:
def _read_ovf_from_ova_dir(ova_path):
name = _find_ovf(ova_path)
if name is not None:
with open(os.path.join(ova_path, name), 'r') as ovf_file:
return ovf_file.read()
raise ClientError('OVA directory %s does not contain ovf file' % ova_path)
Line 767: return zf.read(name)
Line 768: raise ClientError('OVA does not contains file with .ovf suffix')
Line 769:
Line 770:
--
To view, visit https://gerrit.ovirt.org/48129
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I494b88709c4c23fd39690b589eff1134e74f81ba
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list