diff mbox series

[ovs-dev] python: Handle refTable values with setkey()

Message ID 20200314001121.7258-1-twilson@redhat.com
State Superseded
Headers show
Series [ovs-dev] python: Handle refTable values with setkey() | expand

Commit Message

Terry Wilson March 14, 2020, 12:11 a.m. UTC
For columns like QoS.queues where we have a map containing refTable
values, assigning w/ __setattr__ e.g. qos.queues={1: $queue_row}
works, but using using qos.setkey('queues', 1, $queue_row) results
in an Exception. The opdat argument can essentially just be the
JSON representation of the map column instead of trying to build
it.

Signed-off-by: Terry Wilson <twilson@redhat.com>
---
 python/ovs/db/idl.py    |  3 +--
 tests/idltest.ovsschema | 15 +++++++++++++++
 tests/ovsdb-idl.at      | 12 ++++++++++++
 tests/test-ovsdb.py     | 23 ++++++++++++++++++++++-
 4 files changed, 50 insertions(+), 3 deletions(-)

Comments

Numan Siddique March 19, 2020, 3:47 p.m. UTC | #1
On Sat, Mar 14, 2020 at 5:41 AM Terry Wilson <twilson@redhat.com> wrote:
>
> For columns like QoS.queues where we have a map containing refTable
> values, assigning w/ __setattr__ e.g. qos.queues={1: $queue_row}
> works, but using using qos.setkey('queues', 1, $queue_row) results
> in an Exception. The opdat argument can essentially just be the
> JSON representation of the map column instead of trying to build
> it.
>
> Signed-off-by: Terry Wilson <twilson@redhat.com>

Hi Terry,

The  below test case is failing for me. Can you please check

## openvswitch 2.13.90 test suite. ##
## ------------------------------- ##
2097: idl handling of missing tables and columns - C  FAILED (ovsdb-idl.at:956)


Thanks
Numan

> ---
>  python/ovs/db/idl.py    |  3 +--
>  tests/idltest.ovsschema | 15 +++++++++++++++
>  tests/ovsdb-idl.at      | 12 ++++++++++++
>  tests/test-ovsdb.py     | 23 ++++++++++++++++++++++-
>  4 files changed, 50 insertions(+), 3 deletions(-)
>
> diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
> index 020291d48..5850ac7ab 100644
> --- a/python/ovs/db/idl.py
> +++ b/python/ovs/db/idl.py
> @@ -1567,10 +1567,9 @@ class Transaction(object):
>                      for col, val in row._mutations['_inserts'].items():
>                          column = row._table.columns[col]
>                          if column.type.is_map():
> -                            opdat = ["map"]
>                              datum = data.Datum.from_python(column.type, val,
>                                                             _row_to_uuid)
> -                            opdat.append(datum.as_list())
> +                            opdat = self._substitute_uuids(datum.to_json())
>                          else:
>                              opdat = ["set"]
>                              inner_opdat = []
> diff --git a/tests/idltest.ovsschema b/tests/idltest.ovsschema
> index bee79fc50..e02b975bc 100644
> --- a/tests/idltest.ovsschema
> +++ b/tests/idltest.ovsschema
> @@ -171,6 +171,21 @@
>        },
>        "isRoot" : false
>      },
> +    "simple5": {
> +      "columns" : {
> +        "name": {"type": "string"},
> +        "irefmap": {
> +          "type": {
> +            "key": {"type": "integer"},
> +            "value": {"type": "uuid",
> +                      "refTable": "simple3"},
> +            "min": 0,
> +            "max": "unlimited"
> +          }
> +        }
> +      },
> +      "isRoot": true
> +    },
>      "singleton" : {
>        "columns" : {
>          "name" : {
> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
> index cc38d69c1..fc5844357 100644
> --- a/tests/ovsdb-idl.at
> +++ b/tests/ovsdb-idl.at
> @@ -1288,6 +1288,18 @@ OVSDB_CHECK_IDL_PY([partial-map idl],
>  009: done
>  ]])
>
> +OVSDB_CHECK_IDL_PY([partial-map update set refmap idl],
> +[['["idltest", {"op":"insert", "table":"simple3", "row":{"name":"myString1"}},
> +               {"op":"insert", "table":"simple5", "row":{"name":"myString2"}}]']],
> +['partialmapmutateirefmap'],
> +[[000: name=myString1 uset=[]
> +000: name=myString2 irefmap=[]
> +001: commit, status=success
> +002: name=myString1 uset=[]
> +002: name=myString2 irefmap=[(1 <0>)]
> +003: done
> +]])
> +
>  m4_define([OVSDB_CHECK_IDL_PARTIAL_UPDATE_SET_COLUMN],
>    [AT_SETUP([$1 - C])
>     AT_KEYWORDS([ovsdb server idl partial update set column positive $5])
> diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py
> index 1b94b79a0..a19680274 100644
> --- a/tests/test-ovsdb.py
> +++ b/tests/test-ovsdb.py
> @@ -28,6 +28,7 @@ import ovs.util
>  import ovs.vlog
>  from ovs.db import data
>  from ovs.db import error
> +from ovs.db.idl import _row_to_uuid as row_to_uuid
>  from ovs.fatal_signal import signal_alarm
>
>  vlog = ovs.vlog.Vlog("test-ovsdb")
> @@ -159,7 +160,8 @@ def get_simple_printable_row_string(row, columns):
>                                           is ovs.db.data.Atom):
>              value = getattr(row, column)
>              if isinstance(value, dict):
> -                value = sorted(value.items())
> +                value = sorted((row_to_uuid(k), row_to_uuid(v))
> +                               for k, v in value.items())
>              s += "%s=%s " % (column, value)
>      s = s.strip()
>      s = re.sub('""|,|u?\'', "", s)
> @@ -212,6 +214,14 @@ def print_idl(idl, step):
>              print(s)
>              n += 1
>
> +    if "simple5" in idl.tables:
> +        simple5 = idl.tables["simple5"].rows
> +        for row in simple5.values():
> +            s = "%03d: " % step
> +            s += get_simple_printable_row_string(row, ["name", "irefmap"])
> +            print(s)
> +            n += 1
> +
>      if "link1" in idl.tables:
>          l1 = idl.tables["link1"].rows
>          for row in l1.values():
> @@ -303,6 +313,11 @@ def idltest_find_simple3(idl, i):
>      return next(idl.index_equal("simple3", "simple3_by_name", i), None)
>
>
> +def idltest_find(idl, table, col, match):
> +    return next((r for r in idl.tables[table].rows.values() if
> +                getattr(r, col) == match), None)
> +
> +
>  def idl_set(idl, commands, step):
>      txn = ovs.db.idl.Transaction(idl)
>      increment = False
> @@ -524,6 +539,12 @@ def idl_set(idl, commands, step):
>              setattr(new_row3, 'name', 'String3')
>              new_row3.addvalue('uset', new_row41.uuid)
>              assert len(getattr(new_row3, 'uset', [])) == 1
> +        elif name == 'partialmapmutateirefmap':
> +            row3 = idltest_find_simple3(idl, "myString1")
> +            row5 = idltest_find(idl, "simple5", "name", "myString2")
> +            row5.setkey('irefmap', 1, row3.uuid)
> +            maplen = len(row5.irefmap)
> +            assert maplen == 1, "expected 1, got %d" % maplen
>          else:
>              sys.stderr.write("unknown command %s\n" % name)
>              sys.exit(1)
> --
> 2.21.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index 020291d48..5850ac7ab 100644
--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
@@ -1567,10 +1567,9 @@  class Transaction(object):
                     for col, val in row._mutations['_inserts'].items():
                         column = row._table.columns[col]
                         if column.type.is_map():
-                            opdat = ["map"]
                             datum = data.Datum.from_python(column.type, val,
                                                            _row_to_uuid)
-                            opdat.append(datum.as_list())
+                            opdat = self._substitute_uuids(datum.to_json())
                         else:
                             opdat = ["set"]
                             inner_opdat = []
diff --git a/tests/idltest.ovsschema b/tests/idltest.ovsschema
index bee79fc50..e02b975bc 100644
--- a/tests/idltest.ovsschema
+++ b/tests/idltest.ovsschema
@@ -171,6 +171,21 @@ 
       },
       "isRoot" : false
     },
+    "simple5": {
+      "columns" : {
+        "name": {"type": "string"},
+        "irefmap": {
+          "type": {
+            "key": {"type": "integer"},
+            "value": {"type": "uuid",
+                      "refTable": "simple3"},
+            "min": 0,
+            "max": "unlimited"
+          }
+        }
+      },
+      "isRoot": true
+    },
     "singleton" : {
       "columns" : {
         "name" : {
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index cc38d69c1..fc5844357 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -1288,6 +1288,18 @@  OVSDB_CHECK_IDL_PY([partial-map idl],
 009: done
 ]])
 
+OVSDB_CHECK_IDL_PY([partial-map update set refmap idl],
+[['["idltest", {"op":"insert", "table":"simple3", "row":{"name":"myString1"}},
+               {"op":"insert", "table":"simple5", "row":{"name":"myString2"}}]']],
+['partialmapmutateirefmap'],
+[[000: name=myString1 uset=[]
+000: name=myString2 irefmap=[]
+001: commit, status=success
+002: name=myString1 uset=[]
+002: name=myString2 irefmap=[(1 <0>)]
+003: done
+]])
+
 m4_define([OVSDB_CHECK_IDL_PARTIAL_UPDATE_SET_COLUMN],
   [AT_SETUP([$1 - C])
    AT_KEYWORDS([ovsdb server idl partial update set column positive $5])
diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py
index 1b94b79a0..a19680274 100644
--- a/tests/test-ovsdb.py
+++ b/tests/test-ovsdb.py
@@ -28,6 +28,7 @@  import ovs.util
 import ovs.vlog
 from ovs.db import data
 from ovs.db import error
+from ovs.db.idl import _row_to_uuid as row_to_uuid
 from ovs.fatal_signal import signal_alarm
 
 vlog = ovs.vlog.Vlog("test-ovsdb")
@@ -159,7 +160,8 @@  def get_simple_printable_row_string(row, columns):
                                          is ovs.db.data.Atom):
             value = getattr(row, column)
             if isinstance(value, dict):
-                value = sorted(value.items())
+                value = sorted((row_to_uuid(k), row_to_uuid(v))
+                               for k, v in value.items())
             s += "%s=%s " % (column, value)
     s = s.strip()
     s = re.sub('""|,|u?\'', "", s)
@@ -212,6 +214,14 @@  def print_idl(idl, step):
             print(s)
             n += 1
 
+    if "simple5" in idl.tables:
+        simple5 = idl.tables["simple5"].rows
+        for row in simple5.values():
+            s = "%03d: " % step
+            s += get_simple_printable_row_string(row, ["name", "irefmap"])
+            print(s)
+            n += 1
+
     if "link1" in idl.tables:
         l1 = idl.tables["link1"].rows
         for row in l1.values():
@@ -303,6 +313,11 @@  def idltest_find_simple3(idl, i):
     return next(idl.index_equal("simple3", "simple3_by_name", i), None)
 
 
+def idltest_find(idl, table, col, match):
+    return next((r for r in idl.tables[table].rows.values() if
+                getattr(r, col) == match), None)
+
+
 def idl_set(idl, commands, step):
     txn = ovs.db.idl.Transaction(idl)
     increment = False
@@ -524,6 +539,12 @@  def idl_set(idl, commands, step):
             setattr(new_row3, 'name', 'String3')
             new_row3.addvalue('uset', new_row41.uuid)
             assert len(getattr(new_row3, 'uset', [])) == 1
+        elif name == 'partialmapmutateirefmap':
+            row3 = idltest_find_simple3(idl, "myString1")
+            row5 = idltest_find(idl, "simple5", "name", "myString2")
+            row5.setkey('irefmap', 1, row3.uuid)
+            maplen = len(row5.irefmap)
+            assert maplen == 1, "expected 1, got %d" % maplen
         else:
             sys.stderr.write("unknown command %s\n" % name)
             sys.exit(1)