[node-patches] Change in ovirt-node[master]: add return value when persist command does not return an err...

[fabiand at fedoraproject.org]

Fabian Deutsch has posted comments on this change.

Change subject: add return value when persist command does not return an error code on failure
......................................................................


Patch Set 7: -Code-Review

(1 comment)

I added a suggestion of how to make the func even more pythonic. But I don't have objections merging it now, so it finally lands.
The python-isation can also happen in a followup patch.

....................................................
File src/ovirtnode/ovirtfunctions.py
Line 819:                 system_closefds("echo "+filename+" >> /config/files")
Line 820:                 logger.info("Successfully persisted: " + filename)
Line 821:         else:
Line 822:             logger.warn(filename + " Already persisted")
Line 823:     return rc
Instead of returning rc you could raise different exceptions based on the rc value.
E.g. (in psuedo code):

    if rc == 4:
        raise FileAlreadyPersistedError([files_in_question])
    elif rc == ...:
        ...
    return True

Then you can do later on:

    try:
        ovirt_store_config_retnum()
    except FileAlreadyPersistedError as e:
        logger.warn("Files already persisted: %s" % e.already_persisted_files)

That would be the pythonic way.
Line 824: 
Line 825: 
Line 826: def ovirt_store_config_atomic(filename, source=None):
Line 827:     rc = True


-- 
To view, visit http://gerrit.ovirt.org/14737
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I458adeddeb17f63c5d003db808044ad6c8e9e097
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Changming Bai <baichm at linux.vnet.ibm.com>
Gerrit-Reviewer: Changming Bai <baichm at linux.vnet.ibm.com>
Gerrit-Reviewer: Eli Qiao <taget at linux.vnet.ibm.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at fedoraproject.org>
Gerrit-Reviewer: Joey Boggs <jboggs at redhat.com>
Gerrit-Reviewer: Michael Burns <mburns at redhat.com>
Gerrit-Reviewer: Wenyi Gao <wenyi at linux.vnet.ibm.com>
Gerrit-HasComments: Yes