On Tue, Sep 22, 2026 at 3:31 PM Terry Wilson <[email protected]> wrote: > > The Python IDL's row_update2 "modify" handling always generated a > ROW_UPDATE notification and bumped change_seqno, even when the diff only > touched columns registered without an alert. This diverged from the C > IDL, whose ovsdb_idl_modify_row_by_diff() reports whether a monitored > (alerted) column actually changed and only then treats the row as > updated. > > Make _apply_diff() report whether an alerted column changed, and have > _process_update2()'s "modify" branch notify only in that case, matching > the C behavior. This is also a prerequisite for faithfully porting the > inconsistency detection that keys off modify_row_by_diff()'s return > value. > > The pre-existing comparison against row._data[column_name] produced the > same result but was obscure: for sets and maps diff() had already > updated that entry in place, so it compared an object against itself. > > There is no single C commit to port here: the alert gating originates in > C commit c547535a7 ("ovsdb-idl: Make it possible to omit or pay less > attention to columns.") applied to the row-update2 diff path added in > db2b57573 ("lib: add monitor2 support in ovsdb-idl."). This commit > brings the Python IDL in line with that long-standing C behavior. > > De-mangle __apply_diff/__process_update2 to single-underscore names as > they are modified here. > > Assisted-by: Claude Opus 4.8 > Signed-off-by: Terry Wilson <[email protected]> > --- > python/ovs/db/idl.py | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py > index fe504a63a..09ff16960 100644 > --- a/python/ovs/db/idl.py > +++ b/python/ovs/db/idl.py > @@ -962,7 +962,7 @@ class Idl(object): > self.cooperative_yield() > > if version in (OVSDB_UPDATE2, OVSDB_UPDATE3): > - changes = self.__process_update2(table, uuid, row_update) > + changes = self._process_update2(table, uuid, row_update) > if changes and tables is not self.server_tables: > notices.append(changes) > self.change_seqno += 1 > @@ -984,7 +984,7 @@ class Idl(object): > for notice in notices: > self.notify(*notice) > > - def __process_update2(self, table, uuid, row_update): > + def _process_update2(self, table, uuid, row_update): > """Returns Notice if a column changed, False otherwise.""" > row = table.rows.get(uuid) > if "delete" in row_update: > @@ -1015,9 +1015,11 @@ class Idl(object): > raise error.Error('Modify non-existing row') > > del table.rows[uuid] > - old_row = self.__apply_diff(table, row, row_update['modify']) > + old_row, changed = self._apply_diff(table, row, > + row_update['modify']) > table.rows[uuid] = row > - return Notice(ROW_UPDATE, row, Row(self, table, uuid, old_row)) > + if changed: > + return Notice(ROW_UPDATE, row, Row(self, table, uuid, > old_row)) > else: > raise error.Error('<row-update> unknown operation', > row_update) > @@ -1152,8 +1154,9 @@ class Idl(object): > if column.type.n_min != 0 and not column.type.is_map(): > row_update[column.name] = self.__column_name(column) > > - def __apply_diff(self, table, row, row_diff): > + def _apply_diff(self, table, row, row_diff): > old_row = {} > + changed = False > for column_name, datum_diff_json in row_diff.items(): > column = table.columns.get(column_name) > if not column: > @@ -1170,12 +1173,19 @@ class Idl(object): > % (column_name, table.name, e)) > continue > > - old_row[column_name] = row._data[column_name].copy() > + # Datum.diff() mutates the datum in place for sets and maps and > + # returns 'self', so compare the new value against the pre-diff > + # copy rather than against row._data[column_name] (which diff() > + # may already have updated). > + old = row._data[column_name].copy() > datum = row._data[column_name].diff(datum_diff) > - if datum != row._data[column_name]: > + if datum != old: > + old_row[column_name] = old > row._data[column_name] = datum > + if column.alert: > + changed = True > > - return old_row > + return old_row, changed > > def __row_update(self, table, row, row_json): > changed = False > -- > 2.55.0 > Fixes: 897c8064f55c ("python: move Python idl to work with monitor_cond")
_______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
