Change in vdsm[master]: v2v: cleanup v2v directory leftovers

fromani at redhat.com fromani at redhat.com
Wed Apr 15 11:22:55 UTC 2015


Francesco Romani has posted comments on this change.

Change subject: v2v: cleanup v2v directory leftovers
......................................................................


Patch Set 2:

(5 comments)

mostly OK, but I'd like a bit stricter exception checking.

https://gerrit.ovirt.org/#/c/39789/2/vdsm/v2v.py
File vdsm/v2v.py:

Line 26: 
Line 27: from collections import namedtuple
Line 28: from contextlib import closing, contextmanager
Line 29: import errno
Line 30: from glob import glob
let's try to import modules -not names- as much as we can, so
let's just use

  import glob
Line 31: import logging
Line 32: import os
Line 33: import re
Line 34: import threading


Line 213:     return ret
Line 214: 
Line 215: 
Line 216: def clean_leftovers():
Line 217:     for f in glob('%s/*.ovf' % _V2V_DIR):
> true,
'f' is a bit too terse
Line 218:         try:
Line 219:             os.remove(f)
Line 220:         except Exception as e:
Line 221:             logging.warn('Cannot remove file: %s, err: %s', f, e)


Line 216: def clean_leftovers():
Line 217:     for f in glob('%s/*.ovf' % _V2V_DIR):
Line 218:         try:
Line 219:             os.remove(f)
Line 220:         except Exception as e:
Mayb narrow down to (OSError, IOError) and also check errno?
Line 221:             logging.warn('Cannot remove file: %s, err: %s', f, e)
Line 222: 
Line 223:     for f in glob('%s/*.tmp' % _V2V_DIR):
Line 224:         try:


Line 219:             os.remove(f)
Line 220:         except Exception as e:
Line 221:             logging.warn('Cannot remove file: %s, err: %s', f, e)
Line 222: 
Line 223:     for f in glob('%s/*.tmp' % _V2V_DIR):
same, 'f' is too terse. Moreover, better not to recycle identifiers
Line 224:         try:
Line 225:             os.remove(f)
Line 226:         except Exception as e:
Line 227:             logging.warn('Cannot remove file: %s, err: %s', f, e)


Line 222: 
Line 223:     for f in glob('%s/*.tmp' % _V2V_DIR):
Line 224:         try:
Line 225:             os.remove(f)
Line 226:         except Exception as e:
same
Line 227:             logging.warn('Cannot remove file: %s, err: %s', f, e)
Line 228: 
Line 229: 
Line 230: def _read_ovf(job_id):


-- 
To view, visit https://gerrit.ovirt.org/39789
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2f5b63863f4f7dc85d6af0ca015099274715441
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list