Yaniv Bronhaim has posted comments on this change.
Change subject: lib: Revert and refine error handling in tmap() ......................................................................
Patch Set 1:
(3 comments)
https://gerrit.ovirt.org/#/c/39211/1//COMMIT_MSG Commit Message:
Line 52: Line 53: succeeded = concurrent.tmap(func, values) Line 54: Line 55: if not all(succeeded): Line 56: ... We knew that... I'm not sure I understand why you changed your mind. checking each result is less efficient but you keep the error for later use and I thought your usage could require that Line 57: Line 58: We can ignore unexpected errors, since tmap() will log them and fail Line 59: loudly. We can also minimize try except block for expected errors. Line 60:
https://gerrit.ovirt.org/#/c/39211/1/lib/vdsm/concurrent.py File lib/vdsm/concurrent.py:
Line 66: t.join() Line 67: Line 68: if error[0] is not None: Line 69: t, v, tb = error[0] Line 70: raise t, v, tb just raise error[0]. and if you raise, the caller doesn't need to check - if not all(succeeded), just cover with try block Line 71:
https://gerrit.ovirt.org/#/c/39211/1/tests/concurrentTests.py File tests/concurrentTests.py:
Line 56: raise Error(x) Line 57: try: Line 58: concurrent.tmap(func, (1, 2, 3)) Line 59: except Error as e: Line 60: self.assertEqual(e.args, (3,)) can't you just check assertRaises ? simpler .. Line 61: else: