Federico Simoncelli has posted comments on this change.

Change subject: core: BindingXMLRPC - exporting logic out from do_PUT.
......................................................................


Patch Set 3:

(5 comments)

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

Line 148:                                          json_response)
Line 149: 
Line 150:                 self._performSocketOp(operation)
Line 151: 
Line 152:             def _performSocketOp(self, execFunc):
Let's remove this.
Line 153:                 try:
Line 154:                     execFunc()
Line 155: 
Line 156:                 except socket.timeout:


Line 160:                 except Exception:
Line 161:                     self.send_error(httplib.INTERNAL_SERVER_ERROR,
Line 162:                                     "error during execution", 
exc_info=True)
Line 163: 
Line 164:             def _constructImageFromRequestInfo(self):
> Then it should be _constructIageFromRequestHeaders. But because there is on
Let's keep this.
Line 165:                 # Required headers
Line 166:                 spUUID = self.headers.getheader(self.HEADER_POOL)
Line 167:                 sdUUID = self.headers.getheader(self.HEADER_DOMAIN)
Line 168:                 imgUUID = self.headers.getheader(self.HEADER_IMAGE)


Line 176:                 return API.Image(imgUUID, spUUID, sdUUID)
Line 177: 
Line 178:             @staticmethod
Line 179:             def _createEventWithCallback(self):
Line 180:                 opFinishedEvent = threading.Event()
> If we do add this event decorator, it should be in a general place so we ca
I think we'll be able to drop the need of the event. So for now I don't care, 
we can keep this.
Line 181: 
Line 182:                 def setCallback():
Line 183:                     opFinishedEvent.set()
Line 184: 


Line 186: 
Line 187:             @staticmethod
Line 188:             def _waitForEvent(event):
Line 189:                 while not event.is_set():
Line 190:                     event.wait()
> See the comment above.
I think we'll be able to drop the need of the event. So for now I don't care, 
we can keep this.
Line 191: 
Line 192:             @staticmethod
Line 193:             def _convertToJson(data):
Line 194:                 return json.dumps(data)


Line 190:                     event.wait()
Line 191: 
Line 192:             @staticmethod
Line 193:             def _convertToJson(data):
Line 194:                 return json.dumps(data)
> Why do we need this method?
Let's drop this.
Line 195: 
Line 196:             def _getIntHeader(self, headerName, missingError):
Line 197:                 value = self.headers.getheader(
Line 198:                     headerName)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I64bb1b0a4cb85ce822929f1907847dd63eb69fc2
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: Tal Nisan <[email protected]>
Gerrit-Reviewer: Yoav Kleinberger <[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