)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"97ce7e83bdf5c3424574b2e6f49e3e56d10e6b71","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"57181d58_eea50093","updated":"2023-02-07 23:59:38.000000000","message":"Proactively set WF-1 while upstream is in feature freeze.","commit_id":"1ea369c1525b3330d7c6d3ffc965e3cdfb8e1bda"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"e1552b573e2563466d088c1b94e5be3b9f1a6d40","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c0de2f4b_44986ced","updated":"2023-02-16 09:02:33.000000000","message":"Just technical -1 to wait for the spec: https://review.opendev.org/c/openstack/neutron-specs/+/870030","commit_id":"3bddf1ee7c188675065abf3cf5826533f815aa36"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"45e534e8f2f7289a3cc067ee796732380c8d2495","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"68e62193_c47f9497","updated":"2023-04-14 16:41:41.000000000","message":"Looks good, just a couple of questions/suggestions. Thanks!","commit_id":"3bddf1ee7c188675065abf3cf5826533f815aa36"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"ef7af243e86e4085e5d57c9620a84427669effc5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c77ba78c_d18a00e7","updated":"2023-04-19 13:21:42.000000000","message":"Thank you for the review, Terry, much appreciated. I have an answer for one of your questions and will follow up with an updated patch set shortly based on two of your recommendations.","commit_id":"3bddf1ee7c188675065abf3cf5826533f815aa36"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"f82132a247b93335b3130699a4dc51e4afdad0ab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"8d86fa47_cdb2de1e","updated":"2023-03-31 06:28:32.000000000","message":"The spec has now merged [0] and there is an implementation consuming this change up for review [1].\n\n0: https://review.opendev.org/c/openstack/neutron-specs/+/870030\n1: https://review.opendev.org/c/openstack/neutron/+/878543","commit_id":"3bddf1ee7c188675065abf3cf5826533f815aa36"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ee192033d34ad2498947d08435de42b191683410","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"a5465f5d_2d6ca739","updated":"2023-04-19 15:22:22.000000000","message":"If I\u0027m not wrong, all Terry\u0027s comments are addressed.","commit_id":"73b19d6a0af47af4012b349ae0dd1e54b6cf5132"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"016187b65cff2fc8892674ff3716ee6e88c54a68","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b31b21d1_e7886523","in_reply_to":"a5465f5d_2d6ca739","updated":"2023-04-19 15:31:47.000000000","message":"Thank you, Rodolfo, added one more update to address the extraneous lookup. Apologies for the extra iteration, and hope you still find the patch set in good shape.","commit_id":"73b19d6a0af47af4012b349ae0dd1e54b6cf5132"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"f5795b1b26e101a2a63489592183a396aa806b0e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"cad77158_65c569a5","updated":"2023-04-20 13:56:19.000000000","message":"Looks good to me! Thanks so much for the patch!","commit_id":"f8fe13af906004380d9b8f040586383cffb60733"}],"ovsdbapp/schema/ovn_northbound/commands.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"45e534e8f2f7289a3cc067ee796732380c8d2495","unresolved":true,"context_lines":[{"line_number":1041,"context_line":"        #"},{"line_number":1042,"context_line":"        # The base cmd.DbFindCommand only support one condition, so we perform"},{"line_number":1043,"context_line":"        # index match on logical_port and then filter on dst_ip."},{"line_number":1044,"context_line":"        self.result \u003d ["},{"line_number":1045,"context_line":"            r"},{"line_number":1046,"context_line":"            for r in self.result if r[\u0027dst_ip\u0027] \u003d\u003d self.dst_ip"},{"line_number":1047,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":4,"id":"67c9eb3b_d503a95d","line":1044,"updated":"2023-04-14 16:41:41.000000000","message":"It looks like db_find allows multiple conditions to be passed. Is this not working?\n```\n    @abc.abstractmethod\n    def db_find(self, table, *conditions, **kwargs):\n        \"\"\"Create a command to return find OVSDB records matching conditions\n\n        :param table:     The OVS table to query\n        :type table:      string\n        :param conditions:The conditions to satisfy the query\n        :type conditions: 3-tuples containing (column, operation, match)\n                          Type of \u0027match\u0027 parameter MUST be identical to column\n                          type\n                          Examples:\n                              atomic: (\u0027tag\u0027, \u0027\u003d\u0027, 7)\n                              map: (\u0027external_ids\u0027 \u0027\u003d\u0027, {\u0027iface-id\u0027: \u0027xxx\u0027})\n                              field exists?\n                                  (\u0027external_ids\u0027, \u0027!\u003d\u0027, {\u0027iface-id\u0027, \u0027\u0027})\n                              set contains?:\n                                  (\u0027protocols\u0027, \u0027{\u003e\u003d}\u0027, \u0027OpenFlow13\u0027)\n                          See the ovs-vsctl man page for more operations\n        :param columns:   Limit results to only columns, None means all columns\n        :type columns:    list of column names or None\n        :returns:         :class:`Command` with [{\u0027column\u0027, value}, ...] result\n        \"\"\"\n```","commit_id":"3bddf1ee7c188675065abf3cf5826533f815aa36"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"8738bc5131ddac29634f57866a0e56aaa7cf4b88","unresolved":false,"context_lines":[{"line_number":1041,"context_line":"        #"},{"line_number":1042,"context_line":"        # The base cmd.DbFindCommand only support one condition, so we perform"},{"line_number":1043,"context_line":"        # index match on logical_port and then filter on dst_ip."},{"line_number":1044,"context_line":"        self.result \u003d ["},{"line_number":1045,"context_line":"            r"},{"line_number":1046,"context_line":"            for r in self.result if r[\u0027dst_ip\u0027] \u003d\u003d self.dst_ip"},{"line_number":1047,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":4,"id":"937720a8_83bcc0a9","line":1044,"in_reply_to":"2f7c3031_aa4e7083","updated":"2023-04-19 13:49:32.000000000","message":"Ah, of course, thank you for clearing that up!","commit_id":"3bddf1ee7c188675065abf3cf5826533f815aa36"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"ef7af243e86e4085e5d57c9620a84427669effc5","unresolved":true,"context_lines":[{"line_number":1041,"context_line":"        #"},{"line_number":1042,"context_line":"        # The base cmd.DbFindCommand only support one condition, so we perform"},{"line_number":1043,"context_line":"        # index match on logical_port and then filter on dst_ip."},{"line_number":1044,"context_line":"        self.result \u003d ["},{"line_number":1045,"context_line":"            r"},{"line_number":1046,"context_line":"            for r in self.result if r[\u0027dst_ip\u0027] \u003d\u003d self.dst_ip"},{"line_number":1047,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":4,"id":"acea58c7_87ba499a","line":1044,"in_reply_to":"67c9eb3b_d503a95d","updated":"2023-04-19 13:21:42.000000000","message":"The handling of multiple conditions for the `db_find` method does not appear to work, if I try to do something like this:\n\n```diff\n--- a/ovsdbapp/schema/ovn_northbound/commands.py\n+++ b/ovsdbapp/schema/ovn_northbound/commands.py\n@@ -1030,12 +1030,13 @@ class BFDFindCommand(cmd.DbFindCommand):\n         super().__init__(\n             api,\n             self.table,\n-            (\u0027logical_port\u0027, \u0027\u003d\u0027, port),\n+            ((\u0027logical_port\u0027, \u0027\u003d\u0027, port),\n+             (\u0027dst_ip\u0027, \u0027\u003d\u0027, dst_ip))\n         )\n         self.dst_ip \u003d dst_ip\n```\n\nThe code will blow up like this:\n```\n    ERROR [ovsdbapp.backend.ovs_idl.command] Error executing command (BFDFindCommand)\nTraceback (most recent call last):\n  File \"/home/ubuntu/src/ovsdbapp/ovsdbapp/backend/ovs_idl/command.py\", line 47, in execute\n    self.run_idl(None)\n  File \"/home/ubuntu/src/ovsdbapp/ovsdbapp/schema/ovn_northbound/commands.py\", line 1039, in run_idl\n    return super().run_idl(txn)\n  File \"/home/ubuntu/src/ovsdbapp/ovsdbapp/backend/ovs_idl/command.py\", line 320, in run_idl\n    rows \u003d (idlutils.index_condition_match(self.table, *self.conditions) or\n  File \"/home/ubuntu/src/ovsdbapp/ovsdbapp/backend/ovs_idl/idlutils.py\", line 320, in index_condition_match\n    index_columns \u003d condition_index_columns(table, *conditions)\n  File \"/home/ubuntu/src/ovsdbapp/ovsdbapp/backend/ovs_idl/idlutils.py\", line 315, in condition_index_columns\n    return [(col, op, match) for col, op, match in conditions\n  File \"/home/ubuntu/src/ovsdbapp/ovsdbapp/backend/ovs_idl/idlutils.py\", line 315, in \u003clistcomp\u003e\n    return [(col, op, match) for col, op, match in conditions\nValueError: not enough values to unpack (expected 3, got 2)\n   ERROR [ovsdbapp.backend.ovs_idl.transaction] Traceback (most recent call last):\n  File \"/home/ubuntu/src/ovsdbapp/ovsdbapp/backend/ovs_idl/connection.py\", line 118, in run\n    txn.results.put(txn.do_commit())\n  File \"/home/ubuntu/src/ovsdbapp/ovsdbapp/backend/ovs_idl/transaction.py\", line 92, in do_commit\n    command.run_idl(txn)\n  File \"/home/ubuntu/src/ovsdbapp/ovsdbapp/schema/ovn_northbound/commands.py\", line 1095, in run_idl\n    bfd \u003d BFDFindCommand(\n  File \"/home/ubuntu/src/ovsdbapp/ovsdbapp/backend/ovs_idl/command.py\", line 47, in execute\n    self.run_idl(None)\n  File \"/home/ubuntu/src/ovsdbapp/ovsdbapp/schema/ovn_northbound/commands.py\", line 1039, in run_idl\n    return super().run_idl(txn)\n  File \"/home/ubuntu/src/ovsdbapp/ovsdbapp/backend/ovs_idl/command.py\", line 320, in run_idl\n    rows \u003d (idlutils.index_condition_match(self.table, *self.conditions) or\n  File \"/home/ubuntu/src/ovsdbapp/ovsdbapp/backend/ovs_idl/idlutils.py\", line 320, in index_condition_match\n    index_columns \u003d condition_index_columns(table, *conditions)\n  File \"/home/ubuntu/src/ovsdbapp/ovsdbapp/backend/ovs_idl/idlutils.py\", line 315, in condition_index_columns\n    return [(col, op, match) for col, op, match in conditions\n  File \"/home/ubuntu/src/ovsdbapp/ovsdbapp/backend/ovs_idl/idlutils.py\", line 315, in \u003clistcomp\u003e\n    return [(col, op, match) for col, op, match in conditions\nValueError: not enough values to unpack (expected 3, got 2)\n```","commit_id":"3bddf1ee7c188675065abf3cf5826533f815aa36"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"62ad19e5958cc5e699968086801432c5cd58bdb6","unresolved":true,"context_lines":[{"line_number":1041,"context_line":"        #"},{"line_number":1042,"context_line":"        # The base cmd.DbFindCommand only support one condition, so we perform"},{"line_number":1043,"context_line":"        # index match on logical_port and then filter on dst_ip."},{"line_number":1044,"context_line":"        self.result \u003d ["},{"line_number":1045,"context_line":"            r"},{"line_number":1046,"context_line":"            for r in self.result if r[\u0027dst_ip\u0027] \u003d\u003d self.dst_ip"},{"line_number":1047,"context_line":"        ]"}],"source_content_type":"text/x-python","patch_set":4,"id":"2f7c3031_aa4e7083","line":1044,"in_reply_to":"acea58c7_87ba499a","updated":"2023-04-19 13:31:47.000000000","message":"I think the problem with the code you tried is that the conditions should be passed individually (*args) like (\u0027logical_port\u0027, \u0027\u003d\u0027, port), (\u0027dst_ip\u0027, \u0027\u003d\u0027, dst_ip) and not contained within in single tuple.","commit_id":"3bddf1ee7c188675065abf3cf5826533f815aa36"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"45e534e8f2f7289a3cc067ee796732380c8d2495","unresolved":true,"context_lines":[{"line_number":1075,"context_line":"        self.options \u003d options or {}"},{"line_number":1076,"context_line":"        self.may_exist \u003d may_exist"},{"line_number":1077,"context_line":""},{"line_number":1078,"context_line":"    def _update_bfd(self, bfd):"},{"line_number":1079,"context_line":"        for column in ["},{"line_number":1080,"context_line":"                c"},{"line_number":1081,"context_line":"                for c in self.api.tables[self.table_name].columns.keys()"}],"source_content_type":"text/x-python","patch_set":4,"id":"61d71928_590a4974","line":1078,"updated":"2023-04-14 16:41:41.000000000","message":"This basically does what self.set_columns() already does. Could make a self.columns in __init__ that contains keys for the columns you\u0027re assigning to self and just use set_columns()","commit_id":"3bddf1ee7c188675065abf3cf5826533f815aa36"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"ef7af243e86e4085e5d57c9620a84427669effc5","unresolved":false,"context_lines":[{"line_number":1075,"context_line":"        self.options \u003d options or {}"},{"line_number":1076,"context_line":"        self.may_exist \u003d may_exist"},{"line_number":1077,"context_line":""},{"line_number":1078,"context_line":"    def _update_bfd(self, bfd):"},{"line_number":1079,"context_line":"        for column in ["},{"line_number":1080,"context_line":"                c"},{"line_number":1081,"context_line":"                for c in self.api.tables[self.table_name].columns.keys()"}],"source_content_type":"text/x-python","patch_set":4,"id":"2d38c048_7c5c7730","line":1078,"in_reply_to":"61d71928_590a4974","updated":"2023-04-19 13:21:42.000000000","message":"Thank you for the pointer, I\u0027ll make a change along those lines.","commit_id":"3bddf1ee7c188675065abf3cf5826533f815aa36"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"45e534e8f2f7289a3cc067ee796732380c8d2495","unresolved":true,"context_lines":[{"line_number":1100,"context_line":"                raise RuntimeError("},{"line_number":1101,"context_line":"                    \"Unexpected duplicates in database for port %s \""},{"line_number":1102,"context_line":"                    \"and dst_ip %s\" % (self.logical_port, self.dst_ip))"},{"line_number":1103,"context_line":"            bfd \u003d self.api.lookup(\u0027BFD\u0027, bfd[0][\u0027_uuid\u0027])"},{"line_number":1104,"context_line":"            if self.may_exist:"},{"line_number":1105,"context_line":"                self._update_bfd(bfd)"},{"line_number":1106,"context_line":"                # When no changes are made to a record, the parent"}],"source_content_type":"text/x-python","patch_set":4,"id":"d8f9e02d_502da313","line":1103,"updated":"2023-04-14 16:41:41.000000000","message":"If DbFindCommand is passed row\u003dTrue (see db_find_rows()), it will pass back RowView objects so you won\u0027t have to do a separate lookup() here or convert when assigning self.result.","commit_id":"3bddf1ee7c188675065abf3cf5826533f815aa36"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"ef7af243e86e4085e5d57c9620a84427669effc5","unresolved":false,"context_lines":[{"line_number":1100,"context_line":"                raise RuntimeError("},{"line_number":1101,"context_line":"                    \"Unexpected duplicates in database for port %s \""},{"line_number":1102,"context_line":"                    \"and dst_ip %s\" % (self.logical_port, self.dst_ip))"},{"line_number":1103,"context_line":"            bfd \u003d self.api.lookup(\u0027BFD\u0027, bfd[0][\u0027_uuid\u0027])"},{"line_number":1104,"context_line":"            if self.may_exist:"},{"line_number":1105,"context_line":"                self._update_bfd(bfd)"},{"line_number":1106,"context_line":"                # When no changes are made to a record, the parent"}],"source_content_type":"text/x-python","patch_set":4,"id":"b0e37a0d_ae34dd61","line":1103,"in_reply_to":"d8f9e02d_502da313","updated":"2023-04-19 13:21:42.000000000","message":"Ah, I did not know that, thank you for the pointer! Will update.","commit_id":"3bddf1ee7c188675065abf3cf5826533f815aa36"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"7b28573b5dee062e17f737f7b1404609286da05c","unresolved":true,"context_lines":[{"line_number":1092,"context_line":""},{"line_number":1093,"context_line":"    def run_idl(self, txn):"},{"line_number":1094,"context_line":"        bfd_result \u003d BFDFindCommand("},{"line_number":1095,"context_line":"            self.api, self.logical_port, self.dst_ip).execute(check_error\u003dTrue)"},{"line_number":1096,"context_line":"        if bfd_result:"},{"line_number":1097,"context_line":"            if len(bfd_result) \u003e 1:"},{"line_number":1098,"context_line":"                # With the current database schema, this cannot happen, but"}],"source_content_type":"text/x-python","patch_set":6,"id":"23d4f7ba_993d11ee","line":1095,"range":{"start_line":1095,"start_character":54,"end_line":1095,"end_character":61},"updated":"2023-04-19 16:27:10.000000000","message":"Sorry, I just noticed this. This would do a nested txn, which isn\u0027t a huge deal due to it being a read-only txn that doesn\u0027t end up going to the server, but generally when running a Command from inside another Command, I\u0027d recommend doing something like:\n\n```\ncmd \u003d BFDFindCommand(...)\ncmd.run_idl(txn)\nbfd_result \u003d cmd.result\n```\n\nAn example would be LrpAddCommand.","commit_id":"9eaa0243d8a24058b304f2c80aa3d86e3fa5702a"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"acc3481556c4019e1c0b2866139dd21f082730c1","unresolved":false,"context_lines":[{"line_number":1092,"context_line":""},{"line_number":1093,"context_line":"    def run_idl(self, txn):"},{"line_number":1094,"context_line":"        bfd_result \u003d BFDFindCommand("},{"line_number":1095,"context_line":"            self.api, self.logical_port, self.dst_ip).execute(check_error\u003dTrue)"},{"line_number":1096,"context_line":"        if bfd_result:"},{"line_number":1097,"context_line":"            if len(bfd_result) \u003e 1:"},{"line_number":1098,"context_line":"                # With the current database schema, this cannot happen, but"}],"source_content_type":"text/x-python","patch_set":6,"id":"83599fe8_8a488778","line":1095,"range":{"start_line":1095,"start_character":54,"end_line":1095,"end_character":61},"in_reply_to":"23d4f7ba_993d11ee","updated":"2023-04-19 17:13:07.000000000","message":"Indeed, and thank you for pointing that out. I learned this trick when updating some of the neutron ovsdb implementation code and forgot to go back here to update.\n\nNew patch set to follow momentarily.","commit_id":"9eaa0243d8a24058b304f2c80aa3d86e3fa5702a"}]}