Change in vdsm[master]: tool: change configure verb logic.

ybronhei at redhat.com ybronhei at redhat.com
Thu Jan 1 09:29:42 UTC 2015


Yaniv Bronhaim has posted comments on this change.

Change subject: tool: change configure verb logic.
......................................................................


Patch Set 7:

(2 comments)

http://gerrit.ovirt.org/#/c/34100/7/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:

Line 106:     args = _parse_args(*args)
Line 107: 
Line 108:     sys.stdout.write("\nChecking configuration status...\n\n")
Line 109:     configurer_to_trigger = [c for c in args.modules
Line 110:                              if _should_configure(c, args)]
pass only agrs.force if you use only that
Line 111: 
Line 112:     services = []
Line 113:     for c in configurer_to_trigger:
Line 114:         for s in _getservices(c):


Line 333: 
Line 334: def _should_configure(c, args):
Line 335:     configured = _isconfigured(c)
Line 336:     configure_allowed = (configured == configurators.NO or
Line 337:                          (configured == configurators.MAYBE and args.force))
want to make it even more readable ? do:

 if configured == NO:
  return True
 elif configured == MAYBE and force:
  return True
 elif not _validate(c):
  raise VALIDATION ERROR
 else
  return False

and you can even pass the _isconfigured(c) directly to should_configure 

 configurer_to_trigger = [c for c in args.modules if _should_configure(_isconfigured(c), args)] )


no?
I'll understand if you go with your way.. you're decision.
Line 338:     if not _validate(c) and not configure_allowed:
Line 339:         raise configurators.InvalidConfig(
Line 340:             "Confiuration of %s is invalid" % c.name
Line 341:         )


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I166fbae855ebff93ef84270f11b74f32fcc115c2
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer <mtayer at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list