Nir Soffer 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 t Isn't this little over-engineered? Why do you care about checking discard? We can clean this up by moving discard to the object: self._discard_after(timeout) try: callable() except ... log... finally: self._cancel_discard() def _start_discard_timeout(self, timeout): self._discard = ... def _cancel_discard(self): if self._discard is not None: self._discard.cancel() self._discard = None This can also be easier to debug because we can access the timeout object of the blocked thread (assuming we get into the process using manhole or something similar). 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
