Change in vdsm[master]: move get-conf-item/set-conf-item to vdsm-tool

shaohef at linux.vnet.ibm.com shaohef at linux.vnet.ibm.com
Sun Sep 30 15:31:23 UTC 2012


ShaoHe Feng has posted comments on this change.

Change subject: move get-conf-item/set-conf-item to vdsm-tool
......................................................................


Patch Set 3: (5 inline comments)

....................................................
File vdsm-tool/get-conf-item.py
Line 16: #
Line 17: # Refer to the README and COPYING files for full details of the license
Line 18: #
Line 19: 
Line 20: # Access python's config files from bash by vdsm-tool
agree,  doc string is a good idea.
Line 21: # Usage: vdsm-tool get-conf-item filename.conf section item default
Line 22: 
Line 23: from vdsm.tool import expose
Line 24: try:


Line 29:     vdsmconf = ConfigParser.ConfigParser()
Line 30: 
Line 31: 
Line 32: def _get_conf_item(file, section, item, default):
Line 33:     vdsmconf.read(file)
surely, we can add an condition to check whether vdsm.config is imported successfully.
 That there seems no effect,  the original vdsm/get-conf-item still load the file again. just want to skip a condition?
Line 34:     try:
Line 35:         return vdsmconf.get(section, item)
Line 36:     except:
Line 37:         return default


Line 37:         return default
Line 38: 
Line 39: 
Line 40: @expose("get-conf-item")
Line 41: def print_conf_item(file, section, item, default):
do you means print_conf_item(section, item, default, file=VDSMCONFFILE) with an default file value or print_conf_item(section, item, default) without file?
  In my fist patch set, I did not expose configuration file as a parameter, for I find all this command called was given the same default value vdsm.conf.
  but I read the comment of vdsm/get-conf-item, I just say "# Access python's config files from bash.", the I think that it means not only can get vdsm.conf but it should can also get the other of configuration file, such as mom.conf.
Line 42:     """
Line 43:     get item from config files
Line 44:     Usage: get-conf-item filename.conf section item default
Line 45:     """


....................................................
File vdsm-tool/Makefile.am
Line 23: 	vdsm-tool
Line 24: 
Line 25: EXTRA_DIST = \
Line 26: 	get-conf-item.py \
Line 27: 	set-conf-item.py \
Sorry. It's my fault. Reason, the previous patch (patch-set1) I add a file *.py.in in this list, and the later patch I remove it, I make a mistake to put a source file in this list and do not consider what the 'EXTRA_DIST' at all.
Line 28: 	validate_ovirt_certs.py.in
Line 29: 
Line 30: dist_vdsmtool_DATA = \
Line 31: 	__init__.py \


....................................................
File vdsm-tool/set-conf-item.py
Line 20: # Set python's config files from the shell by vdsm-tool.
Line 21: # Usage: vdsm-tool set-conf-item filename.conf section item value
Line 22: 
Line 23: # WARNING: comments and item order within config file will be destoryed
Line 24: 
and rename the new  integration file, conf-item?
Line 25: from vdsm.tool import expose
Line 26: try:
Line 27:     from vdsm.config import config
Line 28:     vdsmconf = config


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5c64de097bbaea6a8cf862b43243377e10e00391
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl at us.ibm.com>
Gerrit-Reviewer: Bing Bu Cao <mars at linux.vnet.ibm.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh at us.ibm.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
Gerrit-Reviewer: Shu Ming <shuming at linux.vnet.ibm.com>
Gerrit-Reviewer: Wenyi Gao <wenyi at linux.vnet.ibm.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list