[Roundup-users] Reactor functions get all old values, not just changed old values. (possible breaking change)

[John R. <ro...@ie...>]

Hello All:

I was working on getting some additional info put in place for
a reactor detector issue and discovered a mismatch between what is
documented and how the reactor is called.

The reactor signature is:

    def reactor(db, cl, nodeid, oldvalues):

 Currently the expression:

   "interesting_prop" in oldvalues

is always true even if interesting_prop wasn't changed in this transaction.

The spec (references.txt) for reactors says olddata (usually named
oldvalues in actual code) says:

    For a ``set()`` operation, ``olddata`` contains the names and previous
    values of properties that were changed.

oldvalues has the previous value for *all* properties of the object, not
just changed properties. This differs from an auditor where:

  "propname" in newdata

is a useful statement since newdata only has keys for properties being changed.

I think this is an old bug. It has been this way since at least version
0.5.0 AFAICT.

 I suggest changing the roundup core so that olddata has exactly the
same keys that newdata has so that I can use:

  if 'prop' not in oldvalues:
     return

successfully to return from a reactor if the props that trigger the
reactor are missing.

To do this I can change set() in class Class and class FileClass in hyperdb.py
to add:

  oldvalues = {prop: val for prop, val in oldvalues.items() if prop in
propvalues}

where oldvalues is a copy.deepcopy() of the object lookup before
anything is changed. propvalues is the newdata passed to an auditor.
The assignment above will be done after the auditor returns so it
picks up new values added to newdata. Then pass the filtered oldvalues
to the reactor.

Is anybody's reactor expecting olddata/oldvalues to have every value
for the object even those that have not changed? If the value is
unchanged it can be
looked up in the database.

I seem to remember a discussion about this a long time ago, but I can't
find it.

If this is considered a bad idea, I will update the reference.txt doc
to indicate
all of the properties of the object are passed to the reactor.

Thoughts?

-- rouilj