)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"3477bee40ca6821768b57f6e15705e97e39a8d83","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"ef3bd823_f2bf94cd","updated":"2024-10-09 12:16:38.000000000","message":"Keeping +1 to hear back about the exception part","commit_id":"842d660b3fd47bc5333c322c59c3e32499e2686b"}],"ovn_bgp_agent/privileged/ovs_vsctl.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"dbabcf715dc061c175398c78d5f8c4a85cb78bca","unresolved":true,"context_lines":[{"line_number":29,"context_line":"    try:"},{"line_number":30,"context_line":"        return processutils.execute(*full_args)"},{"line_number":31,"context_line":"    except processutils.ProcessExecutionError:"},{"line_number":32,"context_line":"        if \u0027ovs-ofctl\u0027 !\u003d command:"},{"line_number":33,"context_line":"            LOG.exception(\"Unable to execute %s %s\", command, full_args)"},{"line_number":34,"context_line":"            raise"},{"line_number":35,"context_line":"        full_args +\u003d [\u0027-O\u0027, \u0027OpenFlow13\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"6192df78_6cfdce19","line":32,"range":{"start_line":32,"start_character":11,"end_line":32,"end_character":34},"updated":"2024-10-08 08:11:45.000000000","message":"super nit: usually it is written the other way \"command !\u003d \u0027ovs-ofctl\u0027\"","commit_id":"36fd9ebd0eafc00e58dca6fdfa125747ccc3029b"},{"author":{"_account_id":31291,"name":"Eduardo Olivares","email":"eolivare@redhat.com","username":"eolivare"},"change_message_id":"4841f9c5136655700710d0d985fbca99c4cb143c","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    try:"},{"line_number":30,"context_line":"        return processutils.execute(*full_args)"},{"line_number":31,"context_line":"    except processutils.ProcessExecutionError:"},{"line_number":32,"context_line":"        if \u0027ovs-ofctl\u0027 !\u003d command:"},{"line_number":33,"context_line":"            LOG.exception(\"Unable to execute %s %s\", command, full_args)"},{"line_number":34,"context_line":"            raise"},{"line_number":35,"context_line":"        full_args +\u003d [\u0027-O\u0027, \u0027OpenFlow13\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"b94e4a53_f4b69fa5","line":32,"range":{"start_line":32,"start_character":11,"end_line":32,"end_character":34},"in_reply_to":"6192df78_6cfdce19","updated":"2024-10-08 08:25:07.000000000","message":"I changed it because it\u0027s more coherent with the rest of the code, but you have to know my method is better:\nhttps://en.wikipedia.org/wiki/Yoda_conditions\n😊\nWell, that\u0027s only valid for \"equal\" conditions, not for \"different\". Never mind.","commit_id":"36fd9ebd0eafc00e58dca6fdfa125747ccc3029b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"e4c17a96ac4fa5be1e2b507442da84f62bfa61aa","unresolved":true,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"@ovn_bgp_agent.privileged.ovs_vsctl_cmd.entrypoint"},{"line_number":24,"context_line":"def ovs_cmd(command, args, timeout\u003dNone):"},{"line_number":25,"context_line":"    full_args \u003d [command]"},{"line_number":26,"context_line":"    if timeout is not None:"},{"line_number":27,"context_line":"        full_args +\u003d [\u0027--timeout\u003d%s\u0027 % timeout]"}],"source_content_type":"text/x-python","patch_set":5,"id":"a5f2900a_e6c1d375","line":24,"range":{"start_line":24,"start_character":4,"end_line":24,"end_character":11},"updated":"2024-10-08 12:30:56.000000000","message":"I\u0027d split this into three functions to simplify the function body. Before we call into the `ovs_cmd`, we already know what type of `ctl` we want.\n\nAlso I don\u0027t like this module is called ovs_vsctl but it really is a generic purpose but meh.\n\nWhat about this:\n```\n@ovn_bgp_agent.privileged.ovs_vsctl_cmd.entrypoint\ndef ovs_cmd(command, args, timeout):\n    ...\n\n    \ndef ovs_vsctl(command, args, timeout\u003dNone):\n    return ovs_cmd(\u0027ovs-vsctl\u0027, args, timeout)\n    \n\ndef ovs_ofctl(command, args, timeout\u003dNone):\n    try:\n        return ovs_cmd(\u0027ovs-ofctl\u0027, args, timeout)\n    except processutils.ProcessExecutionError:\n        ... do the magic here\n```\n\nAnother solution that would require a bit more work would be to have an instance of OVS IDL in memory and return the idl with some `get_instance()` class method and then use the IDL to query the DB instead of vsctl and have the `ovs_cmd` replaced fully by `ovs-ofctl`.\n\nI like better the second solution but the first one seems like a good compromise, easy with little effort.","commit_id":"a89f1cb53c5e691ed87cdc81e79252a8f7344b05"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d0ea6fefd67002d03b7c5ba8e393c82854267f36","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"@ovn_bgp_agent.privileged.ovs_vsctl_cmd.entrypoint"},{"line_number":24,"context_line":"def ovs_cmd(command, args, timeout\u003dNone):"},{"line_number":25,"context_line":"    full_args \u003d [command]"},{"line_number":26,"context_line":"    if timeout is not None:"},{"line_number":27,"context_line":"        full_args +\u003d [\u0027--timeout\u003d%s\u0027 % timeout]"}],"source_content_type":"text/x-python","patch_set":5,"id":"1c0c6a2d_cb95ad92","line":24,"range":{"start_line":24,"start_character":4,"end_line":24,"end_character":11},"in_reply_to":"a5f2900a_e6c1d375","updated":"2024-10-09 11:53:35.000000000","message":"Done","commit_id":"a89f1cb53c5e691ed87cdc81e79252a8f7344b05"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"fc7d316db6dcdb7e7b82f6ebb4d30ad4d2ae782e","unresolved":true,"context_lines":[{"line_number":29,"context_line":"    try:"},{"line_number":30,"context_line":"        return processutils.execute(*full_args)"},{"line_number":31,"context_line":"    except Exception:"},{"line_number":32,"context_line":"        LOG.exception(\"Unable to execute %s %s\", command, full_args)"},{"line_number":33,"context_line":"        raise"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"de2e2de3_c5cf0649","line":32,"range":{"start_line":32,"start_character":12,"end_line":32,"end_character":21},"updated":"2024-10-09 11:24:43.000000000","message":"I\u0027d go with `error` here, unless there is a reason why we would need to see where this call was made from by inspecting the traceback. Seems like this function is well isolated and we have all the input arguments logged.","commit_id":"6e47cb5ea07d20d0401568e3b9888fcc1d9a9eea"},{"author":{"_account_id":31291,"name":"Eduardo Olivares","email":"eolivare@redhat.com","username":"eolivare"},"change_message_id":"7eb993e48ca5806f1eb0f6b9707b6dae5ec39519","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    try:"},{"line_number":30,"context_line":"        return processutils.execute(*full_args)"},{"line_number":31,"context_line":"    except Exception:"},{"line_number":32,"context_line":"        LOG.exception(\"Unable to execute %s %s\", command, full_args)"},{"line_number":33,"context_line":"        raise"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"e268f99f_03f2c89a","line":32,"range":{"start_line":32,"start_character":12,"end_line":32,"end_character":21},"in_reply_to":"de2e2de3_c5cf0649","updated":"2024-10-09 12:47:30.000000000","message":"Done","commit_id":"6e47cb5ea07d20d0401568e3b9888fcc1d9a9eea"}],"ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovs.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"fc7d316db6dcdb7e7b82f6ebb4d30ad4d2ae782e","unresolved":true,"context_lines":[{"line_number":186,"context_line":"        port_iface \u003d \u00271\u0027"},{"line_number":187,"context_line":"        ovs_port_iface \u003d \u00272\u0027"},{"line_number":188,"context_line":"        net \u003d \u0027fake-net\u0027"},{"line_number":189,"context_line":"        self.mock_ovs_vsctl.ovs_vsctl.side_effect \u003d ["},{"line_number":190,"context_line":"            \u0027%s\\n%s\\n\u0027 % (port, ovs_port)]"},{"line_number":191,"context_line":"        self.mock_ovs_vsctl.ovs_ofctl.side_effect \u003d None"},{"line_number":192,"context_line":"        mock_ofport.side_effect \u003d (ovs_port_iface, port_iface)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1266b9a7_88f00ca8","line":189,"range":{"start_line":189,"start_character":38,"end_line":189,"end_character":49},"updated":"2024-10-09 11:24:43.000000000","message":"`return_value`","commit_id":"6e47cb5ea07d20d0401568e3b9888fcc1d9a9eea"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d0ea6fefd67002d03b7c5ba8e393c82854267f36","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        port_iface \u003d \u00271\u0027"},{"line_number":187,"context_line":"        ovs_port_iface \u003d \u00272\u0027"},{"line_number":188,"context_line":"        net \u003d \u0027fake-net\u0027"},{"line_number":189,"context_line":"        self.mock_ovs_vsctl.ovs_vsctl.side_effect \u003d ["},{"line_number":190,"context_line":"            \u0027%s\\n%s\\n\u0027 % (port, ovs_port)]"},{"line_number":191,"context_line":"        self.mock_ovs_vsctl.ovs_ofctl.side_effect \u003d None"},{"line_number":192,"context_line":"        mock_ofport.side_effect \u003d (ovs_port_iface, port_iface)"}],"source_content_type":"text/x-python","patch_set":6,"id":"0b27707e_2f70833a","line":189,"range":{"start_line":189,"start_character":38,"end_line":189,"end_character":49},"in_reply_to":"1266b9a7_88f00ca8","updated":"2024-10-09 11:53:35.000000000","message":"Done","commit_id":"6e47cb5ea07d20d0401568e3b9888fcc1d9a9eea"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"fc7d316db6dcdb7e7b82f6ebb4d30ad4d2ae782e","unresolved":true,"context_lines":[{"line_number":188,"context_line":"        net \u003d \u0027fake-net\u0027"},{"line_number":189,"context_line":"        self.mock_ovs_vsctl.ovs_vsctl.side_effect \u003d ["},{"line_number":190,"context_line":"            \u0027%s\\n%s\\n\u0027 % (port, ovs_port)]"},{"line_number":191,"context_line":"        self.mock_ovs_vsctl.ovs_ofctl.side_effect \u003d None"},{"line_number":192,"context_line":"        mock_ofport.side_effect \u003d (ovs_port_iface, port_iface)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"        # Invoke the method"}],"source_content_type":"text/x-python","patch_set":6,"id":"99d5d85b_8245ba61","line":191,"range":{"start_line":191,"start_character":38,"end_line":191,"end_character":49},"updated":"2024-10-09 11:24:43.000000000","message":"`return_value`","commit_id":"6e47cb5ea07d20d0401568e3b9888fcc1d9a9eea"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d0ea6fefd67002d03b7c5ba8e393c82854267f36","unresolved":false,"context_lines":[{"line_number":188,"context_line":"        net \u003d \u0027fake-net\u0027"},{"line_number":189,"context_line":"        self.mock_ovs_vsctl.ovs_vsctl.side_effect \u003d ["},{"line_number":190,"context_line":"            \u0027%s\\n%s\\n\u0027 % (port, ovs_port)]"},{"line_number":191,"context_line":"        self.mock_ovs_vsctl.ovs_ofctl.side_effect \u003d None"},{"line_number":192,"context_line":"        mock_ofport.side_effect \u003d (ovs_port_iface, port_iface)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"        # Invoke the method"}],"source_content_type":"text/x-python","patch_set":6,"id":"362930cb_5c2d8f65","line":191,"range":{"start_line":191,"start_character":38,"end_line":191,"end_character":49},"in_reply_to":"99d5d85b_8245ba61","updated":"2024-10-09 11:53:35.000000000","message":"Done","commit_id":"6e47cb5ea07d20d0401568e3b9888fcc1d9a9eea"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"fc7d316db6dcdb7e7b82f6ebb4d30ad4d2ae782e","unresolved":true,"context_lines":[{"line_number":254,"context_line":"    def test_remove_evpn_router_ovs_flows(self, mock_ofport):"},{"line_number":255,"context_line":"        ovs_port \u003d constants.OVS_PATCH_PROVNET_PORT_PREFIX + \u0027fake-port\u0027"},{"line_number":256,"context_line":"        ovs_port_iface \u003d \u00271\u0027"},{"line_number":257,"context_line":"        self.mock_ovs_vsctl.ovs_vsctl.side_effect \u003d [ovs_port]"},{"line_number":258,"context_line":"        self.mock_ovs_vsctl.ovs_ofctl.side_effect \u003d (None, None)"},{"line_number":259,"context_line":"        mock_ofport.return_value \u003d ovs_port_iface"},{"line_number":260,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3f2f807a_e1059f4d","line":257,"range":{"start_line":257,"start_character":38,"end_line":257,"end_character":49},"updated":"2024-10-09 11:24:43.000000000","message":"I think it should be now `return_value`. The `side_effect` having an iterable means the first call would return `[ovs_port]`, second `None` and third `None`. Now that we split the cmd into two, we will call vsctl only once and hence the `return_value`.","commit_id":"6e47cb5ea07d20d0401568e3b9888fcc1d9a9eea"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d0ea6fefd67002d03b7c5ba8e393c82854267f36","unresolved":false,"context_lines":[{"line_number":254,"context_line":"    def test_remove_evpn_router_ovs_flows(self, mock_ofport):"},{"line_number":255,"context_line":"        ovs_port \u003d constants.OVS_PATCH_PROVNET_PORT_PREFIX + \u0027fake-port\u0027"},{"line_number":256,"context_line":"        ovs_port_iface \u003d \u00271\u0027"},{"line_number":257,"context_line":"        self.mock_ovs_vsctl.ovs_vsctl.side_effect \u003d [ovs_port]"},{"line_number":258,"context_line":"        self.mock_ovs_vsctl.ovs_ofctl.side_effect \u003d (None, None)"},{"line_number":259,"context_line":"        mock_ofport.return_value \u003d ovs_port_iface"},{"line_number":260,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"67384793_6a9caa24","line":257,"range":{"start_line":257,"start_character":38,"end_line":257,"end_character":49},"in_reply_to":"3f2f807a_e1059f4d","updated":"2024-10-09 11:53:35.000000000","message":"Done","commit_id":"6e47cb5ea07d20d0401568e3b9888fcc1d9a9eea"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"fc7d316db6dcdb7e7b82f6ebb4d30ad4d2ae782e","unresolved":true,"context_lines":[{"line_number":276,"context_line":"        self.mock_ovs_vsctl.ovs_vsctl.assert_has_calls(vsctl_expected_calls)"},{"line_number":277,"context_line":"        self.mock_ovs_vsctl.ovs_ofctl.assert_has_calls(ofctl_expected_calls)"},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"        self.assertEqual(len(vsctl_expected_calls),"},{"line_number":280,"context_line":"                         self.mock_ovs_vsctl.ovs_vsctl.call_count)"},{"line_number":281,"context_line":"        self.assertEqual(len(ofctl_expected_calls),"},{"line_number":282,"context_line":"                         self.mock_ovs_vsctl.ovs_ofctl.call_count)"},{"line_number":283,"context_line":"        mock_ofport.assert_called_once_with(ovs_port)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    def test_remove_evpn_router_ovs_flows_no_ovs_port(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"29cbf656_32f8011a","line":282,"range":{"start_line":279,"start_character":0,"end_line":282,"end_character":66},"updated":"2024-10-09 11:24:43.000000000","message":"This is redunand to the L276 and 278 - if the length was different to the call_count it would have failed there already.","commit_id":"6e47cb5ea07d20d0401568e3b9888fcc1d9a9eea"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d0ea6fefd67002d03b7c5ba8e393c82854267f36","unresolved":false,"context_lines":[{"line_number":276,"context_line":"        self.mock_ovs_vsctl.ovs_vsctl.assert_has_calls(vsctl_expected_calls)"},{"line_number":277,"context_line":"        self.mock_ovs_vsctl.ovs_ofctl.assert_has_calls(ofctl_expected_calls)"},{"line_number":278,"context_line":""},{"line_number":279,"context_line":"        self.assertEqual(len(vsctl_expected_calls),"},{"line_number":280,"context_line":"                         self.mock_ovs_vsctl.ovs_vsctl.call_count)"},{"line_number":281,"context_line":"        self.assertEqual(len(ofctl_expected_calls),"},{"line_number":282,"context_line":"                         self.mock_ovs_vsctl.ovs_ofctl.call_count)"},{"line_number":283,"context_line":"        mock_ofport.assert_called_once_with(ovs_port)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    def test_remove_evpn_router_ovs_flows_no_ovs_port(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"e4315511_4f72ec7f","line":282,"range":{"start_line":279,"start_character":0,"end_line":282,"end_character":66},"in_reply_to":"29cbf656_32f8011a","updated":"2024-10-09 11:53:35.000000000","message":"Done","commit_id":"6e47cb5ea07d20d0401568e3b9888fcc1d9a9eea"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"fc7d316db6dcdb7e7b82f6ebb4d30ad4d2ae782e","unresolved":true,"context_lines":[{"line_number":302,"context_line":"        net \u003d \u0027fake-net\u0027"},{"line_number":303,"context_line":"        mock_ip_version.return_value \u003d ip_version"},{"line_number":304,"context_line":"        mock_ofport.return_value \u003d ovs_port_iface"},{"line_number":305,"context_line":"        self.mock_ovs_vsctl.ovs_vsctl.side_effect \u003d [ovs_port]"},{"line_number":306,"context_line":"        self.mock_ovs_vsctl.ovs_ofctl.side_effect \u003d None"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"        ovs_utils.remove_evpn_network_ovs_flow("}],"source_content_type":"text/x-python","patch_set":6,"id":"0011dcbf_016ad0bf","line":305,"range":{"start_line":305,"start_character":38,"end_line":305,"end_character":49},"updated":"2024-10-09 11:24:43.000000000","message":"`return_value`","commit_id":"6e47cb5ea07d20d0401568e3b9888fcc1d9a9eea"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d0ea6fefd67002d03b7c5ba8e393c82854267f36","unresolved":false,"context_lines":[{"line_number":302,"context_line":"        net \u003d \u0027fake-net\u0027"},{"line_number":303,"context_line":"        mock_ip_version.return_value \u003d ip_version"},{"line_number":304,"context_line":"        mock_ofport.return_value \u003d ovs_port_iface"},{"line_number":305,"context_line":"        self.mock_ovs_vsctl.ovs_vsctl.side_effect \u003d [ovs_port]"},{"line_number":306,"context_line":"        self.mock_ovs_vsctl.ovs_ofctl.side_effect \u003d None"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"        ovs_utils.remove_evpn_network_ovs_flow("}],"source_content_type":"text/x-python","patch_set":6,"id":"69db1e7e_f419f1e3","line":305,"range":{"start_line":305,"start_character":38,"end_line":305,"end_character":49},"in_reply_to":"0011dcbf_016ad0bf","updated":"2024-10-09 11:53:35.000000000","message":"Done","commit_id":"6e47cb5ea07d20d0401568e3b9888fcc1d9a9eea"}]}