Dan Kenigsberg has posted comments on this change. Change subject: vdsm: storage replace/remove too generic except handlers ......................................................................
Patch Set 2: Code-Review-1 (2 comments) very partial review. I think that we should use "except:" as pointers to find bad code. Once found, the bad code should be truly fixed. .................................................... File vdsm/storage/dispatcher.py Line 40: self.help = None Line 41: try: Line 42: if hasattr(func.im_self, "help"): Line 43: self.help = getattr(func.im_self, "help") Line 44: except AttributeError: how can AttributeError be raised here? self.help = getattr(func.im_self, "help", None) or getattr(func, "__doc__", "No help available for method %s" % name) is a shorter equivalent of the current code... Line 45: pass Line 46: Line 47: if not self.help: Line 48: try: .................................................... File vdsm/storage/hsm.py Line 3481: exc_info=True) Line 3482: continue Line 3483: Line 3484: self.taskMng.prepareForShutdown() Line 3485: except Exception: keeping the silent "pass" here is a sin. drop the try-block - our only caller should be able to test if this failed. Line 3486: pass Line 3487: Line 3488: @classmethod Line 3489: def __releaseLocks(cls): -- To view, visit http://gerrit.ovirt.org/17751 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia09dcada40bca3f16f94ad7a6c83e6d7a85ade77 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: Better Saggi <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Eduardo <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Ohad Basan <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Yeela Kaplan <[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
