Saggi Mizrahi has uploaded a new change for review.
Change subject: Move fenceNode out of API.py ......................................................................
Move fenceNode out of API.py
The actual details of how the response is mangles is part of the bindings so API.py really doesn't serve a purpose.
This is the first step in a general trend of moving the logic out of API.py to dedicated modules and move the response mangling to the binding wrappers.
Change-Id: Idaba551333a0f289abaff11dc113e09c426d591a Signed-off-by: Saggi Mizrahi smizrahi@redhat.com --- M vdsm/API.py M vdsm/BindingXMLRPC.py 2 files changed, 25 insertions(+), 38 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/91/7191/1
diff --git a/vdsm/API.py b/vdsm/API.py index 5826d81..4fdc7e7 100644 --- a/vdsm/API.py +++ b/vdsm/API.py @@ -37,7 +37,6 @@ from vdsm.define import doneCode, errCode, Kbytes, Mbytes import caps from vdsm.config import config -import fenceAgent
import supervdsm
@@ -972,40 +971,6 @@ APIBase.__init__(self)
# General Host functions - def fenceNode(self, addr, port, agent, username, password, action, - secure=False, options=''): - """Send a fencing command to a remote node. - - agent is one of (rsa, ilo, drac5, ipmilan, etc) - action can be one of (status, on, off, reboot).""" - - self.log.debug('fenceNode(addr=%s,port=%s,agent=%s,user=%s,' + - 'passwd=%s,action=%s,secure=%s,options=%s)', addr, port, agent, - username, 'XXXX', action, secure, options) - - secure = utils.tobool(secure) - port = int(port) - - if action == "status": - try: - power = fenceAgent.getFenceStatus(addr, port, agent, username, - password, secure, options) - - return {'status': doneCode, - 'power': power} - - except fenceAgent.FenceStatusCheckError as e: - return {'status': {'code': 1, 'message': str(e)}} - - try: - fenceAgent.fenceNode(addr, port, agent, username, password, secure, - options, self._cif.shutdownEvent) - - return {'status': doneCode} - - except fenceAgent.UnsupportedFencingAgentError: - return errCode['fenceAgent'] - def ping(self): "Ping the server. Useful for tests" return {'status': doneCode} diff --git a/vdsm/BindingXMLRPC.py b/vdsm/BindingXMLRPC.py index 48f4723..7eee29e 100644 --- a/vdsm/BindingXMLRPC.py +++ b/vdsm/BindingXMLRPC.py @@ -32,6 +32,8 @@ from vdsm.define import doneCode, errCode import API from vdsm.exception import VdsmException +import fenceAgent + try: from gluster.api import getGlusterMethods _glusterEnabled = True @@ -355,9 +357,29 @@
def fenceNode(self, addr, port, agent, username, password, action, secure=False, options=''): - api = API.Global() - return api.fenceNode(addr, port, agent, username, password, - action, secure, options) + + secure = utils.tobool(secure) + port = int(port) + + if action == "status": + try: + power = fenceAgent.getFenceStatus(addr, port, agent, username, + password, secure, options) + + return {'status': doneCode, + 'power': power} + + except fenceAgent.FenceStatusCheckError as e: + return {'status': {'code': 1, 'message': str(e)}} + + try: + fenceAgent.fenceNode(addr, port, agent, username, password, secure, + options, self.cif.shutdownEvent) + + return {'status': doneCode} + + except fenceAgent.UnsupportedFencingAgentError: + return errCode['fenceAgent']
def setLogLevel(self, level): api = API.Global()
-- To view, visit http://gerrit.ovirt.org/7191 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: Idaba551333a0f289abaff11dc113e09c426d591a Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com
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@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
Saggi Mizrahi has posted comments on this change.
Change subject: Move fenceNode out of API.py ......................................................................
Patch Set 2: (2 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: No, What belongs here is backward compatibility glue code until we throw away this modules.
Because the status action returns a completely different response and has different logic it will be a separate call in the new API. 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)}} This is problematic as it binds the modules to the bindings. If we want to change the codes for the new bindings there will be no way to do that. Line 374: Line 375: try: Line 376: fenceAgent.fenceNode(addr, port, agent, username, password, secure, Line 377: options, self.cif.shutdownEvent)
-- 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@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Move fenceNode out of API.py ......................................................................
Patch Set 3:
Build Failed
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/466/ : FAILURE
-- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Move fenceNode out of API.py ......................................................................
Patch Set 4:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/468/ : SUCCESS
-- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Move fenceNode out of API.py ......................................................................
Patch Set 5:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/605/ : SUCCESS
-- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Move fenceNode out of API.py ......................................................................
Patch Set 6:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/647/ : SUCCESS
-- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Move fenceNode out of API.py ......................................................................
Patch Set 7:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/694/ : SUCCESS
-- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Adam Litke has posted comments on this change.
Change subject: Move fenceNode out of API.py ......................................................................
Patch Set 7:
I don't understand this patch. You have removed a function from API,py. Does that mean you are removing a previously supported API? How would it be exposed in libvdsm? Do we need to duplicate the BindingXMLRPC logic everywhere?
-- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Move fenceNode out of API.py ......................................................................
Patch Set 7: I would prefer that you didn't submit this
(1 inline comment)
.................................................... Commit Message Line 5: CommitDate: 2012-08-27 11:16:03 -0400 Line 6: Line 7: Move fenceNode out of API.py Line 8: Line 9: The actual details of how the response is mangles is part of the mangles->mangled
Understood. But why fenceNode is completely obliterated from API.py? it is still a public verb, so it should still be exposed to the world via API.
It may be a silly
fenceNode = fenceAgent=fenceNode
but as long as we keep API.py, all public verbs should be there. Line 10: bindings so API.py really doesn't serve a purpose. Line 11: Line 12: This is the first step in a general trend of moving the logic out of Line 13: API.py to dedicated modules and move the response mangling to the
-- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Adam Litke has posted comments on this change.
Change subject: Move fenceNode out of API.py ......................................................................
Patch Set 8: -Code-Review
Itamar Heim has posted comments on this change.
Change subject: Move fenceNode out of API.py ......................................................................
Patch Set 8:
ping
Itamar Heim has abandoned this change.
Change subject: Move fenceNode out of API.py ......................................................................
Abandoned
abandoning per no reply. please restore if still relevant.
vdsm-patches@lists.fedorahosted.org