9:04 p.m.
Douglas Schilling Landgraf has uploaded a new change for review.
Change subject: [WIP] Adding flowID
......................................................................
[WIP] Adding flowID
This patch will implement the flowID schema. The idea is provide a
mechanism to identify the transactions between ovirt engine and vdsm.
Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
---
M vdsm/clientIF.py
1 file changed, 30 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/21/1221/1
--
To view, visit http://gerrit.ovirt.org/1221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
2:08 p.m.
New subject: Change in vdsm[master]: Adding flowID
Dan Kenigsberg has posted comments on this change.
Change subject: Adding flowID
......................................................................
Patch Set 5: I would prefer that you didn't submit this
(1 inline comment)
please remember my little refactoring patch. you may want to take it *before* your own
patch, so that you do not have to duplicate code.
....................................................
File vdsm/BindingXMLRPC.py
Line 139: threadLocal.flowID =
self.headers.get(constants.HTTP_HEADER_FLOWID)
it makes sense for this to be a CONSTANT, particularly if used twice.
However, there is no justification to throw it into the global trash can called
"constants". Use a module-local _CONSTANT instead.
--
To view, visit http://gerrit.ovirt.org/1221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
1:17 a.m.
New subject: Change in vdsm[master]: Adding flowID
Moti Asayag has posted comments on this change.
Change subject: Adding flowID
......................................................................
Patch Set 4: (1 inline comment)
....................................................
Commit Message
Line 10: mechanism to identify the transactions between ovirt engine and vdsm.
Just to sharpen the description, the same flowID could be shared between several API calls
between ovirt engine and vdsm in order to identify the entire scope of the backend action
(which possibly consisted of several VSDM calls).
--
To view, visit http://gerrit.ovirt.org/1221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
2:16 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: [WIP] Adding flowID
......................................................................
Patch Set 2: (2 inline comments)
....................................................
File vdsm/clientIF.py
Line 75: argsStr = str(args).split(",")
parsing strings is slow and error prone. We have easily accessible in-memory objects, let
us use them.
Line 100: logStr += " flowID [%s]" % argsDict['flowID']
As I told you privately, you can start with logging on API entry.
--
To view, visit http://gerrit.ovirt.org/1221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
4:34 p.m.
Douglas Schilling Landgraf has posted comments on this change.
Change subject: [WIP] Adding flowID
......................................................................
Patch Set 2: (2 inline comments)
....................................................
File vdsm/clientIF.py
Line 75: argsStr = str(args).split(",")
humm, I remember it was a tuple instead of dict. /me going to check again. I just
converted to str, then dict because it could safe time in the future if we need to change
this code again. (Avoid to read each element of tuple every time we need it to locate the
argument and then read +1 position to get the value, with dict it`s easy to manage such
situation).
Line 100: logStr += " flowID [%s]" % argsDict['flowID']
I thought it was enough. Going to prepare a new version. thanks
--
To view, visit http://gerrit.ovirt.org/1221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
2:42 p.m.
Dan Kenigsberg has posted comments on this change.
Change subject: [WIP] Adding flowID
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(2 inline comments)
....................................................
File vdsm/clientIF.py
Line 75: argsStr = str(args).split(",")
I do not understand what's going on here. Are you converting the arguments to a string
and then back to a dictionary? Why? you should look for a dict in args list.
Line 100: logStr += " flowID [%s]" % argsDict['flowID']
logging flowID on API entry point is nice, but was deemed not enough.
Please consider writing a LogAdapter that prints it from threadLocal.
--
To view, visit http://gerrit.ovirt.org/1221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
12:28 a.m.
Dan Kenigsberg has posted comments on this change.
Change subject: [WIP] Adding flowID
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(1 inline comment)
....................................................
File vdsm/clientIF.py
Line 648: try:
I have imagined something more generic, embedded into wrapApiMethod and applying to all
verbs.
In any case, you cannot change self.log this way - it is raceful and would quickly explode
in length (note that you add an Adapter on every api call, never removing).
Instead, I think that you should copy the flowID to threadLocal, and have one constant
LogAdapter that reads flowID from there when needed.
--
To view, visit http://gerrit.ovirt.org/1221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
3:33 p.m.
New subject: Change in vdsm[master]: Adding flowID
Dan Kenigsberg has posted comments on this change.
Change subject: Adding flowID
......................................................................
Patch Set 4: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/1221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
8:58 a.m.
New subject: Change in vdsm[master]: Adding flowID
Saggi Mizrahi has posted comments on this change.
Change subject: Adding flowID
......................................................................
Patch Set 4: I would prefer that you didn't submit this
(1 inline comment)
I would have put -2 if the patch wasn't so tiny.
Anyway, I still think flowID is useless but as long as it doesn't get in my way and
gets deprecated when we move to REST/QMF I don't care if there is another stupid UUID
in the logs.
....................................................
File vdsm/BindingXMLRPC.py
Line 139: threadLocal.flowID = self.headers.get('X-FlowID')
* Thread local storage is evil
* X- prefix is deprecated (http://tools.ietf.org/html/draft-ietf-appsawg-xdash-02)
* Header name should be in a constant
--
To view, visit http://gerrit.ovirt.org/1221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
12:47 p.m.
New subject: Change in vdsm[master]: Adding flowID
Douglas Schilling Landgraf has posted comments on this change.
Change subject: Adding flowID
......................................................................
Patch Set 4: (2 inline comments)
....................................................
Commit Message
Line 10: mechanism to identify the transactions between ovirt engine and vdsm.
Done
....................................................
File vdsm/BindingXMLRPC.py
Line 139: threadLocal.flowID = self.headers.get('X-FlowID')
Done
--
To view, visit http://gerrit.ovirt.org/1221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
12:47 p.m.
New subject: Change in vdsm[master]: Adding flowID
Douglas Schilling Landgraf has posted comments on this change.
Change subject: Adding flowID
......................................................................
Patch Set 5: Verified
--
To view, visit http://gerrit.ovirt.org/1221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
8:06 p.m.
New subject: Change in vdsm[master]: Adding flowID
Douglas Schilling Landgraf has posted comments on this change.
Change subject: Adding flowID
......................................................................
Patch Set 6: Verified
--
To view, visit http://gerrit.ovirt.org/1221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
9:54 a.m.
New subject: Change in vdsm[master]: Adding flowID
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Adding flowID
......................................................................
Adding flowID
This patch will implement the flowID schema in vdsm side. The idea is
provide a mechanism to identify the flow between ovirt engine and vdsm API calls
in order to identify the entire scope of the backend action
(which possibly consisted of several VSDM calls). At moment, we are
going to use HTTP headers to implement this feature to keep the
compability with current API, avoiding possible breakes.
Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
---
M vdsm/BindingXMLRPC.py
1 file changed, 20 insertions(+), 3 deletions(-)
Approvals:
Dan Kenigsberg: Verified; Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/1221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
9:54 a.m.
New subject: Change in vdsm[master]: Adding flowID
Dan Kenigsberg has posted comments on this change.
Change subject: Adding flowID
......................................................................
Patch Set 7: Verified; Looks good to me, approved
rebased
--
To view, visit http://gerrit.ovirt.org/1221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
9:51 a.m.
New subject: Change in vdsm[master]: Adding flowID
Dan Kenigsberg has posted comments on this change.
Change subject: Adding flowID
......................................................................
Patch Set 6: Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/1221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
9:31 a.m.
New subject: Change in vdsm[master]: Adding flowID
Moti Asayag has posted comments on this change.
Change subject: Adding flowID
......................................................................
Patch Set 6: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.ovirt.org/1221
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c818d7a90fea41663fd48620ac9718451a8662f
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
4456
days inactive
4475
days old
vdsm-patches@lists.fedorahosted.org
15 comments
4 participants
participants (4)
-
Dan Kenigsberg -
Douglas Schilling Landgraf -
masayag＠redhat.com -
smizrahi＠redhat.com