[node-patches] Change in ovirt-node[master]: Add ovirt-node generic registration script

fabiand at fedoraproject.org fabiand at fedoraproject.org
Wed May 21 16:15:46 UTC 2014


Fabian Deutsch has posted comments on this change.

Change subject: Add ovirt-node generic registration script
......................................................................


Patch Set 1:

(3 comments)

Mainly the request for some documentation.

http://gerrit.ovirt.org/#/c/27976/1/registration/example.json
File registration/example.json:

Line 1: {
Line 2:     "register": {
Line 3:         "steps": [ 
Trailing whitespace.
Line 4:             {
Line 5:                 "name": "get_ca",
Line 6:                 "action": "get",
Line 7:                 "parameters": {


http://gerrit.ovirt.org/#/c/27976/1/registration/ovirt-node-registration.py
File registration/ovirt-node-registration.py:

Line 33:             format='%(levelname)10s %(asctime)s ' \
Line 34:                 '%(pathname)s:%(lineno)s:%(funcName)s %(message)s',
Line 35:             level=logging.DEBUG
Line 36:            )
Line 37:         self.logger = logging.getLogger('ovirt-node-registration')
IIUIC this module is shall also be used form the TUI.
As the TUI already does some logging configuration, I'd suggest to move this logging configuration to the __main__ method.
Line 38: 
Line 39:         self.maps = {"get": self.get,
Line 40:                      "ui": self.ui,
Line 41:                      "persist": self.persist,


Line 86:         self.logger.debug("Output of %s was: %s" % (kwargs["cmd"], output))
Line 87: 
Line 88: 
Line 89: def main():
Line 90:     parser = OptionParser()
Please add some - I am just suggesting - inline help or man page, so the reviewer and user knows how the app shall be called and what can be achieved.
Line 91:     parser.add_option("-f", "--file", dest="filename",
Line 92:                       help="read from FILE", metavar="FILE")
Line 93:     parser.add_option("-a", "--action", dest="action",
Line 94:                       help="action to execute")


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9457b05716c4dc5a47a280b8335a41e6fae73c0d
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Ryan Barry <rbarry at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at fedoraproject.org>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes



More information about the node-patches mailing list