[ovs-dev] [PATCH v1] Python IDL Partial Set and Map Updates

Amitabha Biswas azbiswas at gmail.com
Mon Aug 15 09:21:55 UTC 2016


This patch fixes a couple of bugs in commit a59912a0
(python: add support for partial map and partial set updates)
and reverses a simplication added in commit 884d9bad
(Simplify partial map Py3 IDL test) to make the Python3 test
cases passes.

The following changes have been made:

1. Allow multiple map updates on the same column in a transaction.
2. Partial map Py3 IDL test can now support multiple elements.
3. SetAttr overrides pre-existing insert and remove updates.
4. addvalue/delvalue contains unique elements

Signed-off-by: Amitabha Biswas <abiswas at us.ibm.com>
---
 python/ovs/db/idl.py | 111 ++++++++++++++++++++++++++-------------------------
 tests/ovsdb-idl.at   |  20 ++++++----
 tests/test-ovsdb.py  |  63 +++++++++++++++--------------
 3 files changed, 101 insertions(+), 93 deletions(-)

diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index 6f376c7..96b668c 100644
--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
@@ -841,63 +841,66 @@ class Row(object):
 
     def addvalue(self, column_name, key):
         self._idl.txn._txn_rows[self.uuid] = self
-        self._init_mutations_if_needed()
-        if column_name in self._data.keys():
-            if column_name not in self._mutations['_inserts'].keys():
-                self._mutations['_inserts'][column_name] = []
-            self._mutations['_inserts'][column_name].append(key)
+        column = self._table.columns[column_name]
+        try:
+            data.Datum.from_python(column.type, key, _row_to_uuid)
+        except error.Error as e:
+            # XXX rate-limit
+            vlog.err("attempting to write bad value to column %s (%s)"
+                     % (column_name, e))
+            return
+        inserts = self._mutations.setdefault('_inserts', {})
+        column_value = inserts.setdefault(column_name, set())
+        column_value.add(key)
 
     def delvalue(self, column_name, key):
         self._idl.txn._txn_rows[self.uuid] = self
-        self._init_mutations_if_needed()
-        if column_name in self._data.keys():
-            if column_name not in self._mutations['_removes'].keys():
-                self._mutations['_removes'][column_name] = []
-            self._mutations['_removes'][column_name].append(key)
+        column = self._table.columns[column_name]
+        try:
+            data.Datum.from_python(column.type, key, _row_to_uuid)
+        except error.Error as e:
+            # XXX rate-limit
+            vlog.err("attempting to delete bad value from column %s (%s)"
+                     % (column_name, e))
+            return
+        removes = self._mutations.setdefault('_removes', {})
+        column_value = removes.setdefault(column_name, set())
+        column_value.add(key)
 
     def setkey(self, column_name, key, value):
         self._idl.txn._txn_rows[self.uuid] = self
         column = self._table.columns[column_name]
         try:
-            datum = data.Datum.from_python(column.type, {key: value},
-                                           _row_to_uuid)
+            data.Datum.from_python(column.type, {key: value}, _row_to_uuid)
         except error.Error as e:
             # XXX rate-limit
             vlog.err("attempting to write bad value to column %s (%s)"
                      % (column_name, e))
             return
-        self._init_mutations_if_needed()
-        if column_name in self._data.keys():
-            if column_name not in self._mutations['_removes'].keys():
-                self._mutations['_removes'][column_name] = []
-            self._mutations['_removes'][column_name].append(key)
-        if self._mutations['_inserts'] is None:
-            self._mutations['_inserts'] = {}
-        self._mutations['_inserts'][column_name] = datum
-
-    def delkey(self, column_name, key):
+        if column_name in self._data:
+            # Remove existing key/value before updating.
+            removes = self._mutations.setdefault('_removes', {})
+            column_value = removes.setdefault(column_name, set())
+            column_value.add(key)
+        inserts = self._mutations.setdefault('_inserts', {})
+        column_value = inserts.setdefault(column_name, {})
+        column_value[key] = value
+
+    def delkey(self, column_name, key, value=None):
         self._idl.txn._txn_rows[self.uuid] = self
-        self._init_mutations_if_needed()
-        if column_name not in self._mutations['_removes'].keys():
-            self._mutations['_removes'][column_name] = []
-        self._mutations['_removes'][column_name].append(key)
-
-    def delkey(self, column_name, key, value):
-        self._idl.txn._txn_rows[self.uuid] = self
-        try:
-            old_value = data.Datum.to_python(self._data[column_name],
-                                             _uuid_to_row)
-        except error.Error:
-            return
-        if key not in old_value.keys():
-            return
-        if old_value[key] != value:
-            return
-
-        self._init_mutations_if_needed()
-        if column_name not in self._mutations['_removes'].keys():
-            self._mutations['_removes'][column_name] = []
-        self._mutations['_removes'][column_name].append(key)
+        if value:
+            try:
+                old_value = data.Datum.to_python(self._data[column_name],
+                                                 _uuid_to_row)
+            except error.Error:
+                return
+            if key not in old_value:
+                return
+            if old_value[key] != value:
+                return
+        removes = self._mutations.setdefault('_removes', {})
+        column_value = removes.setdefault(column_name, set())
+        column_value.add(key)
         return
 
     @classmethod
@@ -1282,7 +1285,7 @@ class Transaction(object):
                         column = row._table.columns[col]
                         if column.type.is_map():
                             opdat = ["set"]
-                            opdat.append(dat)
+                            opdat.append(list(dat))
                         else:
                             opdat = ["set"]
                             inner_opdat = []
@@ -1299,15 +1302,17 @@ class Transaction(object):
                         op["mutations"].append(mutation)
                         addop = True
                 if '_inserts' in row._mutations.keys():
-                    for col, dat in six.iteritems(row._mutations['_inserts']):
+                    for col, val in six.iteritems(row._mutations['_inserts']):
                         column = row._table.columns[col]
                         if column.type.is_map():
                             opdat = ["map"]
-                            opdat.append(dat.as_list())
+                            datum = data.Datum.from_python(column.type, val,
+                                                           _row_to_uuid)
+                            opdat.append(datum.as_list())
                         else:
                             opdat = ["set"]
                             inner_opdat = []
-                            for ele in dat:
+                            for ele in val:
                                 try:
                                     datum = data.Datum.from_python(column.type,
                                         ele, _row_to_uuid)
@@ -1471,13 +1476,11 @@ class Transaction(object):
                 return
 
         txn._txn_rows[row.uuid] = row
-        if '_inserts' in row._mutations.keys():
-            if column.name in row._mutations['_inserts']:
-                row._mutations['_inserts'][column.name] = datum.copy()
-            else:
-                row._changes[column.name] = datum.copy()
-        else:
-            row._changes[column.name] = datum.copy()
+        if '_inserts' in row._mutations:
+            row._mutations['_inserts'].pop(column.name, None)
+        if '_removes' in row._mutations:
+            row._mutations['_removes'].pop(column.name, None)
+        row._changes[column.name] = datum.copy()
 
     def insert(self, table, new_uuid=None):
         """Inserts and returns a new row in 'table', which must be one of the
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index b2899b6..2804419 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -1110,15 +1110,17 @@ OVSDB_CHECK_IDL_PARTIAL_UPDATE_MAP_COLUMN([map, simple2 idl-partial-update-map-c
 
 OVSDB_CHECK_IDL_PY([partial-map idl],
 [['["idltest", {"op":"insert", "table":"simple2",
-                "row":{"name":"myString1","smap":["map",[["key1","value1"]]]} }]']
+                "row":{"name":"myString1","smap":["map",[["key1","value1"],["key2","value2"]]]} }]']
 ],
-  [?simple2:name,smap,imap 'partialmapinsertelement' 'partialmapdelelement'],
-[[000: name=myString1 smap={key1: value1} imap={}
+  [?simple2:name,smap,imap 'partialmapinsertelement' 'partialmapinsertmultipleelements' 'partialmapdelelements'],
+[[000: name=myString1 smap=[(key1 value1) (key2 value2)] imap=[]
 001: commit, status=success
-002: name=String2 smap={key1: myList1} imap={3: myids2}
+002: name=String2 smap=[(key1 myList1) (key2 value2)] imap=[(3 myids2)]
 003: commit, status=success
-004: name=String2 smap={} imap={3: myids2}
-005: done
+004: name=String2 smap=[(key1 myList1) (key2 myList2) (key3 myList3)] imap=[(3 myids2)]
+005: commit, status=success
+006: name=String2 smap=[(key2 myList2)] imap=[(3 myids2)]
+007: done
 ]])
 
 m4_define([OVSDB_CHECK_IDL_PARTIAL_UPDATE_SET_COLUMN],
@@ -1161,7 +1163,7 @@ OVSDB_CHECK_IDL_PY([partial-set idl],
 [['["idltest", {"op":"insert", "table":"simple3",
                 "row":{"name":"mySet1","uset":["set", [[ "uuid", "0005b872-f9e5-43be-ae02-3184b9680e75" ], [ "uuid", "000d2f6a-76af-412f-b59d-e7bcd3e84eff" ]]]} }, {"op":"insert", "table":"simple4", "row":{"name":"seed"}}]']
 ],
-  ['partialrenamesetadd' 'partialsetadd2' 'partialsetdel' 'partialsetref'],
+  ['partialrenamesetadd' 'partialduplicateadd' 'partialsetdel' 'partialsetref' 'partialsetoverrideops'],
 [[000: name=mySet1 uset=[<0> <1>]
 001: commit, status=success
 002: name=String2 uset=[<0> <1> <2>]
@@ -1171,7 +1173,9 @@ OVSDB_CHECK_IDL_PY([partial-set idl],
 006: name=String2 uset=[<0> <1> <3>]
 007: commit, status=success
 008: name=String2 uset=[<0> <1> <3>]
-009: done
+009: commit, status=success
+010: name=String2 uset=[<3>]
+011: done
 ]])
 
 m4_define([OVSDB_CHECK_IDL_NOTIFY_PY],
diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py
index 2ea2bd5..e4e3395 100644
--- a/tests/test-ovsdb.py
+++ b/tests/test-ovsdb.py
@@ -149,14 +149,15 @@ def do_parse_schema(schema_string):
     print(ovs.json.to_string(schema.to_json(), sort_keys=True))
 
 
-def get_simple_table_printable_row(row):
-    simple_columns = ["i", "r", "b", "s", "u", "ia",
-                      "ra", "ba", "sa", "ua", "uuid"]
+def get_simple_printable_row_string(row, columns):
     s = ""
-    for column in simple_columns:
+    for column in columns:
         if hasattr(row, column) and not (type(getattr(row, column))
                                          is ovs.db.data.Atom):
-            s += "%s=%s " % (column, getattr(row, column))
+            value = getattr(row, column)
+            if isinstance(value, dict):
+                value = sorted(value.items())
+            s += "%s=%s " % (column, value)
     s = s.strip()
     s = re.sub('""|,|u?\'', "", s)
     s = re.sub('UUID\(([^)]+)\)', r'\1', s)
@@ -166,36 +167,20 @@ def get_simple_table_printable_row(row):
     return s
 
 
+def get_simple_table_printable_row(row):
+    simple_columns = ["i", "r", "b", "s", "u", "ia",
+                      "ra", "ba", "sa", "ua", "uuid"]
+    return get_simple_printable_row_string(row, simple_columns)
+
+
 def get_simple2_table_printable_row(row):
     simple2_columns = ["name", "smap", "imap"]
-    s = ""
-    for column in simple2_columns:
-        if hasattr(row, column) and not (type(getattr(row, column))
-                                         is ovs.db.data.Atom):
-            s += "%s=%s " % (column, getattr(row, column))
-    s = s.strip()
-    s = re.sub('""|,|u?\'', "", s)
-    s = re.sub('UUID\(([^)]+)\)', r'\1', s)
-    s = re.sub('False', 'false', s)
-    s = re.sub('True', 'true', s)
-    s = re.sub(r'(ba)=([^[][^ ]*) ', r'\1=[\2] ', s)
-    return s
+    return get_simple_printable_row_string(row, simple2_columns)
 
 
 def get_simple3_table_printable_row(row):
     simple3_columns = ["name", "uset"]
-    s = ""
-    for column in simple3_columns:
-        if hasattr(row, column) and not (type(getattr(row, column))
-                                         is ovs.db.data.Atom):
-            s += "%s=%s " % (column, getattr(row, column))
-    s = s.strip()
-    s = re.sub('""|,|u?\'', "", s)
-    s = re.sub('UUID\(([^)]+)\)', r'\1', s)
-    s = re.sub('False', 'false', s)
-    s = re.sub('True', 'true', s)
-    s = re.sub(r'(ba)=([^[][^ ]*) ', r'\1=[\2] ', s)
-    return s
+    return get_simple_printable_row_string(row, simple3_columns)
 
 
 def print_idl(idl, step):
@@ -448,18 +433,26 @@ def idl_set(idl, commands, step):
             row.setkey('smap', 'key1', 'myList1')
             row.setkey('imap', 3, 'myids2')
             row.__setattr__('name', 'String2')
-        elif name == 'partialmapdelelement':
+        elif name == 'partialmapinsertmultipleelements':
+            row = idltest_find_simple2(idl, 'String2')
+            row.setkey('smap', 'key2', 'myList2')
+            row.setkey('smap', 'key3', 'myList3')
+        elif name == 'partialmapdelelements':
             row = idltest_find_simple2(idl, 'String2')
             row.delkey('smap', 'key1', 'myList1')
+            row.delkey('smap', 'key2', 'wrongvalue')
+            row.delkey('smap', 'key3')
         elif name == 'partialrenamesetadd':
             row = idltest_find_simple3(idl, 'mySet1')
             row.addvalue('uset',
                          uuid.UUID("001e43d2-dd3f-4616-ab6a-83a490bb0991"))
             row.__setattr__('name', 'String2')
-        elif name == 'partialsetadd2':
+        elif name == 'partialduplicateadd':
             row = idltest_find_simple3(idl, 'String2')
             row.addvalue('uset',
                          uuid.UUID("0026b3ba-571b-4729-8227-d860a5210ab8"))
+            row.addvalue('uset',
+                         uuid.UUID("0026b3ba-571b-4729-8227-d860a5210ab8"))
         elif name == 'partialsetdel':
             row = idltest_find_simple3(idl, 'String2')
             row.delvalue('uset',
@@ -469,6 +462,14 @@ def idl_set(idl, commands, step):
             new_row.__setattr__('name', 'test')
             row = idltest_find_simple3(idl, 'String2')
             row.addvalue('uref', new_row.uuid)
+        elif name == 'partialsetoverrideops':
+            row = idltest_find_simple3(idl, 'String2')
+            row.addvalue('uset',
+                         uuid.UUID("579e978d-776c-4f19-a225-268e5890e670"))
+            row.delvalue('uset',
+                         uuid.UUID("0026b3ba-571b-4729-8227-d860a5210ab8"))
+            row.__setattr__('uset',
+                [uuid.UUID("0026b3ba-571b-4729-8227-d860a5210ab8")])
         else:
             sys.stderr.write("unknown command %s\n" % name)
             sys.exit(1)
-- 
2.7.4 (Apple Git-66)




More information about the dev mailing list