diff mbox

[ovs-dev] python: Send old values of the updated cols in notify for update2

Message ID eb82d066-34fc-4935-50d2-4a5708fc61e4@redhat.com
State Superseded
Headers show

Commit Message

Numan Siddique July 21, 2016, 10:21 a.m. UTC
When python IDL calls the "notify" function after processing the "update2"
message from ovsdb-server, it is suppose to send the old values of the
updated columns as the last parameter. But the recent commit "897c8064"
sends the updated values. This breaks the behaviour.
This patch fixes this issue. It also updates the description of
the 'updates' param of the notify function to make it more clear.

Fixes: 897c8064 ("python: move Python idl to work with monitor_cond")
Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
---
 python/ovs/db/idl.py | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

Comments

Russell Bryant July 25, 2016, 4:40 p.m. UTC | #1
On Thu, Jul 21, 2016 at 6:21 AM, Numan Siddique <nusiddiq@redhat.com> wrote:

> When python IDL calls the "notify" function after processing the "update2"
> message from ovsdb-server, it is suppose to send the old values of the
> updated columns as the last parameter. But the recent commit "897c8064"
> sends the updated values. This breaks the behaviour.
> This patch fixes this issue. It also updates the description of
> the 'updates' param of the notify function to make it more clear.
>
> Fixes: 897c8064 ("python: move Python idl to work with monitor_cond")
> Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
>

Is there a test case that could be updated or expanded to catch this issue?
Numan Siddique July 25, 2016, 6:31 p.m. UTC | #2
On Mon, Jul 25, 2016 at 10:10 PM, Russell Bryant <russell@ovn.org> wrote:

>
> On Thu, Jul 21, 2016 at 6:21 AM, Numan Siddique <nusiddiq@redhat.com>
> wrote:
>
>> When python IDL calls the "notify" function after processing the "update2"
>> message from ovsdb-server, it is suppose to send the old values of the
>> updated columns as the last parameter. But the recent commit "897c8064"
>> sends the updated values. This breaks the behaviour.
>> This patch fixes this issue. It also updates the description of
>> the 'updates' param of the notify function to make it more clear.
>>
>> Fixes: 897c8064 ("python: move Python idl to work with monitor_cond")
>> Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
>>
>
> Is there a test case that could be updated or expanded to catch this issue?
>
>
>
​I dont think there is any. I will add a new one.

​


> --
> Russell Bryant
>
diff mbox

Patch

diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index 437e9b0..92a7382 100644
--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
@@ -333,7 +333,8 @@  class Idl(object):
         :type event:    ROW_CREATE, ROW_UPDATE, or ROW_DELETE
         :param row:     The row as it is after the operation has occured
         :type row:      Row
-        :param updates: For updates, row with only updated columns
+        :param updates: For updates, row with only old values of the changed
+                        columns
         :type updates:  Row
         """
 
@@ -511,9 +512,10 @@  class Idl(object):
             if not row:
                 raise error.Error('Modify non-existing row')
 
-            self.__apply_diff(table, row, row_update['modify'])
+            old_row_diff_json = self.__apply_diff(table, row,
+                                                  row_update['modify'])
             self.notify(ROW_UPDATE, row,
-                        Row.from_json(self, table, uuid, row_update['modify']))
+                        Row.from_json(self, table, uuid, old_row_diff_json))
             changed = True
         else:
             raise error.Error('<row-update> unknown operation',
@@ -576,7 +578,8 @@  class Idl(object):
                         row_update[column.name] = self.__column_name(column)
 
     def __apply_diff(self, table, row, row_diff):
-        for column_name, datum_json in six.iteritems(row_diff):
+        old_row_diff_json = {}
+        for column_name, datum_diff_json in six.iteritems(row_diff):
             column = table.columns.get(column_name)
             if not column:
                 # XXX rate-limit
@@ -585,17 +588,21 @@  class Idl(object):
                 continue
 
             try:
-                datum = ovs.db.data.Datum.from_json(column.type, datum_json)
+                datum_diff = ovs.db.data.Datum.from_json(column.type,
+                                                         datum_diff_json)
             except error.Error as e:
                 # XXX rate-limit
                 vlog.warn("error parsing column %s in table %s: %s"
                           % (column_name, table.name, e))
                 continue
 
-            datum = row._data[column_name].diff(datum)
+            old_row_diff_json[column_name] = row._data[column_name].to_json()
+            datum = row._data[column_name].diff(datum_diff)
             if datum != row._data[column_name]:
                 row._data[column_name] = datum
 
+        return old_row_diff_json
+
     def __row_update(self, table, row, row_json):
         changed = False
         for column_name, datum_json in six.iteritems(row_json):