)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7c3126fb9a1226456299aa46358ad898c3e74c7d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"253679be_9860c0c5","updated":"2022-03-22 15:00:36.000000000","message":"Code looks fine and that should be enough to fix the issue.\n\nNow we need:\n1) Wait for a ovsdbapp new release with [1]. If I\u0027m not wrong, independently of the ovs version (\u003e\u003d2.17 or \u003c2.17), we\u0027ll need the latest ovsdbapp version always. We need to bump the lower-requirements.txt too.\n2) Add functional tests, initializing \"OvnInitPGNbIdl\" and \"MetadataAgentOvnSbIdl\" and testing the \".condition\" member.\n\n[1]https://review.opendev.org/c/openstack/ovsdbapp/+/834544","commit_id":"af04908e629f40e98453d2d354cb5ebc7bdfc4da"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"934b5a81bbbdede8d2ad17a2b63879b627a870b3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"8b7b626c_a3e76cd8","updated":"2022-03-23 01:42:09.000000000","message":"Thanks for the review. Now to wait for the ovsdbapp patch/release.","commit_id":"af04908e629f40e98453d2d354cb5ebc7bdfc4da"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"934b5a81bbbdede8d2ad17a2b63879b627a870b3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"83ce5d6d_4436cfee","in_reply_to":"253679be_9860c0c5","updated":"2022-03-23 01:42:09.000000000","message":"This particular patch doesn\u0027t technically depend on the ovsdbapp change, but for ovs\u003e\u003d2.17 we will be broken without both ovsdbapp and neutron changes. So we\u0027ll have to backport both changes however far we want to support ovs \u003e\u003d 2.17 and bump ovs release requirements accordingly.\n\nRe: tests, with ovs master and the existing neutron code, the  ovn.mech_driver.test_mech_driver tests fail/timeout with the error in the lp bug (with or without the ovsdbapp change). More tests are always good, but this particular bug is at least already reproducible with the existing functional tests. We just haven\u0027t gotten the ovs 2.17.0 release yet to break everything. (I\u0027m specifically holding off releasing it until this is all fixed). With the patch, the mech_driver tests pass for me.\n\nOnce we have a new ovsdbapp release, it might be good to see if we can pin python-ovs to a git checkout to verify nothing breaks horribly before I do the new python-ovs release.","commit_id":"af04908e629f40e98453d2d354cb5ebc7bdfc4da"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"836520501ae65d925f8e429adb4b76a04ea7ba98","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"01355eb2_5ee0d89f","updated":"2022-03-30 13:00:50.000000000","message":"Ah, code looks OK. It will have my +2 after the ovsdbapp release and version bump.","commit_id":"4d58eb33fa9ea69d8500ae6faf952d9b1fa78174"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"5f8e6b00018cf3b983f0de0cb218f3df7bb25b0f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6544ee37_6ea99757","updated":"2022-04-11 17:55:51.000000000","message":"If I understand well we dont need to bump ovsdbapp (1.16.0?)","commit_id":"4d58eb33fa9ea69d8500ae6faf952d9b1fa78174"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7ad9565e8a5e61b09234a73db63c5ecaf9f95cb8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"5b0b0991_3219f30f","updated":"2022-04-05 09:42:59.000000000","message":"Releases patch: https://review.opendev.org/c/openstack/releases/+/836584","commit_id":"4d58eb33fa9ea69d8500ae6faf952d9b1fa78174"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"db06556b79856ba9b307feaa2b3373a42970710b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0fbbf556_2b490cc9","updated":"2022-03-30 13:00:04.000000000","message":"We can\u0027t merge this patch yet. We need a new ovsdbapp release (\"openstack/releases\" didn\u0027t open yet the Zed branch) and we need to bump the ovsdbapp version.","commit_id":"4d58eb33fa9ea69d8500ae6faf952d9b1fa78174"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"9f43b379247e1e5dc63dc55ba75da0aa4b995986","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"43b85557_23ae54e9","in_reply_to":"0fbbf556_2b490cc9","updated":"2022-04-01 12:28:22.000000000","message":"+1\nI suppose next week we will have Zed folder in releases, so we can release from next week if we have anything to be released :-)","commit_id":"4d58eb33fa9ea69d8500ae6faf952d9b1fa78174"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ea0fa0b94cac331a3d61043fee9d2eba05f12bca","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"05bf14ec_7caa2ba1","in_reply_to":"6544ee37_6ea99757","updated":"2022-04-12 14:21:43.000000000","message":"We do, without the ovsdbapp patch, we won\u0027t be setting the table.condition correctly:\n  table.condition \u003d idl.ConditionState()\n\nupper-constraints.txt has 1.16.0, we can bump it now","commit_id":"4d58eb33fa9ea69d8500ae6faf952d9b1fa78174"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7c3126fb9a1226456299aa46358ad898c3e74c7d","unresolved":true,"context_lines":[{"line_number":970,"context_line":"        Stream.ssl_set_ca_cert_file(ca_cert_file)"},{"line_number":971,"context_line":""},{"line_number":972,"context_line":""},{"line_number":973,"context_line":"def set_idl_table_condition(idl, table_name, condition):"},{"line_number":974,"context_line":"    # Prior to ovs commit 46d44cf3be0, self.cond_change() doesn\u0027t work here but"},{"line_number":975,"context_line":"    # after that commit, setting table.condtion doesn\u0027t work."},{"line_number":976,"context_line":"    if hasattr(ovs_idl_mod, \u0027ConditionState\u0027):"}],"source_content_type":"text/x-python","patch_set":1,"id":"2b391b5b_a5dbec86","line":973,"range":{"start_line":973,"start_character":4,"end_line":973,"end_character":27},"updated":"2022-03-22 15:00:36.000000000","message":"**micro nit**: I would have implemented this in \"Ml2OvnIdlBase\"","commit_id":"af04908e629f40e98453d2d354cb5ebc7bdfc4da"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"934b5a81bbbdede8d2ad17a2b63879b627a870b3","unresolved":false,"context_lines":[{"line_number":970,"context_line":"        Stream.ssl_set_ca_cert_file(ca_cert_file)"},{"line_number":971,"context_line":""},{"line_number":972,"context_line":""},{"line_number":973,"context_line":"def set_idl_table_condition(idl, table_name, condition):"},{"line_number":974,"context_line":"    # Prior to ovs commit 46d44cf3be0, self.cond_change() doesn\u0027t work here but"},{"line_number":975,"context_line":"    # after that commit, setting table.condtion doesn\u0027t work."},{"line_number":976,"context_line":"    if hasattr(ovs_idl_mod, \u0027ConditionState\u0027):"}],"source_content_type":"text/x-python","patch_set":1,"id":"e40e93d6_39b71e7a","line":973,"range":{"start_line":973,"start_character":4,"end_line":973,"end_character":27},"in_reply_to":"2b391b5b_a5dbec86","updated":"2022-03-23 01:42:09.000000000","message":"Good point, will do.","commit_id":"af04908e629f40e98453d2d354cb5ebc7bdfc4da"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7c3126fb9a1226456299aa46358ad898c3e74c7d","unresolved":true,"context_lines":[{"line_number":971,"context_line":""},{"line_number":972,"context_line":""},{"line_number":973,"context_line":"def set_idl_table_condition(idl, table_name, condition):"},{"line_number":974,"context_line":"    # Prior to ovs commit 46d44cf3be0, self.cond_change() doesn\u0027t work here but"},{"line_number":975,"context_line":"    # after that commit, setting table.condtion doesn\u0027t work."},{"line_number":976,"context_line":"    if hasattr(ovs_idl_mod, \u0027ConditionState\u0027):"},{"line_number":977,"context_line":"        idl.cond_change(table_name, condition)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fa52a7a_1066ad3a","line":974,"range":{"start_line":974,"start_character":4,"end_line":974,"end_character":5},"updated":"2022-03-22 15:00:36.000000000","message":"I\u0027ll add a note saying that second branch won\u0027t be necessary once ovsdbapp\u003e\u003d1.16.0 and ovs\u003e\u003d2.17.0","commit_id":"af04908e629f40e98453d2d354cb5ebc7bdfc4da"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"934b5a81bbbdede8d2ad17a2b63879b627a870b3","unresolved":false,"context_lines":[{"line_number":971,"context_line":""},{"line_number":972,"context_line":""},{"line_number":973,"context_line":"def set_idl_table_condition(idl, table_name, condition):"},{"line_number":974,"context_line":"    # Prior to ovs commit 46d44cf3be0, self.cond_change() doesn\u0027t work here but"},{"line_number":975,"context_line":"    # after that commit, setting table.condtion doesn\u0027t work."},{"line_number":976,"context_line":"    if hasattr(ovs_idl_mod, \u0027ConditionState\u0027):"},{"line_number":977,"context_line":"        idl.cond_change(table_name, condition)"}],"source_content_type":"text/x-python","patch_set":1,"id":"76e3158c_e0e4574b","line":974,"range":{"start_line":974,"start_character":4,"end_line":974,"end_character":5},"in_reply_to":"3fa52a7a_1066ad3a","updated":"2022-03-23 01:42:09.000000000","message":"Will do with ovs\u003e\u003d2.17.0. ovsdbapp version doesn\u0027t really matter for this method , it\u0027s just that ovsdbapp\u0027s update_tables() is also busted and we use that elsewhere in neutron.","commit_id":"af04908e629f40e98453d2d354cb5ebc7bdfc4da"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ccf0c4582b1f832b7a1fa2d31bdd37ad460bdd5f","unresolved":true,"context_lines":[{"line_number":694,"context_line":"        super(Ml2OvnIdlBase, self).__init__("},{"line_number":695,"context_line":"            remote, schema, probe_interval\u003dprobe_interval, **kwargs)"},{"line_number":696,"context_line":""},{"line_number":697,"context_line":"    def set_table_condition(self, table_name, condition):"},{"line_number":698,"context_line":"        # Prior to ovs commit 46d44cf3be0, self.cond_change() doesn\u0027t work here"},{"line_number":699,"context_line":"        # but after that commit, setting table.condtion doesn\u0027t work."},{"line_number":700,"context_line":"        if hasattr(ovs_idl_mod, \u0027ConditionState\u0027):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a14883e_42489472","line":697,"updated":"2022-04-19 17:37:53.000000000","message":"I think we should test the behavior of this method by unit tests.","commit_id":"a13a4cb2da03e2cdce04b0e7b4bc30e44ebdbd02"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"d423d248aa72c046cee3abfa02c0dd48156a393c","unresolved":true,"context_lines":[{"line_number":694,"context_line":"        super(Ml2OvnIdlBase, self).__init__("},{"line_number":695,"context_line":"            remote, schema, probe_interval\u003dprobe_interval, **kwargs)"},{"line_number":696,"context_line":""},{"line_number":697,"context_line":"    def set_table_condition(self, table_name, condition):"},{"line_number":698,"context_line":"        # Prior to ovs commit 46d44cf3be0, self.cond_change() doesn\u0027t work here"},{"line_number":699,"context_line":"        # but after that commit, setting table.condtion doesn\u0027t work."},{"line_number":700,"context_line":"        if hasattr(ovs_idl_mod, \u0027ConditionState\u0027):"}],"source_content_type":"text/x-python","patch_set":4,"id":"fc7d21b0_316ff329","line":697,"in_reply_to":"7a14883e_42489472","updated":"2022-04-19 19:03:03.000000000","message":"There are 3 cases:\n\u003c 2.17.0: No ConditionState, can\u0027t use cond_change(), setting/reading condition directly not officially supported\n\u003d 2.17.[01]: ConditionState, must use cond_change(), setting/reading condition directly not officially supported\n\u003e 2.17.1: ConditionState, can assign/read table.condition or use cond_change(), setting/reading condition directly officially supported.\n\nSo there\u0027s going to be a very small window where this code is going to be needed at all. And the PyPI release of 2.17.0 may actually have the fix since it hasn\u0027t been released yet. (distros will not).\n\nBut basically, there being a ConditionState attribute won\u0027t universally mean we *should* call cond_change(), just that we *can*. So I don\u0027t want to enshrine in a unit test that this exact behavior is maintained, because if it is modified at some point to just do table.condition \u003d condition, that\u0027d be fine. I\u0027m still thinking about how to write a test that I don\u0027t hate, but I also can\u0027t imagine this method *ever* changing other than to be deleted. And if it was, I\u0027d prefer to find a way to write a test that doesn\u0027t just have to change with it.\n\nThis brings up another issue, RAFT support in python-ovs is 2.17+. We have code to support non-leaderonly connections, but if someone uses RAFT with ovs \u003c\u003d 2.17 (or a distro with the backported RAFT support), every time it does a snapshot it will transfer leaders and break connections--which will not be fast reconnects, but instead full db dumps. So, when should we bump the requirement to 2.17+ (it\u0027s very new)? If it was soon, then requiring 2.17.2 would also make this code go away.","commit_id":"a13a4cb2da03e2cdce04b0e7b4bc30e44ebdbd02"}]}