From patchwork Sat Aug 6 01:35:36 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ryan Moats X-Patchwork-Id: 656324 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (archives.nicira.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id 3s5mV508Wlz9t0J for ; Sat, 6 Aug 2016 11:36:12 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 3BE1410D4F; Fri, 5 Aug 2016 18:36:12 -0700 (PDT) X-Original-To: dev@openvswitch.com Delivered-To: dev@openvswitch.com Received: from fed1rmfepo201.cox.net (fed1rmfepo201.cox.net [68.230.241.146]) by archives.nicira.com (Postfix) with ESMTP id D919610D3C for ; Fri, 5 Aug 2016 18:36:10 -0700 (PDT) Received: from fed1rmimpo210.cox.net ([68.230.241.161]) by fed1rmfepo201.cox.net (InterMail vM.8.01.05.28 201-2260-151-171-20160122) with ESMTP id <20160806013610.HOAI18419.fed1rmfepo201.cox.net@fed1rmimpo210.cox.net> for ; Fri, 5 Aug 2016 21:36:10 -0400 Received: from Ryans-MacBook-Pro-4.local ([68.13.99.247]) by fed1rmimpo210.cox.net with cox id Tdc91t00C5LF6cs01dc9WC; Fri, 05 Aug 2016 21:36:10 -0400 X-CT-Class: Clean X-CT-Score: 0.00 X-CT-RefID: str=0001.0A090201.57A53F0A.001D, ss=1, re=0.000, recu=0.000, reip=0.000, cl=1, cld=1, fgs=0 X-CT-Spam: 0 X-Authority-Analysis: v=2.1 cv=YqGvP9sX c=1 sm=1 tr=0 a=Jmqd6mthTashISSy/JkQqg==:117 a=Jmqd6mthTashISSy/JkQqg==:17 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=7z1cN_iqozsA:10 a=VnNF1IyMAAAA:8 a=-o8F5S3qNiZrRnL6_I0A:9 a=skCgnbhlp52w9zbo2JeP:22 X-CM-Score: 0.00 Authentication-Results: cox.net; none Received: by Ryans-MacBook-Pro-4.local (Postfix, from userid 501) id B766262B556; Fri, 5 Aug 2016 20:36:08 -0500 (CDT) From: Ryan Moats To: dev@openvswitch.com Date: Fri, 5 Aug 2016 20:35:36 -0500 Message-Id: <1470447336-25959-3-git-send-email-rmoats@us.ibm.com> X-Mailer: git-send-email 2.7.4 (Apple Git-66) In-Reply-To: <1470447336-25959-1-git-send-email-rmoats@us.ibm.com> References: <1470447336-25959-1-git-send-email-rmoats@us.ibm.com> Subject: [ovs-dev] [PATCH v4 2/2] python: Add support for partial map and partial set updates X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dev-bounces@openvswitch.org Sender: "dev" Allow the python IDL to use mutate operations more freely by mimicing the partial map and partial set operations now available in the C IDL. Unit tests for both of these types of operations are included. They are not carbon copies of the C tests, because testing idempotency is a bit difficult for the current python IDL test harness. Signed-off-by: Ryan Moats --- python/ovs/db/idl.py | 196 ++++++++++++++++++++++++++++++++++++++++++++++++--- tests/ovsdb-idl.at | 30 ++++++++ tests/test-ovsdb.py | 88 +++++++++++++++++++++++ 3 files changed, 303 insertions(+), 11 deletions(-) diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py index 92a7382..6f376c7 100644 --- a/python/ovs/db/idl.py +++ b/python/ovs/db/idl.py @@ -18,6 +18,7 @@ import uuid import six import ovs.jsonrpc +import ovs.db.data as data import ovs.db.parser import ovs.db.schema from ovs.db import error @@ -588,8 +589,7 @@ class Idl(object): continue try: - datum_diff = ovs.db.data.Datum.from_json(column.type, - datum_diff_json) + datum_diff = 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" @@ -614,7 +614,7 @@ class Idl(object): continue try: - datum = ovs.db.data.Datum.from_json(column.type, datum_json) + datum = data.Datum.from_json(column.type, datum_json) except error.Error as e: # XXX rate-limit vlog.warn("error parsing column %s in table %s: %s" @@ -730,6 +730,24 @@ class Row(object): # - None, if this transaction deletes this row. self.__dict__["_changes"] = {} + # _mutations describes changes to this row to be handled via a + # mutate operation on the wire. It takes the following values: + # + # - {}, the empty dictionary, if no transaction is active or if the + # row has yet not been mutated within this transaction. + # + # - A dictionary that contains two keys: + # + # - "_inserts" contains a dictionary that maps column names to + # new keys/key-value pairs that should be inserted into the + # column + # - "_removes" contains a dictionary that maps column names to + # the keys/key-value pairs that should be removed from the + # column + # + # - None, if this transaction deletes this row. + self.__dict__["_mutations"] = {} + # A dictionary whose keys are the names of columns that must be # verified as prerequisites when the transaction commits. The values # in the dictionary are all None. @@ -750,17 +768,47 @@ class Row(object): def __getattr__(self, column_name): assert self._changes is not None + assert self._mutations is not None datum = self._changes.get(column_name) + inserts = None + if '_inserts' in self._mutations.keys(): + inserts = self._mutations['_inserts'].get(column_name) + removes = None + if '_removes' in self._mutations.keys(): + removes = self._mutations['_removes'].get(column_name) if datum is None: if self._data is None: - raise AttributeError("%s instance has no attribute '%s'" % - (self.__class__.__name__, column_name)) + if inserts is None: + raise AttributeError("%s instance has no attribute '%s'" % + (self.__class__.__name__, + column_name)) + else: + datum = inserts if column_name in self._data: datum = self._data[column_name] + try: + if inserts is not None: + datum.extend(inserts) + if removes is not None: + datum = [x for x in datum if x not in removes] + except error.Error: + pass + try: + if inserts is not None: + datum.merge(inserts) + if removes is not None: + for key in removes.keys(): + del datum[key] + except error.Error: + pass else: - raise AttributeError("%s instance has no attribute '%s'" % - (self.__class__.__name__, column_name)) + if inserts is None: + raise AttributeError("%s instance has no attribute '%s'" % + (self.__class__.__name__, + column_name)) + else: + datum = inserts return datum.to_python(_uuid_to_row) @@ -776,8 +824,7 @@ class Row(object): column = self._table.columns[column_name] try: - datum = ovs.db.data.Datum.from_python(column.type, value, - _row_to_uuid) + datum = data.Datum.from_python(column.type, value, _row_to_uuid) except error.Error as e: # XXX rate-limit vlog.err("attempting to write bad value to column %s (%s)" @@ -785,6 +832,74 @@ class Row(object): return self._idl.txn._write(self, column, datum) + def _init_mutations_if_needed(self): + if '_inserts' not in self._mutations.keys(): + self._mutations['_inserts'] = {} + if '_removes' not in self._mutations.keys(): + self._mutations['_removes'] = {} + return + + 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) + + 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) + + 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) + 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): + 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) + return + @classmethod def from_json(cls, idl, table, uuid, row_json): data = {} @@ -1024,6 +1139,7 @@ class Transaction(object): elif row._data is None: del row._table.rows[row.uuid] row.__dict__["_changes"] = {} + row.__dict__["_mutations"] = {} row.__dict__["_prereqs"] = {} self._txn_rows = {} @@ -1155,6 +1271,57 @@ class Transaction(object): if row._data is None or row_json: operations.append(op) + if row._mutations: + addop = False + op = {"table": row._table.name} + op["op"] = "mutate" + op["where"] = _where_uuid_equals(row.uuid) + op["mutations"] = [] + if '_removes' in row._mutations.keys(): + for col, dat in six.iteritems(row._mutations['_removes']): + column = row._table.columns[col] + if column.type.is_map(): + opdat = ["set"] + opdat.append(dat) + else: + opdat = ["set"] + inner_opdat = [] + for ele in dat: + try: + datum = data.Datum.from_python(column.type, + ele, _row_to_uuid) + except error.Error: + return + inner_opdat.append( + self._substitute_uuids(datum.to_json())) + opdat.append(inner_opdat) + mutation = [col, "delete", opdat] + op["mutations"].append(mutation) + addop = True + if '_inserts' in row._mutations.keys(): + for col, dat in six.iteritems(row._mutations['_inserts']): + column = row._table.columns[col] + if column.type.is_map(): + opdat = ["map"] + opdat.append(dat.as_list()) + else: + opdat = ["set"] + inner_opdat = [] + for ele in dat: + try: + datum = data.Datum.from_python(column.type, + ele, _row_to_uuid) + except error.Error: + return + inner_opdat.append( + self._substitute_uuids(datum.to_json())) + opdat.append(inner_opdat) + mutation = [col, "insert", opdat] + op["mutations"].append(mutation) + addop = True + if addop: + operations.append(op) + any_updates = True if self._fetch_requests: for fetch in self._fetch_requests: @@ -1281,6 +1448,7 @@ class Transaction(object): def _write(self, row, column, datum): assert row._changes is not None + assert row._mutations is not None txn = row._idl.txn @@ -1303,7 +1471,13 @@ class Transaction(object): return txn._txn_rows[row.uuid] = row - row._changes[column.name] = datum.copy() + 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() def insert(self, table, new_uuid=None): """Inserts and returns a new row in 'table', which must be one of the @@ -1419,7 +1593,7 @@ class Transaction(object): column = table.columns.get(column_name) datum_json = fetched_row.get(column_name) - datum = ovs.db.data.Datum.from_json(column.type, datum_json) + datum = data.Datum.from_json(column.type, datum_json) row._data[column_name] = datum update = True diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at index 243383b..c61d2e7 100644 --- a/tests/ovsdb-idl.at +++ b/tests/ovsdb-idl.at @@ -1108,6 +1108,19 @@ OVSDB_CHECK_IDL_PARTIAL_UPDATE_MAP_COLUMN([map, simple2 idl-partial-update-map-c 010: End test ]]) +OVSDB_CHECK_IDL_PY([partial-map idl], +[['["idltest", {"op":"insert", "table":"simple2", + "row":{"name":"myString1","smap":["map",[["key1","value1"],["key2","value2"]]]} }]'] +], + [?simple2:name,smap,imap 'partialmapinsertelement' 'partialmapdelelement'], +[[000: name=myString1 smap={key2: value2 key1: value1} imap={} +001: commit, status=success +002: name=String2 smap={key2: value2 key1: myList1} imap={3: myids2} +003: commit, status=success +004: name=String2 smap={key1: myList1} imap={3: myids2} +005: 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]) @@ -1144,6 +1157,23 @@ OVSDB_CHECK_IDL_PARTIAL_UPDATE_SET_COLUMN([set, simple3 idl-partial-update-set-c 012: End test ]]) +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'], +[[000: name=mySet1 uset=[<0> <1>] +001: commit, status=success +002: name=String2 uset=[<0> <1> <2>] +003: commit, status=success +004: name=String2 uset=[<0> <1> <2> <3>] +005: commit, status=success +006: name=String2 uset=[<0> <1> <3>] +007: commit, status=success +008: name=String2 uset=[<0> <1> <3>] +009: done +]]) + m4_define([OVSDB_CHECK_IDL_NOTIFY_PY], [AT_SETUP([$1 - Python]) AT_SKIP_IF([test $HAVE_PYTHON = no]) diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py index e9d4510..86f0168 100644 --- a/tests/test-ovsdb.py +++ b/tests/test-ovsdb.py @@ -166,6 +166,38 @@ def get_simple_table_printable_row(row): return s +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 + + +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 + + def print_idl(idl, step): n = 0 if "simple" in idl.tables: @@ -176,6 +208,22 @@ def print_idl(idl, step): print(s) n += 1 + if "simple2" in idl.tables: + simple2 = idl.tables["simple2"].rows + for row in six.itervalues(simple2): + s = "%03d: " % step + s += get_simple2_table_printable_row(row) + print(s) + n += 1 + + if "simple3" in idl.tables: + simple3 = idl.tables["simple3"].rows + for row in six.itervalues(simple3): + s = "%03d: " % step + s += get_simple3_table_printable_row(row) + print(s) + n += 1 + if "link1" in idl.tables: l1 = idl.tables["link1"].rows for row in six.itervalues(l1): @@ -246,6 +294,20 @@ def idltest_find_simple(idl, i): return None +def idltest_find_simple2(idl, i): + for row in six.itervalues(idl.tables["simple2"].rows): + if row.name == i: + return row + return None + + +def idltest_find_simple3(idl, i): + for row in six.itervalues(idl.tables["simple3"].rows): + if row.name == i: + return row + return None + + def idl_set(idl, commands, step): txn = ovs.db.idl.Transaction(idl) increment = False @@ -381,6 +443,32 @@ def idl_set(idl, commands, step): i = getattr(l1, 'i', 1) assert i == 2 l1.k = [l1] + elif name == 'partialmapinsertelement': + row = idltest_find_simple2(idl, 'myString1') + row.setkey('smap', 'key1', 'myList1') + row.setkey('imap', 3, 'myids2') + row.__setattr__('name', 'String2') + elif name == 'partialmapdelelement': + row = idltest_find_simple2(idl, 'String2') + row.delkey('smap', 'key2', 'value2') + 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': + row = idltest_find_simple3(idl, 'String2') + row.addvalue('uset', + uuid.UUID("0026b3ba-571b-4729-8227-d860a5210ab8")) + elif name == 'partialsetdel': + row = idltest_find_simple3(idl, 'String2') + row.delvalue('uset', + uuid.UUID("001e43d2-dd3f-4616-ab6a-83a490bb0991")) + elif name == 'partialsetref': + new_row = txn.insert(idl.tables["simple4"]) + new_row.__setattr__('name', 'test') + row = idltest_find_simple3(idl, 'String2') + row.addvalue('uref', new_row.uuid) else: sys.stderr.write("unknown command %s\n" % name) sys.exit(1)