Change in vdsm[master]: net_configurators: Fix optional configurator importing

Federico Simoncelli fsimonce at redhat.com
Fri Aug 1 11:28:48 UTC 2014


Federico Simoncelli has posted comments on this change.

Change subject: net_configurators: Fix optional configurator importing
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.ovirt.org/#/c/30924/1/vdsm/network/configurators/__init__.py
File vdsm/network/configurators/__init__.py:

Line 32: class RollbackIncomplete(Exception):
Line 33:     pass
Line 34: 
Line 35: 
Line 36: class OptionalConfigurator(ImportError):
It should be OptionalConfiguratorError (if not OptionalConfiguratorImportError)
Line 37:     """Configurator modules should raise this exception if they fail to import
Line 38:     their dependencies and are truly optional configurators"""
Line 39:     pass
Line 40: 


http://gerrit.ovirt.org/#/c/30924/1/vdsm/network/configurators/pyroute_two.py
File vdsm/network/configurators/pyroute_two.py:

Line 30: try:
Line 31:     from pyroute2 import IPDB
Line 32: except ImportError as ie:
Line 33:     oce = OptionalConfigurator('Failed to import the pyroute2_two '
Line 34:                                'configurator', *ie.args)
Not sure if we need this all. See next file.
Line 35:     raise oce
Line 36: 
Line 37: 
Line 38: class PyrouteTwo(Iproute2):


http://gerrit.ovirt.org/#/c/30924/1/vdsm/vdsm-restore-net-config
File vdsm/vdsm-restore-net-config:

Line 113:     for importer, moduleName, isPackage in pkgutil.iter_modules(
Line 114:             configurators.__path__, prefix):
Line 115:         try:
Line 116:             __import__(moduleName, fromlist="_")
Line 117:         except configurators.OptionalConfigurator as oce:
Why not just:

 except ImportError as e:
     ...

The specific exception is not adding much, it's even forcing you to re-raise a different error (see previous file).
Line 118:             logging.debug('Ignoring optional configurator: %s. Reason: %r',
Line 119:                           moduleName, oce.args)
Line 120:             continue
Line 121: 


-- 
To view, visit http://gerrit.ovirt.org/30924
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iff38f5e334167e34ba379be164e690f2888f5fea
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek 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