Yaniv Bronhaim has posted comments on this change. Change subject: tool: change configurators from classes to modules. ......................................................................
Patch Set 7: (3 comments) http://gerrit.ovirt.org/#/c/34047/7/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 110: configurer_to_trigger = [] Line 111: Line 112: sys.stdout.write("\nChecking configuration status...\n\n") Line 113: for c in args.modules: Line 114: isconfigured = getattr(c, 'isconfigured', lambda: NO)() wouldn't it be nicer to have def is_configured(module): return getattr(module, 'isconfigured', lambd: NO)() def validate(module) def depended_services_list(module) def get_requirements(module) return getattr(module, 'requires', frozenset()) ..and so on and on each def put its api description instead of on the top of the file Line 115: override = args.force and isconfigured != YES Line 116: if not override and not getattr(c, 'validate', lambda: True)(): Line 117: raise InvalidConfig( Line 118: "Configuration of %s is invalid" % c.name http://gerrit.ovirt.org/#/c/34047/7/lib/vdsm/tool/configurators/libvirt.py File lib/vdsm/tool/configurators/libvirt.py: Line 487: ), Line 488: 'configure': _prefixAndPrepend, Line 489: 'prependFile': 'LRCONF_EXAMPLE', Line 490: 'removeConf': _unprefixAndRemoveSection, Line 491: 'persisted': True, you changed my change .. see http://gerrit.ovirt.org/35758 Line 492: }, Line 493: Line 494: 'LRCONF_EXAMPLE': { Line 495: 'path': os.path.join( http://gerrit.ovirt.org/#/c/34047/7/lib/vdsm/tool/configurators/sanlock.py File lib/vdsm/tool/configurators/sanlock.py: Line 54: sys.stderr.write(err) Line 55: if rc != 0: Line 56: raise RuntimeError("Failed to perform sanlock config.") Line 57: Line 58: didn't we have removeConf for sanlock?? i recall we did.. Line 59: def isconfigured(): Line 60: """ Line 61: True if sanlock service is configured, False if sanlock service Line 62: requires a restart to reload the relevant supplementary groups. -- To view, visit http://gerrit.ovirt.org/34047 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iff5d588cfbbcfe0d5f4a90d624d24f21ef4cc580 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Dima Kuznetsov <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Yeela Kaplan <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: mooli tayer <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
