Antoni Segura Puimedon has posted comments on this change.

Change subject: Configure modules without --force flag when isconfigure returns 
false
......................................................................


Patch Set 2: Code-Review-1

(2 comments)

http://gerrit.ovirt.org/#/c/30136/2/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:

Line 684: 
Line 685:     sys.stdout.write("\nChecking configuration status...\n\n")
Line 686:     for c in __configurers:
Line 687:         if c.getName() in args.modules:
Line 688:             isconfigured = c.isconfigured() != CONFIGURED
shouldn't this be either:
    isnotconfigured = c.isconfigured() != CONFIGURED
or
    isconfigured = c.isconfigured() == CONFIGURED
?
Line 689:             override = args.force and isconfigured
Line 690:             if not override and not c.validate():
Line 691:                 raise InvalidConfig(
Line 692:                     "Configuration of %s is invalid" % c.getName()


Line 690:             if not override and not c.validate():
Line 691:                 raise InvalidConfig(
Line 692:                     "Configuration of %s is invalid" % c.getName()
Line 693:                 )
Line 694:             if override or isconfigured:
then here either:
    if override or isnotconfigured:
or
    if override or not isconfigured:

(I vote for the latter on both).
Line 695:                 configurer_to_trigger.append(c)
Line 696: 
Line 697:     services = []
Line 698:     for c in configurer_to_trigger:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4bf6f85f7852b84bbe1aa30bb30ce6bcb3d56aed
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to