[node-patches] Change in ovirt-node[master]: module: persist/unpersist module should raise

[fabiand at redhat.com]

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 <dougsland at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes