Change in vdsm[master]: Define ModuleConfigure interface and API for modules configu...

ybronhei at redhat.com ybronhei at redhat.com
Sun Oct 20 15:34:33 UTC 2013


Yaniv Bronhaim has posted comments on this change.

Change subject: Define ModuleConfigure interface and API for modules configuration tool
......................................................................


Patch Set 1:

(2 comments)

....................................................
File lib/vdsm/tool/configurator.py
Line 165:         # no modules specified
Line 166:         raise RuntimeError("No modules were specified for operation.\n"
Line 167:                            "vdsm-tool configure --help for more details")
Line 168: 
Line 169:     if args.test and args.dry:
test is testing the configuration itself, like conflicts. dry just checks if configure is required or not. its explained in the help. runpots is nice, but what's wrong with the current implementation ? with --runopt it will be harder to explain about all the optional opts .. I prefer to split to 2 different verbs.
Line 170:         raise RuntimeError("Can't operate dry run and test at the same time\n"
Line 171:                            "vdsm-tool configure --help for more details")
Line 172: 
Line 173:     modules = args.modules.split(',')


Line 187: 
Line 188: 
Line 189: def parse_configure_args():
Line 190:     parser = argparse.ArgumentParser('vdsm-tool configure')
Line 191:     parser.add_argument('--modules', dest='modules', default=None,
metavar is what printed as example for the command's arguments, this prints:

vdsm-tool configure [--module {whatever i put in metava}] --dry ...

that's why I did it that way. 

why to use action='append', to avoid the split part? what's wrong with that way?

will add default all logic.
Line 192:                         metavar='libvirt,sanlock,foo,boo',
Line 193:                         help='List of modules to configure [libvirt, sanlock]')
Line 194:     parser.add_argument('--dry', dest='dry', default=False,
Line 195:                         action='store_true',


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0bcda0d9778b02281e5fd0699e48bb3447ba4b73
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list