[node-patches] Change in ovirt-node[ovirt-3.5]: Config.exists: fix exists method

[fabiand at redhat.com]

Fabian Deutsch has posted comments on this change.

Change subject: Config.exists: fix exists method
......................................................................


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/44727/2/src/ovirt/node/utils/fs/__init__.py
File src/ovirt/node/utils/fs/__init__.py:

Line 676:         if filename and self.is_enabled():
Line 677:             from ovirtnode import ovirtfunctions
Line 678:             return ovirtfunctions.ovirt_safe_delete_config(filename)
Line 679: 
Line 680:     def exists(self, filename):
> I believe we improving not changing semantics. There were cases where files
The issue is that the exists() call is now behaving different than before.
And that is considered an API breakage.
Before it was returned True when the path was persisted. Now True is returned if the contents of the persisted and target file differ.

Imagine this flow:

    f = "/my/file"
    c = Config()
    if not c.exists(f):
        c.persist(f)  # Copy on /config is created and bind mounted
    change(f)
    if not c.exists(f):
        c.persist(f)  # Nothing happens

Without your change this works, because f will only be persisted if it is not on /config.

With your change it will probably fail, because persist() (AFAIK) does only persist files, it does not "update" them.
So with your change:

    f = "/my/file"
    c = Config()
    if not c.exists(f):
        c.persist(f)  # Copy on /config is created and bind mounted
    change(f)
    if not c.exists(f):  # Is now true, bvecause f is changed
        c.persist(f)  # ERROR because f is already persisted

I would be more open to this change if persist will re-persist ("sync") a file if it is already persisted.
Line 681:         """Check if the given file is persisted
Line 682:         """
Line 683:         persisted_path = self._config_path(filename)
Line 684: 


-- 
To view, visit https://gerrit.ovirt.org/44727
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If1ba23d67516f1690b223e3dd43985e5d9a502ef
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-node
Gerrit-Branch: ovirt-3.5
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: automation at ovirt.org
Gerrit-HasComments: Yes