Open obfusk opened 9 months ago
I found more bugs with dict comparisons. The latest version should fix those.
Would probably be good if someone can run the test suite to make sure nothing breaks.
Deployed our ansible config with these changes. LGTM :)
I also looked at the code using this as best I could. I can't rule out it'll break something. And someone should definitely run the test suite. But it looks to me like none of the usages of object_changed()
in the ansible role should be adversely affected. And all of the edit_
functions in the API seem to use the same pattern of
data = self.get_monitor(id_)
data.update(kwargs)
Which means that there should be no unintended side-effects from the changes that now check that nested dicts and lists will always be considered changed if they are not the same size. The top-level dict is still considered a superset and subset, as it should be, with extra keys present in the superset not counting as changes.
So unless something breaks as a result of actual changes now being recognised as such when they were not before -- which means it relied on broken behaviour -- I do not expect any issues.
FWIW we've been using this in production since I opened this PR and have had zero issues whilst the bug (unable to clear notifications as setting the list to []
is not detected as a change) is indeed fixed.
Currently, e.g. setting a list value (that was not empty before) to
[]
will never be detected as a change as thefor
loop is never run. Neither will changing a list to any prefix of the old one. And it will crash if changing a list to a longer one if the list contains something other than lists or dicts as theif value[i] != value2[i]
doesn't check the length ofvalue2
:All of those correctly return
True
with this patch.I'm not sure this has any unintended side-effects as some existing code might accidentally rely on the existing behaviour, but it's clearly a bug.
Not yet tested with our actual ansible config, just those examples. Will test soon.