Francesco Romani has posted comments on this change. Change subject: utils: Add @throttle decorator ......................................................................
Patch Set 1: (2 comments) https://gerrit.ovirt.org/#/c/47087/1/vdsm/virt/utils.py File vdsm/virt/utils.py: Line 128: Line 129: When block=True we will wait to obtain the semaphore and Line 130: release it after wrapped method execution. Line 131: Line 132: When block=False we won't wait to obtain the semaphore and > Not to my knowledge. It seemed like it could be useful to make this more ge My problem here is that I really don't like 'leeking' decorators. Block=False creates an asymmetry: decorator automatically acquire a resource, which must be released later and manually. This is error prone, and feels awkward. If we don't have a crystal clear use case, I'd drop this and make this decorator self-contained. Line 133: in case this fails, will return the specified error. If the execution of Line 134: wrapped method succeeds, it is the responsibility of called code to Line 135: release the semaphore (i.e. method is assumed to be async) Line 136: Line 155: except Exception: Line 156: success = False Line 157: raise Line 158: finally: Line 159: if not success or block: whoa, I missed this. We must either found a way to *automatically* release this semaphore, or rearrange this code. Sorry, but I can't really ignore the very bad feeling I have here. Automatic acquisition + manual release feels leaky. Line 160: semaphore.release() Line 161: Line 162: return wrapper Line 163: -- To view, visit https://gerrit.ovirt.org/47087 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iab4cf387f77a9b720afb7794e27ed54efa0d3e3c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Betak <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Betak <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
