)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":32586,"name":"Elvira García Ruiz","display_name":"Elvira","email":"egarciar@redhat.com","username":"elvira"},"change_message_id":"98286105cb7461be6f55b1b2c03eb1b0f6562450","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"8db57398_7f2c9c30","updated":"2022-05-25 14:12:04.000000000","message":"Thanks Rodolfo, LGTM! Just commented a nit","commit_id":"4796b37c7fe98a41e6448f96543ebc02e0e733b7"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"7d454e7aa9513d6633a94314960496da0fdbc59f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"22f1e83e_118f7f04","updated":"2022-05-26 08:33:05.000000000","message":"Would be good to have Terry\u0027s opinion on this too","commit_id":"4796b37c7fe98a41e6448f96543ebc02e0e733b7"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"799a9db8e40d55d8da8d7e9fc38b1160b772864a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"3dd1cacc_a8abd6b3","updated":"2022-06-01 12:29:39.000000000","message":"A query inline","commit_id":"32e8303b3b21e047abcf365c3999cb7379467b0c"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"f3bc952fba6bcad7de4cc9863b70ccb9c8a5df56","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"cec0f17b_c2e8c9f9","updated":"2022-06-01 12:19:12.000000000","message":"Thanks for checking the suggestion. I think this looks fine as is too","commit_id":"32e8303b3b21e047abcf365c3999cb7379467b0c"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"7d454e7aa9513d6633a94314960496da0fdbc59f","unresolved":true,"context_lines":[{"line_number":371,"context_line":"            # SB update and therefore an infinite loop)."},{"line_number":372,"context_line":"            # [1] https://www.mail-archive.com/ovs-dev@openvswitch.org/"},{"line_number":373,"context_line":"            #     msg62836.html"},{"line_number":374,"context_line":"            return"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        if not utils.is_ovn_l3(self.l3_plugin):"},{"line_number":377,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":5,"id":"3e119e34_66dfca3c","line":374,"updated":"2022-05-26 08:33:05.000000000","message":"Suggestion:\n\nAnother potential approach for this, instead of inspecting the private attributes of the object, is to configure the IDL just to \"watch\" for the specific columns from the Port_Binding table.\n\nWhen we configure the IDL to watch for the Port_Binding table in SB DB we could specify which columns we do care about and the rest will be ignored. See my comment below in the file as an example.","commit_id":"4796b37c7fe98a41e6448f96543ebc02e0e733b7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"d59b037551c9428577014198faa91b4e90d5f27a","unresolved":false,"context_lines":[{"line_number":371,"context_line":"            # SB update and therefore an infinite loop)."},{"line_number":372,"context_line":"            # [1] https://www.mail-archive.com/ovs-dev@openvswitch.org/"},{"line_number":373,"context_line":"            #     msg62836.html"},{"line_number":374,"context_line":"            return"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        if not utils.is_ovn_l3(self.l3_plugin):"},{"line_number":377,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":5,"id":"0eee00eb_28e5ff4a","line":374,"in_reply_to":"1f5f28c0_89442941","updated":"2022-06-01 11:38:54.000000000","message":"Done","commit_id":"4796b37c7fe98a41e6448f96543ebc02e0e733b7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"b23f3d0bf65075ac9ebd3da224eeb0a17425850c","unresolved":true,"context_lines":[{"line_number":371,"context_line":"            # SB update and therefore an infinite loop)."},{"line_number":372,"context_line":"            # [1] https://www.mail-archive.com/ovs-dev@openvswitch.org/"},{"line_number":373,"context_line":"            #     msg62836.html"},{"line_number":374,"context_line":"            return"},{"line_number":375,"context_line":""},{"line_number":376,"context_line":"        if not utils.is_ovn_l3(self.l3_plugin):"},{"line_number":377,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":5,"id":"1f5f28c0_89442941","line":374,"in_reply_to":"3e119e34_66dfca3c","updated":"2022-05-31 13:53:42.000000000","message":"Let me check that.","commit_id":"4796b37c7fe98a41e6448f96543ebc02e0e733b7"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"7d454e7aa9513d6633a94314960496da0fdbc59f","unresolved":true,"context_lines":[{"line_number":881,"context_line":"        helper.register_table(\u0027Port_Binding\u0027)"},{"line_number":882,"context_line":"        helper.register_table(\u0027Datapath_Binding\u0027)"},{"line_number":883,"context_line":"        helper.register_table(\u0027Connection\u0027)"},{"line_number":884,"context_line":"        helper.register_columns(\u0027SB_Global\u0027, [\u0027external_ids\u0027])"},{"line_number":885,"context_line":"        try:"},{"line_number":886,"context_line":"            return cls(driver, connection_string, helper, leader_only\u003dFalse)"},{"line_number":887,"context_line":"        except TypeError:"}],"source_content_type":"text/x-python","patch_set":5,"id":"dd7507cd_3a4a0a14","line":884,"updated":"2022-05-26 08:33:05.000000000","message":"Here.\n\nWe could have something like:\n\nhelper.register_columns(\u0027Port_Binding\u0027, [\u0027type\u0027, \u0027datapath\u0027, ...])\n\nThat would require us to look at the code and see what columns do we actually use for our events.\n\nThat all said, I am good with your current implementation. Just wanted to throw this out as an alternative to see what do you think about it as it does not require using private attributes.","commit_id":"4796b37c7fe98a41e6448f96543ebc02e0e733b7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4b9d81b02788305fd56b235df7d78930a9032f32","unresolved":true,"context_lines":[{"line_number":881,"context_line":"        helper.register_table(\u0027Port_Binding\u0027)"},{"line_number":882,"context_line":"        helper.register_table(\u0027Datapath_Binding\u0027)"},{"line_number":883,"context_line":"        helper.register_table(\u0027Connection\u0027)"},{"line_number":884,"context_line":"        helper.register_columns(\u0027SB_Global\u0027, [\u0027external_ids\u0027])"},{"line_number":885,"context_line":"        try:"},{"line_number":886,"context_line":"            return cls(driver, connection_string, helper, leader_only\u003dFalse)"},{"line_number":887,"context_line":"        except TypeError:"}],"source_content_type":"text/x-python","patch_set":5,"id":"e46b68e0_bdc43670","line":884,"in_reply_to":"dd7507cd_3a4a0a14","updated":"2022-06-01 11:35:46.000000000","message":"I\u0027ve tested that but we need too many columns (options, chassis, logical_port, uuid, datapath) that is maybe better to make this check in PortBindingChassisEvent.run method.","commit_id":"4796b37c7fe98a41e6448f96543ebc02e0e733b7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"d59b037551c9428577014198faa91b4e90d5f27a","unresolved":false,"context_lines":[{"line_number":881,"context_line":"        helper.register_table(\u0027Port_Binding\u0027)"},{"line_number":882,"context_line":"        helper.register_table(\u0027Datapath_Binding\u0027)"},{"line_number":883,"context_line":"        helper.register_table(\u0027Connection\u0027)"},{"line_number":884,"context_line":"        helper.register_columns(\u0027SB_Global\u0027, [\u0027external_ids\u0027])"},{"line_number":885,"context_line":"        try:"},{"line_number":886,"context_line":"            return cls(driver, connection_string, helper, leader_only\u003dFalse)"},{"line_number":887,"context_line":"        except TypeError:"}],"source_content_type":"text/x-python","patch_set":5,"id":"f2f61acc_012c8ab3","line":884,"in_reply_to":"e46b68e0_bdc43670","updated":"2022-06-01 11:38:54.000000000","message":"Done","commit_id":"4796b37c7fe98a41e6448f96543ebc02e0e733b7"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"c7b422000c374ab487565eeaedc15a948e14c253","unresolved":true,"context_lines":[{"line_number":404,"context_line":"        self.event_name \u003d \u0027PortBindingChassisEvent\u0027"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    def run(self, event, row, old):"},{"line_number":407,"context_line":"        if len(old._data) \u003d\u003d 1 and \u0027external_ids\u0027 in old._data:"},{"line_number":408,"context_line":"            # NOTE: since [1], the NB logical_router_port.external_ids are"},{"line_number":409,"context_line":"            # copied into the SB port_binding.external_ids. If only the"},{"line_number":410,"context_line":"            # external_ids are changed, this event should be dismissed or it"}],"source_content_type":"text/x-python","patch_set":7,"id":"efcbadb0_d9a09844","line":407,"updated":"2022-06-09 17:14:18.000000000","message":"Good catch. I just ran into this yesterday on a test box as well.\n\nIn general, we should move checks like this and the if not utils.is_ovn_l3() check to a match_fn() so that we don\u0027t match the event at all in cases we don\u0027t want it to run().\n\nLucas\u0027 suggestion of doing a register_columns() where we completely ignore SB Port_Binding.external_ids seems like a good thing to do as well.","commit_id":"32e8303b3b21e047abcf365c3999cb7379467b0c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"356c601d5b39308ed188392b45b6d18b4ae6fe42","unresolved":false,"context_lines":[{"line_number":404,"context_line":"        self.event_name \u003d \u0027PortBindingChassisEvent\u0027"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    def run(self, event, row, old):"},{"line_number":407,"context_line":"        if len(old._data) \u003d\u003d 1 and \u0027external_ids\u0027 in old._data:"},{"line_number":408,"context_line":"            # NOTE: since [1], the NB logical_router_port.external_ids are"},{"line_number":409,"context_line":"            # copied into the SB port_binding.external_ids. If only the"},{"line_number":410,"context_line":"            # external_ids are changed, this event should be dismissed or it"}],"source_content_type":"text/x-python","patch_set":7,"id":"96949200_38fd5fc8","line":407,"in_reply_to":"efcbadb0_d9a09844","updated":"2022-06-13 06:57:08.000000000","message":"Done in https://review.opendev.org/c/openstack/neutron/+/845547\n\nI\u0027ve already tested the column registering with no luck in previous PS.","commit_id":"32e8303b3b21e047abcf365c3999cb7379467b0c"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"799a9db8e40d55d8da8d7e9fc38b1160b772864a","unresolved":true,"context_lines":[{"line_number":403,"context_line":"            events, table, ((\u0027type\u0027, \u0027\u003d\u0027, ovn_const.OVN_CHASSIS_REDIRECT),))"},{"line_number":404,"context_line":"        self.event_name \u003d \u0027PortBindingChassisEvent\u0027"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    def run(self, event, row, old):"},{"line_number":407,"context_line":"        if len(old._data) \u003d\u003d 1 and \u0027external_ids\u0027 in old._data:"},{"line_number":408,"context_line":"            # NOTE: since [1], the NB logical_router_port.external_ids are"},{"line_number":409,"context_line":"            # copied into the SB port_binding.external_ids. If only the"},{"line_number":410,"context_line":"            # external_ids are changed, this event should be dismissed or it"},{"line_number":411,"context_line":"            # will trigger the Neutron NB update (that will trigger the core"},{"line_number":412,"context_line":"            # SB update and therefore an infinite loop)."},{"line_number":413,"context_line":"            # [1] https://www.mail-archive.com/ovs-dev@openvswitch.org/"},{"line_number":414,"context_line":"            #     msg62836.html"},{"line_number":415,"context_line":"            return"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"        if not utils.is_ovn_l3(self.l3_plugin):"},{"line_number":418,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":7,"id":"3bd475cc_1acce747","line":415,"range":{"start_line":406,"start_character":4,"end_line":415,"end_character":18},"updated":"2022-06-01 12:29:39.000000000","message":"Just to be sure it will work when chassis keeps on changing as seen in https://bugzilla.redhat.com/show_bug.cgi?id\u003d1974898 , reading comments seems it will not work can you please cross check.","commit_id":"32e8303b3b21e047abcf365c3999cb7379467b0c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e06f9153f6be32e4e1e4e213208fb0b786f8503a","unresolved":false,"context_lines":[{"line_number":403,"context_line":"            events, table, ((\u0027type\u0027, \u0027\u003d\u0027, ovn_const.OVN_CHASSIS_REDIRECT),))"},{"line_number":404,"context_line":"        self.event_name \u003d \u0027PortBindingChassisEvent\u0027"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    def run(self, event, row, old):"},{"line_number":407,"context_line":"        if len(old._data) \u003d\u003d 1 and \u0027external_ids\u0027 in old._data:"},{"line_number":408,"context_line":"            # NOTE: since [1], the NB logical_router_port.external_ids are"},{"line_number":409,"context_line":"            # copied into the SB port_binding.external_ids. If only the"},{"line_number":410,"context_line":"            # external_ids are changed, this event should be dismissed or it"},{"line_number":411,"context_line":"            # will trigger the Neutron NB update (that will trigger the core"},{"line_number":412,"context_line":"            # SB update and therefore an infinite loop)."},{"line_number":413,"context_line":"            # [1] https://www.mail-archive.com/ovs-dev@openvswitch.org/"},{"line_number":414,"context_line":"            #     msg62836.html"},{"line_number":415,"context_line":"            return"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"        if not utils.is_ovn_l3(self.l3_plugin):"},{"line_number":418,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":7,"id":"49d9536d_6fc685c3","line":415,"range":{"start_line":406,"start_character":4,"end_line":415,"end_character":18},"in_reply_to":"1cee62a9_b56b06f6","updated":"2022-06-07 09:33:17.000000000","message":"Done","commit_id":"32e8303b3b21e047abcf365c3999cb7379467b0c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ea45ac0283847b15a2f334aeea800cb02a70069d","unresolved":true,"context_lines":[{"line_number":403,"context_line":"            events, table, ((\u0027type\u0027, \u0027\u003d\u0027, ovn_const.OVN_CHASSIS_REDIRECT),))"},{"line_number":404,"context_line":"        self.event_name \u003d \u0027PortBindingChassisEvent\u0027"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    def run(self, event, row, old):"},{"line_number":407,"context_line":"        if len(old._data) \u003d\u003d 1 and \u0027external_ids\u0027 in old._data:"},{"line_number":408,"context_line":"            # NOTE: since [1], the NB logical_router_port.external_ids are"},{"line_number":409,"context_line":"            # copied into the SB port_binding.external_ids. If only the"},{"line_number":410,"context_line":"            # external_ids are changed, this event should be dismissed or it"},{"line_number":411,"context_line":"            # will trigger the Neutron NB update (that will trigger the core"},{"line_number":412,"context_line":"            # SB update and therefore an infinite loop)."},{"line_number":413,"context_line":"            # [1] https://www.mail-archive.com/ovs-dev@openvswitch.org/"},{"line_number":414,"context_line":"            #     msg62836.html"},{"line_number":415,"context_line":"            return"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"        if not utils.is_ovn_l3(self.l3_plugin):"},{"line_number":418,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":7,"id":"3e22bd17_0fb59097","line":415,"range":{"start_line":406,"start_character":4,"end_line":415,"end_character":18},"in_reply_to":"3bd475cc_1acce747","updated":"2022-06-03 14:21:52.000000000","message":"But that\u0027s the point: to skip this event when the \"external_ids\" is modified. If the LSP bumps its revision number, ovn-controller will copy the new value in PB too. This check will skip that event.","commit_id":"32e8303b3b21e047abcf365c3999cb7379467b0c"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"174a35062479b8328e579e68b21b10f478edcc84","unresolved":true,"context_lines":[{"line_number":403,"context_line":"            events, table, ((\u0027type\u0027, \u0027\u003d\u0027, ovn_const.OVN_CHASSIS_REDIRECT),))"},{"line_number":404,"context_line":"        self.event_name \u003d \u0027PortBindingChassisEvent\u0027"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    def run(self, event, row, old):"},{"line_number":407,"context_line":"        if len(old._data) \u003d\u003d 1 and \u0027external_ids\u0027 in old._data:"},{"line_number":408,"context_line":"            # NOTE: since [1], the NB logical_router_port.external_ids are"},{"line_number":409,"context_line":"            # copied into the SB port_binding.external_ids. If only the"},{"line_number":410,"context_line":"            # external_ids are changed, this event should be dismissed or it"},{"line_number":411,"context_line":"            # will trigger the Neutron NB update (that will trigger the core"},{"line_number":412,"context_line":"            # SB update and therefore an infinite loop)."},{"line_number":413,"context_line":"            # [1] https://www.mail-archive.com/ovs-dev@openvswitch.org/"},{"line_number":414,"context_line":"            #     msg62836.html"},{"line_number":415,"context_line":"            return"},{"line_number":416,"context_line":""},{"line_number":417,"context_line":"        if not utils.is_ovn_l3(self.l3_plugin):"},{"line_number":418,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":7,"id":"1cee62a9_b56b06f6","line":415,"range":{"start_line":406,"start_character":4,"end_line":415,"end_character":18},"in_reply_to":"3e22bd17_0fb59097","updated":"2022-06-03 14:44:21.000000000","message":"Ok Thanks got it now after reply over IRC. as there will be seperate events for chassis change and external_ids change, here only external_ids change events are skipped.","commit_id":"32e8303b3b21e047abcf365c3999cb7379467b0c"}],"neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py":[{"author":{"_account_id":32586,"name":"Elvira García Ruiz","display_name":"Elvira","email":"egarciar@redhat.com","username":"elvira"},"change_message_id":"98286105cb7461be6f55b1b2c03eb1b0f6562450","unresolved":true,"context_lines":[{"line_number":26,"context_line":"from ovsdbapp.backend.ovs_idl import event"},{"line_number":27,"context_line":"from ovsdbapp.backend.ovs_idl import idlutils"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"from neutron.common.ovn import constants as ovn_const"},{"line_number":31,"context_line":"from neutron.common import utils as n_utils"},{"line_number":32,"context_line":"from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf"}],"source_content_type":"text/x-python","patch_set":5,"id":"c5d9e57c_4b0f5cfb","side":"PARENT","line":29,"updated":"2022-05-25 14:12:04.000000000","message":"nit: is this line removal expected in this patch?","commit_id":"e35188ef3dcc9a923d2450819abd97268100734e"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"d59b037551c9428577014198faa91b4e90d5f27a","unresolved":false,"context_lines":[{"line_number":26,"context_line":"from ovsdbapp.backend.ovs_idl import event"},{"line_number":27,"context_line":"from ovsdbapp.backend.ovs_idl import idlutils"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"from neutron.common.ovn import constants as ovn_const"},{"line_number":31,"context_line":"from neutron.common import utils as n_utils"},{"line_number":32,"context_line":"from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf"}],"source_content_type":"text/x-python","patch_set":5,"id":"16fc7d23_faa7162d","side":"PARENT","line":29,"in_reply_to":"b0a0d893_32165346","updated":"2022-06-01 11:38:54.000000000","message":"Done","commit_id":"e35188ef3dcc9a923d2450819abd97268100734e"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"b99fb38e63ebab3bfe56e987a048179d3d50312d","unresolved":true,"context_lines":[{"line_number":26,"context_line":"from ovsdbapp.backend.ovs_idl import event"},{"line_number":27,"context_line":"from ovsdbapp.backend.ovs_idl import idlutils"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"from neutron.common.ovn import constants as ovn_const"},{"line_number":31,"context_line":"from neutron.common import utils as n_utils"},{"line_number":32,"context_line":"from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf"}],"source_content_type":"text/x-python","patch_set":5,"id":"b0a0d893_32165346","side":"PARENT","line":29,"in_reply_to":"c5d9e57c_4b0f5cfb","updated":"2022-05-26 07:49:56.000000000","message":"Well, not exactly. But when adding the new imports, I saw it and I couldn\u0027t resist...","commit_id":"e35188ef3dcc9a923d2450819abd97268100734e"}]}