Adam Litke has posted comments on this change.
Change subject: Move fenceNode out of API.py
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File vdsm/BindingXMLRPC.py
Line 361: secure = utils.tobool(secure)
Line 362: port = int(port)
Line 363:
Line 364: if action == "status":
Line 365: try:
This has enough logic in it to make me nervous. If the fenceAgent module is not returning
things in an easily consumable form then it should be changed.
What belongs here is only formatting/handling parameters and return values and a simple
function call down to the lower-level function.
Line 366: power = fenceAgent.getFenceStatus(addr, port, agent, username,
Line 367: password, secure, options)
Line 368:
Line 369: return {'status': doneCode,
Line 369: return {'status': doneCode,
Line 370: 'power': power}
Line 371:
Line 372: except fenceAgent.FenceStatusCheckError as e:
Line 373: return {'status': {'code': 1, 'message':
str(e)}}
I am not sure that the binding should be aware of module-specific exceptions. I think the
modules need to return Top-level, user-visible errors that can be passed through and not
manipulated here.
Line 374:
Line 375: try:
Line 376: fenceAgent.fenceNode(addr, port, agent, username, password, secure,
Line 377: options, self.cif.shutdownEvent)
Line 377: options, self.cif.shutdownEvent)
Line 378:
Line 379: return {'status': doneCode}
Line 380:
Line 381: except fenceAgent.UnsupportedFencingAgentError:
Ditto.
Line 382: return errCode['fenceAgent']
Line 383:
Line 384: def setLogLevel(self, level):
Line 385: api = API.Global()
--
To view, visit
http://gerrit.ovirt.org/7191
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idaba551333a0f289abaff11dc113e09c426d591a
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Adam Litke <agl(a)us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>