Nir Soffer has posted comments on this change. Change subject: virt: Limit the number of workers in executor ......................................................................
Patch Set 20: Code-Review-1 (3 comments) https://gerrit.ovirt.org/#/c/57754/20/tests/executorTests.py File tests/executorTests.py: Line 88 Line 89 Line 90 Line 91 Line 92 Old tests are very fragile, it would be nice to replace the sleeps with waiting for events. This is not related to this patch, but it adds nice infrastructure that these tests can use. Line 269: self.wait = wait Line 270: self.error = error Line 271: self.event = event Line 272: self.start_barrier = start_barrier Line 273: self.started = threading.Event() We are not using this event. Lets remove it, and rename start_barrier to started? Line 274: self.executed = threading.Event() Line 275: Line 276: def __call__(self): Line 277: self.started.set() Line 280: if self.wait: Line 281: time.sleep(self.wait) Line 282: if self.event is not None: Line 283: if not self.event.wait(10): Line 284: return Both wait and event have similar usage, making the task block for some time. Can we unify them and always pass an event instead of sleep time? Line 285: self.executed.set() Line 286: if self.error: Line 287: raise self.error Line 288: -- To view, visit https://gerrit.ovirt.org/57754 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iba56d91474c6b14a1cfe2db827b6fd61843a1db2 Gerrit-PatchSet: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
