)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"69d9dbc38373dbf5491d936ece97b422d8104e1a","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     ROBERTO BARTZEN ACOSTA \u003crbartzen@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2022-11-04 17:22:44 -0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"OVN: Fix sql queries when using ovn plugin. The RouterExtraAttributes table is not populated by neutron in this case and must be skipped including the always true statement. Verified with Openstack Yoga + OVN deployment."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I335f9440c31a4b63253edb9f8dc32e1ea26678c3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"6af524bd_0d0eccb3","line":7,"range":{"start_line":7,"start_character":174,"end_line":7,"end_character":220},"updated":"2022-11-07 13:13:18.000000000","message":"This doesn\u0027t belong into the commit message, it is good enough if you add that as comment to the patch.\n\nAlso the same thing I wrote for your other patch holds true here. \"Fix sql queries when using ovn plugin\" (without trailing dot) is fine for the header, then empty line, then the description of the patch.","commit_id":"8e1fea837ddc96582d7ba9c7afcb22d6e991d6b9"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"2a171165ecc59f0c2d9d4906249c8ce338763069","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     ROBERTO BARTZEN ACOSTA \u003crbartzen@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2022-11-04 17:22:44 -0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"OVN: Fix sql queries when using ovn plugin. The RouterExtraAttributes table is not populated by neutron in this case and must be skipped including the always true statement. Verified with Openstack Yoga + OVN deployment."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I335f9440c31a4b63253edb9f8dc32e1ea26678c3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3451df7a_48c4e38f","line":7,"range":{"start_line":7,"start_character":174,"end_line":7,"end_character":220},"in_reply_to":"6af524bd_0d0eccb3","updated":"2022-11-10 14:10:17.000000000","message":"Done","commit_id":"8e1fea837ddc96582d7ba9c7afcb22d6e991d6b9"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"776726119d7ae65c7a61d6134bc3c1a542256235","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix sql queries when using ovn plugin"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The RouterExtraAttributes table is not populated by neutron in this case and must be skipped including the always true statement."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Change-Id: I335f9440c31a4b63253edb9f8dc32e1ea26678c3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"a1431582_b10fcfa8","line":9,"range":{"start_line":9,"start_character":0,"end_line":9,"end_character":129},"updated":"2022-11-10 09:40:52.000000000","message":"nit: please break this line at ~80chars","commit_id":"c385cda1189f9acbf3cf07d5608b371b325405b1"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"2a171165ecc59f0c2d9d4906249c8ce338763069","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix sql queries when using ovn plugin"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The RouterExtraAttributes table is not populated by neutron in this case and must be skipped including the always true statement."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Change-Id: I335f9440c31a4b63253edb9f8dc32e1ea26678c3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"ea9ec588_9f30daca","line":9,"range":{"start_line":9,"start_character":0,"end_line":9,"end_character":129},"in_reply_to":"a1431582_b10fcfa8","updated":"2022-11-10 14:10:17.000000000","message":"Done","commit_id":"c385cda1189f9acbf3cf07d5608b371b325405b1"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"69d9dbc38373dbf5491d936ece97b422d8104e1a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"d60a8606_60293dde","updated":"2022-11-07 13:13:18.000000000","message":"This looks interesting, I made some attempts at solving this issue earlier, but couldn\u0027t finish it. Simply adding the missing attributes sounds like a good idea, we\u0027ll have to add some test job around this to verify it.\n\nFirst the pep8 issues need fixing, please let me know if you need help with that. Also rebase the patch on top of the updated patch below it.","commit_id":"8e1fea837ddc96582d7ba9c7afcb22d6e991d6b9"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"9e069d73378d0bad38cb050e2598e4e0e286c50c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"7377a892_f772622a","updated":"2022-11-08 17:46:42.000000000","message":"I\u0027m not sure what is expected from these queries. However, the child register \"router_extra_attributes\" is requested in most of them, as an outerjoin. It won\u0027t be present because the register is not created.\n\nIMO, what I\u0027m proposing in [1] is more reliable. The child register \"router_extra_attributes\" should always be created when a \"router\" register is. Although this register is not currently used by ML2/OVN, it will be in future features.\n\n[1] is also backportable to previous releases.\n\n[1]https://review.opendev.org/c/openstack/neutron/+/864051","commit_id":"c385cda1189f9acbf3cf07d5608b371b325405b1"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"2a171165ecc59f0c2d9d4906249c8ce338763069","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"120f8bb4_f8680f38","in_reply_to":"7377a892_f772622a","updated":"2022-11-10 14:10:17.000000000","message":"I completely agree with this approach. If neutron-dynamic-routing needs these extra attributes, the correct way would be that they are always filled on the neutron side.","commit_id":"c385cda1189f9acbf3cf07d5608b371b325405b1"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"2a171165ecc59f0c2d9d4906249c8ce338763069","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"b0923d7d_f1fae77a","updated":"2022-11-10 14:10:17.000000000","message":"I also checked your patch [1], and it works for me (without applying this one) - thanks.\n\n[1] - https://review.opendev.org/c/openstack/neutron/+/864051","commit_id":"3c05adc89ed19c8eb15216ff039f55959a74ddff"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"059125e04b15a7ece6b470c9de53d87606a70979","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"6db4b9ba_284b956f","updated":"2022-11-25 09:27:18.000000000","message":"Please consider abandoning this patch. Thanks a lot for the time spent on it and raising the visibility of this issue.","commit_id":"3c05adc89ed19c8eb15216ff039f55959a74ddff"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"f763b58bee7660eb9b8a22c7c9e19b303542d7d0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"df84a2e8_8f3c0402","updated":"2022-11-23 19:59:26.000000000","message":"So it seems this patch is obsolete now and can be abandoned. But thank you very much for triggering the proper solution for this long standing issue having been found and implemented.","commit_id":"3c05adc89ed19c8eb15216ff039f55959a74ddff"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"29681514b2b0293d3e7d7aefee2947da17dd7ceb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"80079b6b_9863f958","updated":"2022-11-25 13:12:14.000000000","message":"Thank you for fix it on the neutron side Rodolfo.","commit_id":"3c05adc89ed19c8eb15216ff039f55959a74ddff"}],"neutron_dynamic_routing/db/bgp_db.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"782824c54ce37a0da8a71ee78cdde3a5b22263b6","unresolved":true,"context_lines":[{"line_number":1212,"context_line":"                            ctx, id\u003dgw_subnet.subnetpool_id)"},{"line_number":1213,"context_line":"                if ext_pool is not None:"},{"line_number":1214,"context_line":"                    ext_scope_set \u003d ext_net_subnetpool_map.get(ext_net_id,"},{"line_number":1215,"context_line":"                    set())"},{"line_number":1216,"context_line":"                    ext_scope_set.add(ext_pool.address_scope_id)"},{"line_number":1217,"context_line":"                    ext_net_subnetpool_map[ext_net_id] \u003d ext_scope_set"},{"line_number":1218,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"721dd43a_a9f31ac6","line":1215,"range":{"start_line":1215,"start_character":0,"end_line":1215,"end_character":26},"updated":"2022-11-08 07:31:02.000000000","message":"fix indentation here, I\u0027m surprise pep8 did not complain about it","commit_id":"ebd4b1d9e5609b39205bdce295d1082621c0c183"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"2a171165ecc59f0c2d9d4906249c8ce338763069","unresolved":false,"context_lines":[{"line_number":1212,"context_line":"                            ctx, id\u003dgw_subnet.subnetpool_id)"},{"line_number":1213,"context_line":"                if ext_pool is not None:"},{"line_number":1214,"context_line":"                    ext_scope_set \u003d ext_net_subnetpool_map.get(ext_net_id,"},{"line_number":1215,"context_line":"                    set())"},{"line_number":1216,"context_line":"                    ext_scope_set.add(ext_pool.address_scope_id)"},{"line_number":1217,"context_line":"                    ext_net_subnetpool_map[ext_net_id] \u003d ext_scope_set"},{"line_number":1218,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"0b3b3a22_6b4123cd","line":1215,"range":{"start_line":1215,"start_character":0,"end_line":1215,"end_character":26},"in_reply_to":"721dd43a_a9f31ac6","updated":"2022-11-10 14:10:17.000000000","message":"I was surprised too, sorry about that.","commit_id":"ebd4b1d9e5609b39205bdce295d1082621c0c183"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"9e069d73378d0bad38cb050e2598e4e0e286c50c","unresolved":true,"context_lines":[{"line_number":619,"context_line":"                binding_alias.bgp_speaker_id \u003d\u003d BgpSpeaker.id,"},{"line_number":620,"context_line":"                BgpSpeaker.id \u003d\u003d bgp_speaker_id,"},{"line_number":621,"context_line":"                BgpSpeaker.advertise_floating_ip_host_routes \u003d\u003d sa.sql.true())"},{"line_number":622,"context_line":"            query \u003d query.outerjoin(router_attrs,"},{"line_number":623,"context_line":"                                    l3_db.Router.id \u003d\u003d router_attrs.router_id)"},{"line_number":624,"context_line":"            query \u003d query.filter(router_attrs.distributed !\u003d sa.sql.true())"},{"line_number":625,"context_line":"            return self._host_route_list_from_tuples(query.all())"},{"line_number":626,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"08fc4508_969bfeeb","line":623,"range":{"start_line":622,"start_character":12,"end_line":623,"end_character":78},"updated":"2022-11-08 17:46:42.000000000","message":"This join will return nothing in case of OVN. I really don\u0027t know if that is OK, but looks wrong.","commit_id":"c385cda1189f9acbf3cf07d5608b371b325405b1"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"2a171165ecc59f0c2d9d4906249c8ce338763069","unresolved":false,"context_lines":[{"line_number":619,"context_line":"                binding_alias.bgp_speaker_id \u003d\u003d BgpSpeaker.id,"},{"line_number":620,"context_line":"                BgpSpeaker.id \u003d\u003d bgp_speaker_id,"},{"line_number":621,"context_line":"                BgpSpeaker.advertise_floating_ip_host_routes \u003d\u003d sa.sql.true())"},{"line_number":622,"context_line":"            query \u003d query.outerjoin(router_attrs,"},{"line_number":623,"context_line":"                                    l3_db.Router.id \u003d\u003d router_attrs.router_id)"},{"line_number":624,"context_line":"            query \u003d query.filter(router_attrs.distributed !\u003d sa.sql.true())"},{"line_number":625,"context_line":"            return self._host_route_list_from_tuples(query.all())"},{"line_number":626,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"09211ac9_629af15b","line":623,"range":{"start_line":622,"start_character":12,"end_line":623,"end_character":78},"in_reply_to":"08fc4508_969bfeeb","updated":"2022-11-10 14:10:17.000000000","message":"Hi Rodolfo, thanks for the feedback.\n\nSo, about the sql queries:\nAll the tree query.outerjoin in this file does not apply to the OVN case. This outerjoin would serve to confirm that the l3.db.router.id is the same as the extra attribute table, and then filter the outer_attrs.distributed attribute.\nIn the case of previously set parameters (proposed fix), the filters by \"l3_db.Router.id \u003d\u003d router_attrs.router_id and\nrouter_attrs.distributed !\u003d sa.sql.true()\" will not change the original query because they will no longer consult in the router_extra_attributes table, but in the l3_db.Router table itself.\n\nIs it correct or did I miss something?\n\nFor example, I can see networks and FIPs in advertised routes of bgpspeaker:\nroot@jump-host:~# openstack floating ip list\n+--------------------------------------+---------------------+------------------+--------------------------------------+--------------------------------------+----------------------------------+\n| ID                                   | Floating IP Address | Fixed IP Address | Port                                 | Floating Network                     | Project                          |\n+--------------------------------------+---------------------+------------------+--------------------------------------+--------------------------------------+----------------------------------+\n| dabcfb33-849f-406c-8aad-19721b6f00df | 200.201.0.130       | 192.168.0.107    | b2da64b6-6746-4a6d-a1ca-e74ed7f36b4d | faf32d19-9633-4e46-82af-52cc102a2eb4 | d11daecfe9d847ddb7d9ce2932c2fe26 |\n+--------------------------------------+---------------------+------------------+--------------------------------------+--------------------------------------+----------------------------------+\nroot@jump-host:~# openstack bgp speaker list advertised routes bgpspeaker-ipv4\n+------------------+---------------+\n| Destination      | Nexthop       |\n+------------------+---------------+\n| 200.201.0.130/32 | 200.201.0.132 |\n| 192.168.0.0/24   | 200.201.0.132 |\n+------------------+---------------+\nroot@jump-host:~#","commit_id":"c385cda1189f9acbf3cf07d5608b371b325405b1"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"776726119d7ae65c7a61d6134bc3c1a542256235","unresolved":true,"context_lines":[{"line_number":619,"context_line":"                binding_alias.bgp_speaker_id \u003d\u003d BgpSpeaker.id,"},{"line_number":620,"context_line":"                BgpSpeaker.id \u003d\u003d bgp_speaker_id,"},{"line_number":621,"context_line":"                BgpSpeaker.advertise_floating_ip_host_routes \u003d\u003d sa.sql.true())"},{"line_number":622,"context_line":"            query \u003d query.outerjoin(router_attrs,"},{"line_number":623,"context_line":"                                    l3_db.Router.id \u003d\u003d router_attrs.router_id)"},{"line_number":624,"context_line":"            query \u003d query.filter(router_attrs.distributed !\u003d sa.sql.true())"},{"line_number":625,"context_line":"            return self._host_route_list_from_tuples(query.all())"},{"line_number":626,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"bbeaef0e_e651749e","line":623,"range":{"start_line":622,"start_character":12,"end_line":623,"end_character":78},"in_reply_to":"08fc4508_969bfeeb","updated":"2022-11-10 09:40:52.000000000","message":"shall this join be conditional totally?","commit_id":"c385cda1189f9acbf3cf07d5608b371b325405b1"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"834d5870499c1f6f485943ad504781cd35c88446","unresolved":true,"context_lines":[{"line_number":619,"context_line":"                binding_alias.bgp_speaker_id \u003d\u003d BgpSpeaker.id,"},{"line_number":620,"context_line":"                BgpSpeaker.id \u003d\u003d bgp_speaker_id,"},{"line_number":621,"context_line":"                BgpSpeaker.advertise_floating_ip_host_routes \u003d\u003d sa.sql.true())"},{"line_number":622,"context_line":"            query \u003d query.outerjoin(router_attrs,"},{"line_number":623,"context_line":"                                    l3_db.Router.id \u003d\u003d router_attrs.router_id)"},{"line_number":624,"context_line":"            query \u003d query.filter(router_attrs.distributed !\u003d sa.sql.true())"},{"line_number":625,"context_line":"            return self._host_route_list_from_tuples(query.all())"},{"line_number":626,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"2353dc46_07d9ef8c","line":623,"range":{"start_line":622,"start_character":12,"end_line":623,"end_character":78},"in_reply_to":"42219f08_7959456f","updated":"2022-11-10 12:51:26.000000000","message":"cool, I check this patch","commit_id":"c385cda1189f9acbf3cf07d5608b371b325405b1"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"839b0e2c05a048b43c15057d526f65afd005ece7","unresolved":true,"context_lines":[{"line_number":619,"context_line":"                binding_alias.bgp_speaker_id \u003d\u003d BgpSpeaker.id,"},{"line_number":620,"context_line":"                BgpSpeaker.id \u003d\u003d bgp_speaker_id,"},{"line_number":621,"context_line":"                BgpSpeaker.advertise_floating_ip_host_routes \u003d\u003d sa.sql.true())"},{"line_number":622,"context_line":"            query \u003d query.outerjoin(router_attrs,"},{"line_number":623,"context_line":"                                    l3_db.Router.id \u003d\u003d router_attrs.router_id)"},{"line_number":624,"context_line":"            query \u003d query.filter(router_attrs.distributed !\u003d sa.sql.true())"},{"line_number":625,"context_line":"            return self._host_route_list_from_tuples(query.all())"},{"line_number":626,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"42219f08_7959456f","line":623,"range":{"start_line":622,"start_character":12,"end_line":623,"end_character":78},"in_reply_to":"bbeaef0e_e651749e","updated":"2022-11-10 11:03:57.000000000","message":"No, what I\u0027m saying is that being a \"outerjoin\", if the \"RouterExtraAttributes\" register does not exist, the values will be null. I don\u0027t think the methods calling this query are expecting this.\n\nThis is why I\u0027m proposing https://review.opendev.org/c/openstack/neutron/+/864051. This patch will always create a \"RouterExtraAttributes\" register, regardless of the ML2 plugin used. By default, this register will be populated with the default values (no dvr, no ha, etc).","commit_id":"c385cda1189f9acbf3cf07d5608b371b325405b1"}]}
