Fabian Deutsch has posted comments on this change. Change subject: module: persist/unpersist module should raise ......................................................................
Patch Set 9: Code-Review-1 (2 comments) https://gerrit.ovirt.org/#/c/43044/9/scripts/persist File scripts/persist: Line 40: if conf.exists(path): Line 41: print "Already persisted: %s" % path Line 42: continue Line 43: Line 44: ret_persist = conf.persist(path) The same as in th eother comment Line 45: Line 46: if ret_persist is None: Line 47: print "%s doesn't exist" % path Line 48: return -1 https://gerrit.ovirt.org/#/c/43044/9/scripts/unpersist File scripts/unpersist: Line 39: if not conf.exists(path): Line 40: print "File not explicitly persisted: %s" % path Line 41: continue Line 42: Line 43: ret_unpersist = conf.unpersist(path) I think we should catch the possible exception and keep the same (user visibe) output. If we just drop the error message ("Cannot unpersist …") then users might get confused. Also showing a stacktrace as the output is probably not so nice. Line 44: Line 45: print "%s successully unpersisted" % path Line 46: Line 47: return 0 -- To view, visit https://gerrit.ovirt.org/43044 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a917b2476957ef31e5ff452bfb01a4152b44f51 Gerrit-PatchSet: 9 Gerrit-Project: ovirt-node Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Fabian Deutsch <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ node-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/node-patches
