Nir Soffer has posted comments on this change.

Change subject: BindingXmlRPC - do_PUT to return execution result in headers
......................................................................


Patch Set 3:

(1 comment)

Looks ok except the fact that the status code and error message are optional, 
some errors will have them and some don't.

http://gerrit.ovirt.org/#/c/27997/3/vdsm/BindingXMLRPC.py
File vdsm/BindingXMLRPC.py:

Line 195:                                  response['status']['message'])
Line 196: 
Line 197:             def send_error(self, error, message, exc_info=False):
Line 198:                 try:
Line 199:                     self.log.error(message, exc_info=exc_info)
If our apis includes Status-Code and Error-Message headers, they should be 
included for every error, to make integration with 3rd party easier.

Client code will like to get these headers on each request, just like xmlrpc 
client code expect the same keys in the dictionary on each request.

If we do not need this info because we have the http status code, then why are 
we returning these headers?
Line 200:                     self.send_response(error)
Line 201:                     self.end_headers()
Line 202:                 except Exception:
Line 203:                     self.log.error("failed to return response",


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I25b13f44a65cb2c794c458b03ba361bf0af92120
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to