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