Change in vdsm[master]: virt: introduce support for periodic operations

fromani at redhat.com fromani at redhat.com
Tue Mar 31 06:25:21 UTC 2015


Francesco Romani has posted comments on this change.

Change subject: virt: introduce support for periodic operations
......................................................................


Patch Set 29:

(1 comment)

https://gerrit.ovirt.org/#/c/37593/29/tests/periodicTests.py
File tests/periodicTests.py:

Line 144:         def _work():
Line 145:             invokations[0] += 1
Line 146:             invokations[1] = monotonic_time()
Line 147:             if invokations[0] == BLOCK_AT:
Line 148:                 # must be > (PERIOD * TIMES) ~= forever
> may I ask why you opted against an explicit done.wait() in this thread?
Sure, unfortunately the answer is in the realm of personal preferences, but here it is:

IMO using time.sleep(), the code can be grokked in a single pass. It is straightforward to see what we do and why we do it that way.

Using done.wait() made me stop for a single tiny moment to think. Indeed was a tiny moment, but was a hiccup anyway: why is the same function using set() and wait() on the same event?!

For this reason I kept sleep(), which also mimics a little better the real world - an operation can be still blocked when we shut down things.

As I said first, this is 99% personal taste, so I have no real objections to switch to done()
Line 149:                 time.sleep(10 * PERIOD * TIMES)
Line 150:             executions[0] += 1
Line 151:             executions[1] = monotonic_time()
Line 152:             if invokations[0] == TIMES:


-- 
To view, visit https://gerrit.ovirt.org/37593
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I834c067b1d3dba16a1f8c83e555d2cc04cbf32ed
Gerrit-PatchSet: 29
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list