Francesco Romani has posted comments on this change.

Change subject: executor: introduce the executor library
......................................................................


Patch Set 32:

(1 comment)

http://gerrit.ovirt.org/#/c/29191/32/lib/vdsm/executor.py
File lib/vdsm/executor.py:

Line 195:         finally:
Line 196:             self._busy = False
Line 197:             if self._discarded:
Line 198:                 raise _WorkerDiscarded()
Line 199:             if discard is not None:
I don't really like this check. I think it is better and slightly cleaner to 
always discard timer after succesfull execution. To avoid that we can add an 
extra-simple private Fake (possibly a singleton to avoid wasteful 
allocation/deallocation):

  class _FakeDiscard(object):
    def cancel(self):
      pass

and return this in line 205 below. What do you think?
Line 200:                 discard.cancel()
Line 201: 
Line 202:     def _discard_after(self, timeout):
Line 203:         if timeout is not None:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic06da1ba57868dc2c7db67a1868ad10087a1cff2
Gerrit-PatchSet: 32
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Dima Kuznetsov <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[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