Nir Soffer has posted comments on this change. Change subject: configurator.py: fix _removeFile to do as described in it's doc string. ......................................................................
Patch Set 5: (2 comments) http://gerrit.ovirt.org/#/c/28782/5/lib/vdsm/tool/configurator.py File lib/vdsm/tool/configurator.py: Line 325: try: Line 326: os.unlink(content['path']) Line 327: except OSError as e: Line 328: if e.errno != errno.ENOENT: Line 329: raise InvalidRun("Removing file: %s failed", content['path']) > yep, not so good. you can still use the InvalidRun exception to avoid backt No, we *want* backtrace in this case! The InvalidRun and other errors were added to not show a backtrace in expected conditions, not to hide real errors that we cannot handle. Line 330: Line 331: def _unprefixAndRemoveSection(self, path): Line 332: """ Line 333: undo changes done by _prefixAndPrepend. Line 326: os.unlink(content['path']) Line 327: except OSError as e: Line 328: if e.errno != errno.ENOENT: Line 329: raise InvalidRun("Removing file: %s failed", content['path']) Line 330: > So should I go with : No, just raise - this is the case where the tool must crash. Something is very wrong with the environment, and the only way is to crash loudly and let the administrator fix the environment. Line 331: def _unprefixAndRemoveSection(self, path): Line 332: """ Line 333: undo changes done by _prefixAndPrepend. Line 334: """ -- To view, visit http://gerrit.ovirt.org/28782 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0436832ae63891c097038ef2b76606c30c40328a Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer <mta...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Douglas Schilling Landgraf <dougsl...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: mooli tayer <mta...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
