)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c3e77b01f5a00ad2c243322f8a9e8531c94a85eb","unresolved":true,"context_lines":[{"line_number":16,"context_line":"requiring services such as the OVN Metadata Agent or DHCP Agent to serve"},{"line_number":17,"context_line":"those external ports and running them on control plane nodes are not"},{"line_number":18,"context_line":"ideal. This is where this patch comes handy allowing these ports to be"},{"line_number":19,"context_line":"scheduled on EDPM compute nodes."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"The patch is also backward compatible and if the new configuration is"},{"line_number":22,"context_line":"not present on the OVN CMS Options, ML2/IOVN will continue to schedule"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"a056bf88_0ebc4a47","line":19,"updated":"2023-10-03 21:07:21.000000000","message":"let\u0027s avoid the terminology here (EDPM is specific to OpenShift solution). Let\u0027s just say that it may be needed to have more flexibility in where the ports are scheduled.","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"310fd351d3f7eb6a49b5179849247a241fa762b1","unresolved":false,"context_lines":[{"line_number":16,"context_line":"requiring services such as the OVN Metadata Agent or DHCP Agent to serve"},{"line_number":17,"context_line":"those external ports and running them on control plane nodes are not"},{"line_number":18,"context_line":"ideal. This is where this patch comes handy allowing these ports to be"},{"line_number":19,"context_line":"scheduled on EDPM compute nodes."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"The patch is also backward compatible and if the new configuration is"},{"line_number":22,"context_line":"not present on the OVN CMS Options, ML2/IOVN will continue to schedule"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"af63f96d_e69d4174","line":19,"in_reply_to":"a056bf88_0ebc4a47","updated":"2023-10-04 09:36:07.000000000","message":"Sounds good","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c3e77b01f5a00ad2c243322f8a9e8531c94a85eb","unresolved":true,"context_lines":[{"line_number":19,"context_line":"scheduled on EDPM compute nodes."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"The patch is also backward compatible and if the new configuration is"},{"line_number":22,"context_line":"not present on the OVN CMS Options, ML2/IOVN will continue to schedule"},{"line_number":23,"context_line":"the external prots on ndoes configured with the previous configuration"},{"line_number":24,"context_line":"like always."},{"line_number":25,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"c842e3d6_8cccc495","line":22,"range":{"start_line":22,"start_character":36,"end_line":22,"end_character":44},"updated":"2023-10-03 21:07:21.000000000","message":"-\u003e ML2/OVN","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"310fd351d3f7eb6a49b5179849247a241fa762b1","unresolved":false,"context_lines":[{"line_number":19,"context_line":"scheduled on EDPM compute nodes."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"The patch is also backward compatible and if the new configuration is"},{"line_number":22,"context_line":"not present on the OVN CMS Options, ML2/IOVN will continue to schedule"},{"line_number":23,"context_line":"the external prots on ndoes configured with the previous configuration"},{"line_number":24,"context_line":"like always."},{"line_number":25,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"bea43571_cbb3d95e","line":22,"range":{"start_line":22,"start_character":36,"end_line":22,"end_character":44},"in_reply_to":"c842e3d6_8cccc495","updated":"2023-10-04 09:36:07.000000000","message":"Done","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c3e77b01f5a00ad2c243322f8a9e8531c94a85eb","unresolved":true,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"The patch is also backward compatible and if the new configuration is"},{"line_number":22,"context_line":"not present on the OVN CMS Options, ML2/IOVN will continue to schedule"},{"line_number":23,"context_line":"the external prots on ndoes configured with the previous configuration"},{"line_number":24,"context_line":"like always."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Documentation will be updated on a follow up patch."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"99b03140_cc92664d","line":23,"updated":"2023-10-03 21:07:21.000000000","message":"prots, ndoes -\u003e ports, nodes","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"310fd351d3f7eb6a49b5179849247a241fa762b1","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"The patch is also backward compatible and if the new configuration is"},{"line_number":22,"context_line":"not present on the OVN CMS Options, ML2/IOVN will continue to schedule"},{"line_number":23,"context_line":"the external prots on ndoes configured with the previous configuration"},{"line_number":24,"context_line":"like always."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Documentation will be updated on a follow up patch."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"5e863e76_94d4db00","line":23,"in_reply_to":"99b03140_cc92664d","updated":"2023-10-04 09:36:07.000000000","message":"Done","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c3e77b01f5a00ad2c243322f8a9e8531c94a85eb","unresolved":true,"context_lines":[{"line_number":23,"context_line":"the external prots on ndoes configured with the previous configuration"},{"line_number":24,"context_line":"like always."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Documentation will be updated on a follow up patch."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Closes-Bug: 2037294"},{"line_number":29,"context_line":"Change-Id: Ic46d847e3aebfe543d5a7ab49d18d1f1abf1342e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"c761c1aa_d340c10f","line":26,"updated":"2023-10-03 21:07:21.000000000","message":"which documentation? why not updating it here in place? or do you mean some docs external to the repo?","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":false,"context_lines":[{"line_number":23,"context_line":"the external prots on ndoes configured with the previous configuration"},{"line_number":24,"context_line":"like always."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Documentation will be updated on a follow up patch."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Closes-Bug: 2037294"},{"line_number":29,"context_line":"Change-Id: Ic46d847e3aebfe543d5a7ab49d18d1f1abf1342e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"94b2c161_9f3851d3","line":26,"in_reply_to":"6057d49a_f4df6ddf","updated":"2023-10-25 10:37:41.000000000","message":"Done","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97145c01d2a9f24d687c5eebf247bab435cdf36","unresolved":true,"context_lines":[{"line_number":23,"context_line":"the external prots on ndoes configured with the previous configuration"},{"line_number":24,"context_line":"like always."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Documentation will be updated on a follow up patch."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Closes-Bug: 2037294"},{"line_number":29,"context_line":"Change-Id: Ic46d847e3aebfe543d5a7ab49d18d1f1abf1342e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"6057d49a_f4df6ddf","line":26,"in_reply_to":"b8130d20_7b893f87","updated":"2023-10-13 09:38:41.000000000","message":"I agree, I will post a follow up patch with the documentation soon","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"310fd351d3f7eb6a49b5179849247a241fa762b1","unresolved":true,"context_lines":[{"line_number":23,"context_line":"the external prots on ndoes configured with the previous configuration"},{"line_number":24,"context_line":"like always."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Documentation will be updated on a follow up patch."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Closes-Bug: 2037294"},{"line_number":29,"context_line":"Change-Id: Ic46d847e3aebfe543d5a7ab49d18d1f1abf1342e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"f917e5f5_7a76d262","line":26,"in_reply_to":"c761c1aa_d340c10f","updated":"2023-10-04 09:36:07.000000000","message":"No they are not external, is just that I usually document things on a separated patch because documentation in general tends to be nitpicked when being reviewed.\n\nBut I can do it either way if you prefer to have everything in the same patch.","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5e426fb86ed2325cfba3dfc6defda6fe7837f4f0","unresolved":true,"context_lines":[{"line_number":23,"context_line":"the external prots on ndoes configured with the previous configuration"},{"line_number":24,"context_line":"like always."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Documentation will be updated on a follow up patch."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Closes-Bug: 2037294"},{"line_number":29,"context_line":"Change-Id: Ic46d847e3aebfe543d5a7ab49d18d1f1abf1342e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"b8130d20_7b893f87","line":26,"in_reply_to":"f917e5f5_7a76d262","updated":"2023-10-12 17:10:08.000000000","message":"I do prefer it. But I don\u0027t insist. But I\u0027d like to see a follow-up with the docs posted to at least have some glimpse and to confirm my understanding of how the feature is going to be used is in line with your vision.","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5e426fb86ed2325cfba3dfc6defda6fe7837f4f0","unresolved":true,"context_lines":[{"line_number":7,"context_line":"[OVN] Enhanced external port scheduling"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch introduces a new configuration for OVN CMS Options called"},{"line_number":10,"context_line":"\"enable-chassis-as-extport-host\". This configuration is used by ML2/OVN"},{"line_number":11,"context_line":"to identify nodes that are eligible for scheduling OVN\u0027s external ports."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Prior to this patch, external ports were always scheduled on centralized"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"f10810f0_d8cb787c","line":10,"updated":"2023-10-12 17:10:08.000000000","message":"is used -\u003e can be used","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97145c01d2a9f24d687c5eebf247bab435cdf36","unresolved":false,"context_lines":[{"line_number":7,"context_line":"[OVN] Enhanced external port scheduling"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch introduces a new configuration for OVN CMS Options called"},{"line_number":10,"context_line":"\"enable-chassis-as-extport-host\". This configuration is used by ML2/OVN"},{"line_number":11,"context_line":"to identify nodes that are eligible for scheduling OVN\u0027s external ports."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Prior to this patch, external ports were always scheduled on centralized"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"2fda1026_c6436b50","line":10,"in_reply_to":"f10810f0_d8cb787c","updated":"2023-10-13 09:38:41.000000000","message":"Done","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7e970e452c490b54353a52a53c3900458d8c48df","unresolved":true,"context_lines":[{"line_number":24,"context_line":"the external ports on nodes configured with the previous configuration"},{"line_number":25,"context_line":"like always."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Documentation will be updated on a follow up patch."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Closes-Bug: 2037294"},{"line_number":30,"context_line":"Change-Id: Ic46d847e3aebfe543d5a7ab49d18d1f1abf1342e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"8ca7817e_2b48f55c","line":27,"updated":"2023-10-27 15:10:58.000000000","message":"this is still missing btw","commit_id":"90dd48843fde08d3a44e94df9a3adc52167f0925"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"aa4cfd4cb18e24babb26664ccee4d3c90ee245bb","unresolved":false,"context_lines":[{"line_number":24,"context_line":"the external ports on nodes configured with the previous configuration"},{"line_number":25,"context_line":"like always."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Documentation will be updated on a follow up patch."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Closes-Bug: 2037294"},{"line_number":30,"context_line":"Change-Id: Ic46d847e3aebfe543d5a7ab49d18d1f1abf1342e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"e9f03522_c298e2a9","line":27,"in_reply_to":"89315cdf_b591e50b","updated":"2023-11-03 16:26:50.000000000","message":"https://review.opendev.org/c/openstack/neutron/+/900030","commit_id":"90dd48843fde08d3a44e94df9a3adc52167f0925"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"932a2770d84c6cd8a9eb86afa6dd71d6f947ac60","unresolved":true,"context_lines":[{"line_number":24,"context_line":"the external ports on nodes configured with the previous configuration"},{"line_number":25,"context_line":"like always."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Documentation will be updated on a follow up patch."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Closes-Bug: 2037294"},{"line_number":30,"context_line":"Change-Id: Ic46d847e3aebfe543d5a7ab49d18d1f1abf1342e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"89315cdf_b591e50b","line":27,"in_reply_to":"8ca7817e_2b48f55c","updated":"2023-10-31 10:19:27.000000000","message":"Ops, yeah sorry I am little behind on the docs patch","commit_id":"90dd48843fde08d3a44e94df9a3adc52167f0925"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c591f83b1ab660102540ddcc71423fe03185b4ff","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"78989a71_94f2277d","updated":"2023-09-21 20:00:47.000000000","message":"I know you said not to review. I am sorry.\n\nI will mainly just say that I took a look and it seems like the right direction. There\u0027s of course lots of polishing to do, but you know that.","commit_id":"32238978ea8df28510601395bd889aebbfefda1b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"34c96dd100ad8c5c2be341152d716a76177e3451","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"917f7fbb_448026bd","updated":"2023-09-22 09:40:29.000000000","message":"Thanks for taking a look at the patch","commit_id":"32238978ea8df28510601395bd889aebbfefda1b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"310fd351d3f7eb6a49b5179849247a241fa762b1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"da48f67b_6aae3fe4","updated":"2023-10-04 09:36:07.000000000","message":"Thanks Ihar for the review","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"43b09634223ddad44b18183915f6d83bfa95a831","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"20c510b3_9b16d8a1","updated":"2023-09-29 14:50:35.000000000","message":"recheck\n\nfullstack failure at test_keepalived_multiple_sighups_does_not_forfeit_primary, unrelated to the patch","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"68b14873fbc5293fdb6ad852d833fda41e37373b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"fdeaa889_2ca7dd76","updated":"2023-09-29 12:29:20.000000000","message":"recheck\n\ntwo functional tests failed, none related to this patch:\n\ntest_dvr_ha_router_interface_mtu_update\ntest_restart_rpc_on_sighup_multiple_workers","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97145c01d2a9f24d687c5eebf247bab435cdf36","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"726cf916_59ee34ab","updated":"2023-10-13 09:38:41.000000000","message":"Thanks Ihar very much for the review! Really appreciate it","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"e960faa5_ef5ce590","updated":"2023-10-25 10:37:41.000000000","message":"Thanks very much ihar for the review","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"b0cb5af7_5f059149","updated":"2023-10-20 16:03:05.000000000","message":"This is a huge progress. I think the new implementation is drastically more readable, despite adding some necessary complexity.\n\nI left some more comments, some functional, some stylistic, some are nice-to-haves or just side notes that don\u0027t require being addressed.","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"eb121eb38af3d0d432ccb6d8b41020fe930b9728","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"3696dd0f_02853906","updated":"2023-10-27 15:08:14.000000000","message":"-1 just for the .additional_chassis (mis)handling in case pb.chassis is None. The rest is subjective and you are free to ignore.\n\nAlso, thanks for the explanation on the relation between MAX_GW_CHASSIS and the new const you introduce (which is at this point, strictly speaking, non-existing.)","commit_id":"90dd48843fde08d3a44e94df9a3adc52167f0925"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"10fb8700d93ec0eff20c9fd12a17f10f6932a5bd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"0f2bf729_c52fc898","updated":"2023-11-03 15:56:16.000000000","message":"Just a quick review after seeing the doc update, will need to review again.","commit_id":"7f021319ac4487dbf7bac1fa74f4e38b5630e3a2"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"6bea449f4fa6e0ac444085194912a2eea74a6029","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"e06eb590_838c7f94","updated":"2023-10-31 13:50:06.000000000","message":"Thank you very much for tenacity and dealing with such a petty reviewer. I hope you like the final version as much as I do.","commit_id":"7f021319ac4487dbf7bac1fa74f4e38b5630e3a2"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"19c7b17370e4ae0a5b3826c1bf62dc7913c7bb0a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"aecc7550_20dee090","updated":"2023-11-13 17:50:02.000000000","message":"Just one nit","commit_id":"973c500d4b4ed67e4ab0ece192fa01c14523eda1"}],"neutron/common/ovn/constants.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c3e77b01f5a00ad2c243322f8a9e8531c94a85eb","unresolved":true,"context_lines":[{"line_number":303,"context_line":"# Maximum chassis count where a gateway port can be hosted"},{"line_number":304,"context_line":"MAX_GW_CHASSIS \u003d 5"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"# Maximum number Chassis in a HA Chassis Group"},{"line_number":307,"context_line":"MAX_HA_CH_GRP_CHASSIS \u003d 3"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"UNKNOWN_ADDR \u003d \u0027unknown\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"97a1e0ec_58556be5","line":306,"updated":"2023-10-03 21:07:21.000000000","message":"maximum number OF chassis in...","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"310fd351d3f7eb6a49b5179849247a241fa762b1","unresolved":false,"context_lines":[{"line_number":303,"context_line":"# Maximum chassis count where a gateway port can be hosted"},{"line_number":304,"context_line":"MAX_GW_CHASSIS \u003d 5"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"# Maximum number Chassis in a HA Chassis Group"},{"line_number":307,"context_line":"MAX_HA_CH_GRP_CHASSIS \u003d 3"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"UNKNOWN_ADDR \u003d \u0027unknown\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"c442534c_e3acbe6c","line":306,"in_reply_to":"97a1e0ec_58556be5","updated":"2023-10-04 09:36:07.000000000","message":"Done","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c3e77b01f5a00ad2c243322f8a9e8531c94a85eb","unresolved":true,"context_lines":[{"line_number":304,"context_line":"MAX_GW_CHASSIS \u003d 5"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"# Maximum number Chassis in a HA Chassis Group"},{"line_number":307,"context_line":"MAX_HA_CH_GRP_CHASSIS \u003d 3"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"UNKNOWN_ADDR \u003d \u0027unknown\u0027"},{"line_number":310,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"c636932b_f92d0473","line":307,"updated":"2023-10-03 21:07:21.000000000","message":"MAX_CHASSIS_IN_HA_GROUP? seems more intelligible?","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"310fd351d3f7eb6a49b5179849247a241fa762b1","unresolved":false,"context_lines":[{"line_number":304,"context_line":"MAX_GW_CHASSIS \u003d 5"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"# Maximum number Chassis in a HA Chassis Group"},{"line_number":307,"context_line":"MAX_HA_CH_GRP_CHASSIS \u003d 3"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"UNKNOWN_ADDR \u003d \u0027unknown\u0027"},{"line_number":310,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"732b926c_9216e9d9","line":307,"in_reply_to":"c636932b_f92d0473","updated":"2023-10-04 09:36:07.000000000","message":"It does, I will change it","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5e426fb86ed2325cfba3dfc6defda6fe7837f4f0","unresolved":true,"context_lines":[{"line_number":304,"context_line":"MAX_GW_CHASSIS \u003d 5"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"# Maximum number of Chassis in a HA Chassis Group"},{"line_number":307,"context_line":"MAX_CHASSIS_IN_HA_GROUP \u003d 3"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"UNKNOWN_ADDR \u003d \u0027unknown\u0027"},{"line_number":310,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"33f592a7_d2af2f5e","line":307,"updated":"2023-10-12 17:10:08.000000000","message":"(Curious) What\u0027s the rationale on picking this exact number here? (I understand that the _GW_ counterpart doesn\u0027t have the rationale captured in code, but I wonder if we should try to capture it here for the new option. If we have one of course.)","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"eb121eb38af3d0d432ccb6d8b41020fe930b9728","unresolved":true,"context_lines":[{"line_number":304,"context_line":"MAX_GW_CHASSIS \u003d 5"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"# Maximum number of Chassis in a HA Chassis Group"},{"line_number":307,"context_line":"MAX_CHASSIS_IN_HA_GROUP \u003d 3"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"UNKNOWN_ADDR \u003d \u0027unknown\u0027"},{"line_number":310,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"e1b11a5c_0b3abcd5","line":307,"in_reply_to":"295791ae_428cf9c4","updated":"2023-10-27 15:08:14.000000000","message":"Thanks for the explanation. I think what I didn\u0027t realize is that this whole sync / get candidates business was not ever used for router code path. (It seems weird that we have two separate ways to \"get candidate chassis\", was that ever discussed? is it for historical reasons? - like maybe external ports were added later?)\n\nRegardless, I think having the two code paths behave similar (in the number of chassis) unless there\u0027s a good reason not to is the right approach, regardless if these code paths to \"get candidates\" are ever combined.","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97145c01d2a9f24d687c5eebf247bab435cdf36","unresolved":true,"context_lines":[{"line_number":304,"context_line":"MAX_GW_CHASSIS \u003d 5"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"# Maximum number of Chassis in a HA Chassis Group"},{"line_number":307,"context_line":"MAX_CHASSIS_IN_HA_GROUP \u003d 3"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"UNKNOWN_ADDR \u003d \u0027unknown\u0027"},{"line_number":310,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"97c8cd6f_d378152f","line":307,"in_reply_to":"33f592a7_d2af2f5e","updated":"2023-10-13 09:38:41.000000000","message":"I can improve the comment. But basically I\u0027m limiting the number because OVN uses BFD to monitor connectivity of the members of each group, so my rationale was that maybe in some deployments we could have a huge number of EDPM nodes and having each group to contain an unlimited number of chassis could potentially put a lot of stress on OVN to monitor it all.\n\nAnd the rationale for 3 is because, just like the -as-gw, those are scheduled on controller nodes which for deployments are almost always 3. So, I used that as a reference. But, if you think we should have more backups members we can bump this number.","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":304,"context_line":"MAX_GW_CHASSIS \u003d 5"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"# Maximum number of Chassis in a HA Chassis Group"},{"line_number":307,"context_line":"MAX_CHASSIS_IN_HA_GROUP \u003d 3"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"UNKNOWN_ADDR \u003d \u0027unknown\u0027"},{"line_number":310,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"b6dd6f92_c491baee","line":307,"in_reply_to":"97c8cd6f_d378152f","updated":"2023-10-20 16:03:05.000000000","message":"I wonder - if you try to mimic -as-gw-chassis here, then why not 5 as in line 304? Why 3? If there\u0027s justification to switch from 5 to 3, I don\u0027t think it should be piggy-backed to this patch (which is supposed to be backwards compatible - changing the number of chassis per gw port is potentially risky since it may in some situation affect the cluster by reducing redundancy.)","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":true,"context_lines":[{"line_number":304,"context_line":"MAX_GW_CHASSIS \u003d 5"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"# Maximum number of Chassis in a HA Chassis Group"},{"line_number":307,"context_line":"MAX_CHASSIS_IN_HA_GROUP \u003d 3"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"UNKNOWN_ADDR \u003d \u0027unknown\u0027"},{"line_number":310,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"295791ae_428cf9c4","line":307,"in_reply_to":"b6dd6f92_c491baee","updated":"2023-10-25 10:37:41.000000000","message":"Ah I see what you mean. There\u0027s a misunderstanding but perhaps keeping the same value as 5 is the best.\n\nBut to explain the misunderstanding, the value from MAX_GW_CHASSIS is basically the maximum number of replicas for each GW chassis scheduled across the cluster. For external ports there was no limit. If you had 10 nodes tagged with enable-chassis-as-gw we would create a HA Chassis Group with all 10 members. So it was already \"misaligned\" with that MAX_GW_CHASSIS value in the first place.\n\nThat all said, yes, perhaps keeping 5 will make things a bit easier as far as keeping things a bit more consistent than having different numbers indeed.\n\nI will change it.","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"932a2770d84c6cd8a9eb86afa6dd71d6f947ac60","unresolved":false,"context_lines":[{"line_number":304,"context_line":"MAX_GW_CHASSIS \u003d 5"},{"line_number":305,"context_line":""},{"line_number":306,"context_line":"# Maximum number of Chassis in a HA Chassis Group"},{"line_number":307,"context_line":"MAX_CHASSIS_IN_HA_GROUP \u003d 3"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"UNKNOWN_ADDR \u003d \u0027unknown\u0027"},{"line_number":310,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"a45f4744_f0a8d375","line":307,"in_reply_to":"e1b11a5c_0b3abcd5","updated":"2023-10-31 10:19:27.000000000","message":"Yeah external ports was added later. At some point we need to refactor how we schedule the GW router ports because they do not use HA Chassis Groups, they use the legacy Gateway_Chassis table in OVN. I guess when we do that we will make share more of the code path between the two.\n\nI agree keeping the same number is better.","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"}],"neutron/common/ovn/utils.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c591f83b1ab660102540ddcc71423fe03185b4ff","unresolved":true,"context_lines":[{"line_number":924,"context_line":"    port (in that network) needs to be associated with."},{"line_number":925,"context_line":""},{"line_number":926,"context_line":"    :param context: Neutron API context."},{"line_number":927,"context_line":"    :param port: The Neutron Port object or OVN Port object."},{"line_number":928,"context_line":"    :param nb_idl: OVN NB IDL"},{"line_number":929,"context_line":"    :param sb_idl: OVN SB IDL"},{"line_number":930,"context_line":"    :param txn: The ovsdbapp transaction object."}],"source_content_type":"text/x-python","patch_set":3,"id":"52fb6508_602b6ced","line":927,"updated":"2023-09-21 20:00:47.000000000","message":"I would advise not to make a shapeshifting parameter. Instead, calculate network_id and port_id outside and pass them as arguments.\n\nVague types are prone to mistakes later.","commit_id":"32238978ea8df28510601395bd889aebbfefda1b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"771f656465d79a6137310b333ff6c1280d293a98","unresolved":false,"context_lines":[{"line_number":924,"context_line":"    port (in that network) needs to be associated with."},{"line_number":925,"context_line":""},{"line_number":926,"context_line":"    :param context: Neutron API context."},{"line_number":927,"context_line":"    :param port: The Neutron Port object or OVN Port object."},{"line_number":928,"context_line":"    :param nb_idl: OVN NB IDL"},{"line_number":929,"context_line":"    :param sb_idl: OVN SB IDL"},{"line_number":930,"context_line":"    :param txn: The ovsdbapp transaction object."}],"source_content_type":"text/x-python","patch_set":3,"id":"44659cd2_ae1f9866","line":927,"in_reply_to":"4e87cedc_a7b0a233","updated":"2023-09-22 13:33:46.000000000","message":"Done","commit_id":"32238978ea8df28510601395bd889aebbfefda1b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"404e2df7e3ffc64e446b8209248b9477732e2562","unresolved":true,"context_lines":[{"line_number":924,"context_line":"    port (in that network) needs to be associated with."},{"line_number":925,"context_line":""},{"line_number":926,"context_line":"    :param context: Neutron API context."},{"line_number":927,"context_line":"    :param port: The Neutron Port object or OVN Port object."},{"line_number":928,"context_line":"    :param nb_idl: OVN NB IDL"},{"line_number":929,"context_line":"    :param sb_idl: OVN SB IDL"},{"line_number":930,"context_line":"    :param txn: The ovsdbapp transaction object."}],"source_content_type":"text/x-python","patch_set":3,"id":"4e87cedc_a7b0a233","line":927,"in_reply_to":"52fb6508_602b6ced","updated":"2023-09-22 09:38:30.000000000","message":"Sounds good, I will change the patch.","commit_id":"32238978ea8df28510601395bd889aebbfefda1b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c591f83b1ab660102540ddcc71423fe03185b4ff","unresolved":true,"context_lines":[{"line_number":932,"context_line":"                           this is the Chassis where the VM with the external"},{"line_number":933,"context_line":"                           port is bound to, so we do not schedule the"},{"line_number":934,"context_line":"                           external port onto the same Chassis"},{"line_number":935,"context_line":"    :returns: The HA Chassis Group UUID or the HA Chassis Group command object."},{"line_number":936,"context_line":"    \"\"\""},{"line_number":937,"context_line":"    max_chassis_number \u003d constants.MAX_HA_CH_GRP_CHASSIS"},{"line_number":938,"context_line":"    network_id \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"8329036e_7ba84985","line":935,"updated":"2023-09-21 20:00:47.000000000","message":"(No action needed) ... though yes, this is also bad in my mind","commit_id":"32238978ea8df28510601395bd889aebbfefda1b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"cff39ea4493063d4ac24f34c68cdc03f840f2e38","unresolved":false,"context_lines":[{"line_number":932,"context_line":"                           this is the Chassis where the VM with the external"},{"line_number":933,"context_line":"                           port is bound to, so we do not schedule the"},{"line_number":934,"context_line":"                           external port onto the same Chassis"},{"line_number":935,"context_line":"    :returns: The HA Chassis Group UUID or the HA Chassis Group command object."},{"line_number":936,"context_line":"    \"\"\""},{"line_number":937,"context_line":"    max_chassis_number \u003d constants.MAX_HA_CH_GRP_CHASSIS"},{"line_number":938,"context_line":"    network_id \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"a23ddf17_4cc372c9","line":935,"in_reply_to":"4cc982f5_b2ce802b","updated":"2023-09-22 13:33:38.000000000","message":"Done","commit_id":"32238978ea8df28510601395bd889aebbfefda1b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"404e2df7e3ffc64e446b8209248b9477732e2562","unresolved":true,"context_lines":[{"line_number":932,"context_line":"                           this is the Chassis where the VM with the external"},{"line_number":933,"context_line":"                           port is bound to, so we do not schedule the"},{"line_number":934,"context_line":"                           external port onto the same Chassis"},{"line_number":935,"context_line":"    :returns: The HA Chassis Group UUID or the HA Chassis Group command object."},{"line_number":936,"context_line":"    \"\"\""},{"line_number":937,"context_line":"    max_chassis_number \u003d constants.MAX_HA_CH_GRP_CHASSIS"},{"line_number":938,"context_line":"    network_id \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"4cc982f5_b2ce802b","line":935,"in_reply_to":"8329036e_7ba84985","updated":"2023-09-22 09:38:30.000000000","message":"Yeah, I was checking out the change that introduced this [0], apparently it was changed to allow for creating everything in the same transaction (ha chassis group and ha chassis).\n\nSo I will keep it. \n\n[0] https://review.opendev.org/c/openstack/neutron/+/872023","commit_id":"32238978ea8df28510601395bd889aebbfefda1b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c3e77b01f5a00ad2c243322f8a9e8531c94a85eb","unresolved":true,"context_lines":[{"line_number":159,"context_line":"    return constants.OVN_PROVNET_PORT_NAME_PREFIX + \u0027%s\u0027 % network_id"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"def ovn_extport_ch_grp_name(port_id):"},{"line_number":163,"context_line":"    # The name of the HA Chassis Group entry will be neutron-extport-\u003cUUID\u003e"},{"line_number":164,"context_line":"    return \u0027neutron-extport-%s\u0027 % port_id"},{"line_number":165,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"d4bb2c68_6be69e76","line":162,"updated":"2023-10-03 21:07:21.000000000","message":"(No action required) I find these abbreviations harder to read. I\u0027d prefer e.g. ovn_extport_chassis_group_name even if it\u0027s longer.","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"310fd351d3f7eb6a49b5179849247a241fa762b1","unresolved":false,"context_lines":[{"line_number":159,"context_line":"    return constants.OVN_PROVNET_PORT_NAME_PREFIX + \u0027%s\u0027 % network_id"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"def ovn_extport_ch_grp_name(port_id):"},{"line_number":163,"context_line":"    # The name of the HA Chassis Group entry will be neutron-extport-\u003cUUID\u003e"},{"line_number":164,"context_line":"    return \u0027neutron-extport-%s\u0027 % port_id"},{"line_number":165,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"308a3b2c_0bbc89d2","line":162,"in_reply_to":"d4bb2c68_6be69e76","updated":"2023-10-04 09:36:07.000000000","message":"True, I know u said no action needed but I will change it while I address the other comments","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c3e77b01f5a00ad2c243322f8a9e8531c94a85eb","unresolved":true,"context_lines":[{"line_number":939,"context_line":"    az_hints \u003d common_utils.get_az_hints("},{"line_number":940,"context_line":"        plugin.get_network(context, network_id))"},{"line_number":941,"context_line":""},{"line_number":942,"context_line":"    # If there are Chassis eligiable for hosting external ports create a HA"},{"line_number":943,"context_line":"    # Chassis Group per external port, otherwise do it at the network level"},{"line_number":944,"context_line":"    ha_ch_grp_name \u003d ovn_extport_ch_grp_name(port_id)"},{"line_number":945,"context_line":"    ch_list \u003d sb_idl.get_extport_chassis_from_cms_options()"}],"source_content_type":"text/x-python","patch_set":8,"id":"a5c8a5b4_5f59aa00","line":942,"range":{"start_line":942,"start_character":27,"end_line":942,"end_character":36},"updated":"2023-10-03 21:07:21.000000000","message":"eligible","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"310fd351d3f7eb6a49b5179849247a241fa762b1","unresolved":false,"context_lines":[{"line_number":939,"context_line":"    az_hints \u003d common_utils.get_az_hints("},{"line_number":940,"context_line":"        plugin.get_network(context, network_id))"},{"line_number":941,"context_line":""},{"line_number":942,"context_line":"    # If there are Chassis eligiable for hosting external ports create a HA"},{"line_number":943,"context_line":"    # Chassis Group per external port, otherwise do it at the network level"},{"line_number":944,"context_line":"    ha_ch_grp_name \u003d ovn_extport_ch_grp_name(port_id)"},{"line_number":945,"context_line":"    ch_list \u003d sb_idl.get_extport_chassis_from_cms_options()"}],"source_content_type":"text/x-python","patch_set":8,"id":"f6b2ea97_e6cf7814","line":942,"range":{"start_line":942,"start_character":27,"end_line":942,"end_character":36},"in_reply_to":"a5c8a5b4_5f59aa00","updated":"2023-10-04 09:36:07.000000000","message":"Done","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5e426fb86ed2325cfba3dfc6defda6fe7837f4f0","unresolved":true,"context_lines":[{"line_number":915,"context_line":"    return address4_scope_id, address6_scope_id"},{"line_number":916,"context_line":""},{"line_number":917,"context_line":""},{"line_number":918,"context_line":"def sync_ha_chassis_group(context, port_id, network_id, nb_idl, sb_idl, txn):"},{"line_number":919,"context_line":"    \"\"\"Return the UUID of the HA Chassis Group or the HA Chassis Group cmd."},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"    Given the Neutron Network ID, this method will return (or create"}],"source_content_type":"text/x-python","patch_set":9,"id":"4c7ef978_75fd90d0","line":918,"updated":"2023-10-12 17:10:08.000000000","message":"(No action required) This function became ~twice longer. I wonder if it could be split into more manageable parts.","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97145c01d2a9f24d687c5eebf247bab435cdf36","unresolved":true,"context_lines":[{"line_number":915,"context_line":"    return address4_scope_id, address6_scope_id"},{"line_number":916,"context_line":""},{"line_number":917,"context_line":""},{"line_number":918,"context_line":"def sync_ha_chassis_group(context, port_id, network_id, nb_idl, sb_idl, txn):"},{"line_number":919,"context_line":"    \"\"\"Return the UUID of the HA Chassis Group or the HA Chassis Group cmd."},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"    Given the Neutron Network ID, this method will return (or create"}],"source_content_type":"text/x-python","patch_set":9,"id":"beb7d0f2_c2508a68","line":918,"in_reply_to":"4c7ef978_75fd90d0","updated":"2023-10-13 09:38:41.000000000","message":"I will look into it","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"b9b9f5d7ca916566f8cb720e2d7732369c484f5e","unresolved":true,"context_lines":[{"line_number":915,"context_line":"    return address4_scope_id, address6_scope_id"},{"line_number":916,"context_line":""},{"line_number":917,"context_line":""},{"line_number":918,"context_line":"def sync_ha_chassis_group(context, port_id, network_id, nb_idl, sb_idl, txn):"},{"line_number":919,"context_line":"    \"\"\"Return the UUID of the HA Chassis Group or the HA Chassis Group cmd."},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"    Given the Neutron Network ID, this method will return (or create"}],"source_content_type":"text/x-python","patch_set":9,"id":"cb1a44d0_faa73eb6","line":918,"in_reply_to":"beb7d0f2_c2508a68","updated":"2023-10-13 12:31:17.000000000","message":"The function is now split in 3:\n\n1) _get_info_for_ha_chassis_group, responsible for getting the information needed to create the HA Chassis Group. Here is where the differences of external port or network ha chassis group is sorted\n\n2) _filter_candidates_for_ha_chassis_group, this is where we filter the candidates for the ha chassis group, taking in consideration AZs and ignored chassis\n\n3) sync_ha_chassis_group, this method only have the common code to create the ha chassis group that is used for both external ports and network cases","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":false,"context_lines":[{"line_number":915,"context_line":"    return address4_scope_id, address6_scope_id"},{"line_number":916,"context_line":""},{"line_number":917,"context_line":""},{"line_number":918,"context_line":"def sync_ha_chassis_group(context, port_id, network_id, nb_idl, sb_idl, txn):"},{"line_number":919,"context_line":"    \"\"\"Return the UUID of the HA Chassis Group or the HA Chassis Group cmd."},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"    Given the Neutron Network ID, this method will return (or create"}],"source_content_type":"text/x-python","patch_set":9,"id":"015c7641_f4507d4c","line":918,"in_reply_to":"cb1a44d0_faa73eb6","updated":"2023-10-20 16:03:05.000000000","message":"Done","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5e426fb86ed2325cfba3dfc6defda6fe7837f4f0","unresolved":true,"context_lines":[{"line_number":973,"context_line":""},{"line_number":974,"context_line":"    # Remove the ignored Chassis, if present"},{"line_number":975,"context_line":"    try:"},{"line_number":976,"context_line":"        az_chassis.remove(ignore_chassis)"},{"line_number":977,"context_line":"        LOG.debug(\u0027Ignoring chassis %s for HA Chassis Group %s\u0027,"},{"line_number":978,"context_line":"                  ignore_chassis, ha_ch_grp_name)"},{"line_number":979,"context_line":"    except KeyError:"}],"source_content_type":"text/x-python","patch_set":9,"id":"0007256c_b61b8496","line":976,"updated":"2023-10-12 17:10:08.000000000","message":"I think it would be better to drop the ignore_chassis from the list immediately in line 958+ where it was identified as a chassis to skip. (It\u0027s easier to follow the code then, in my view.)","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97145c01d2a9f24d687c5eebf247bab435cdf36","unresolved":false,"context_lines":[{"line_number":973,"context_line":""},{"line_number":974,"context_line":"    # Remove the ignored Chassis, if present"},{"line_number":975,"context_line":"    try:"},{"line_number":976,"context_line":"        az_chassis.remove(ignore_chassis)"},{"line_number":977,"context_line":"        LOG.debug(\u0027Ignoring chassis %s for HA Chassis Group %s\u0027,"},{"line_number":978,"context_line":"                  ignore_chassis, ha_ch_grp_name)"},{"line_number":979,"context_line":"    except KeyError:"}],"source_content_type":"text/x-python","patch_set":9,"id":"ae280888_db093c99","line":976,"in_reply_to":"0007256c_b61b8496","updated":"2023-10-13 09:38:41.000000000","message":"Done","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5e426fb86ed2325cfba3dfc6defda6fe7837f4f0","unresolved":true,"context_lines":[{"line_number":985,"context_line":"            ha_ch_grp_name).execute(check_error\u003dTrue)"},{"line_number":986,"context_line":"    except idlutils.RowNotFound:"},{"line_number":987,"context_line":"        ext_ids \u003d {constants.OVN_AZ_HINTS_EXT_ID_KEY: \u0027,\u0027.join(az_hints)}"},{"line_number":988,"context_line":"        ha_ch_grp_cmd \u003d txn.add(nb_idl.ha_chassis_group_add("},{"line_number":989,"context_line":"            ha_ch_grp_name, may_exist\u003dTrue, external_ids\u003dext_ids))"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    max_chassis_number \u003d constants.MAX_CHASSIS_IN_HA_GROUP"}],"source_content_type":"text/x-python","patch_set":9,"id":"6b63810a_c2b67db2","line":988,"updated":"2023-10-12 17:10:08.000000000","message":"Can\u0027t we just unconditionally add the group to the transaction (with may_exist\u003dTrue), without _get?","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"6b775ee885ef1f6b850a2086a663e5c0e21da19d","unresolved":false,"context_lines":[{"line_number":985,"context_line":"            ha_ch_grp_name).execute(check_error\u003dTrue)"},{"line_number":986,"context_line":"    except idlutils.RowNotFound:"},{"line_number":987,"context_line":"        ext_ids \u003d {constants.OVN_AZ_HINTS_EXT_ID_KEY: \u0027,\u0027.join(az_hints)}"},{"line_number":988,"context_line":"        ha_ch_grp_cmd \u003d txn.add(nb_idl.ha_chassis_group_add("},{"line_number":989,"context_line":"            ha_ch_grp_name, may_exist\u003dTrue, external_ids\u003dext_ids))"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    max_chassis_number \u003d constants.MAX_CHASSIS_IN_HA_GROUP"}],"source_content_type":"text/x-python","patch_set":9,"id":"6f4f4357_806b7c32","line":988,"in_reply_to":"47436243_9d38b830","updated":"2023-10-25 14:06:52.000000000","message":"Done","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97145c01d2a9f24d687c5eebf247bab435cdf36","unresolved":true,"context_lines":[{"line_number":985,"context_line":"            ha_ch_grp_name).execute(check_error\u003dTrue)"},{"line_number":986,"context_line":"    except idlutils.RowNotFound:"},{"line_number":987,"context_line":"        ext_ids \u003d {constants.OVN_AZ_HINTS_EXT_ID_KEY: \u0027,\u0027.join(az_hints)}"},{"line_number":988,"context_line":"        ha_ch_grp_cmd \u003d txn.add(nb_idl.ha_chassis_group_add("},{"line_number":989,"context_line":"            ha_ch_grp_name, may_exist\u003dTrue, external_ids\u003dext_ids))"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    max_chassis_number \u003d constants.MAX_CHASSIS_IN_HA_GROUP"}],"source_content_type":"text/x-python","patch_set":9,"id":"47436243_9d38b830","line":988,"in_reply_to":"6b63810a_c2b67db2","updated":"2023-10-13 09:38:41.000000000","message":"It was like what you suggested before see L927-L935 from the left side of the diff.\n\nBut, then we had to check for the return type to see if was a RowView or None... and based on that we knew whether this group existed before or not. IMHO, it was more confusing and in some testing apparently it wasn\u0027t working as expected.\n\nSo I changed it to try to get the group and if it doesn\u0027t exist, create it. At least to me this logic seems more straight forward.","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5e426fb86ed2325cfba3dfc6defda6fe7837f4f0","unresolved":true,"context_lines":[{"line_number":988,"context_line":"        ha_ch_grp_cmd \u003d txn.add(nb_idl.ha_chassis_group_add("},{"line_number":989,"context_line":"            ha_ch_grp_name, may_exist\u003dTrue, external_ids\u003dext_ids))"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    max_chassis_number \u003d constants.MAX_CHASSIS_IN_HA_GROUP"},{"line_number":992,"context_line":"    priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"},{"line_number":993,"context_line":"    if ha_ch_grp:"},{"line_number":994,"context_line":"        # Remove any chassis that no longer belongs to the AZ hints"}],"source_content_type":"text/x-python","patch_set":9,"id":"5aa2cbd7_9dccbdda","line":991,"updated":"2023-10-12 17:10:08.000000000","message":"AFAIU you are applying the new limit to both -as-gw and -extport-host modes of operation. Is it your intent? If so, 1) why? and 2) should it be documented in release note?","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"6b775ee885ef1f6b850a2086a663e5c0e21da19d","unresolved":false,"context_lines":[{"line_number":988,"context_line":"        ha_ch_grp_cmd \u003d txn.add(nb_idl.ha_chassis_group_add("},{"line_number":989,"context_line":"            ha_ch_grp_name, may_exist\u003dTrue, external_ids\u003dext_ids))"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    max_chassis_number \u003d constants.MAX_CHASSIS_IN_HA_GROUP"},{"line_number":992,"context_line":"    priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"},{"line_number":993,"context_line":"    if ha_ch_grp:"},{"line_number":994,"context_line":"        # Remove any chassis that no longer belongs to the AZ hints"}],"source_content_type":"text/x-python","patch_set":9,"id":"f1c08f80_459c4b1d","line":991,"in_reply_to":"2b87feef_23e9399a","updated":"2023-10-25 14:06:52.000000000","message":"Done","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":988,"context_line":"        ha_ch_grp_cmd \u003d txn.add(nb_idl.ha_chassis_group_add("},{"line_number":989,"context_line":"            ha_ch_grp_name, may_exist\u003dTrue, external_ids\u003dext_ids))"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    max_chassis_number \u003d constants.MAX_CHASSIS_IN_HA_GROUP"},{"line_number":992,"context_line":"    priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"},{"line_number":993,"context_line":"    if ha_ch_grp:"},{"line_number":994,"context_line":"        # Remove any chassis that no longer belongs to the AZ hints"}],"source_content_type":"text/x-python","patch_set":9,"id":"75f9832b_661977d9","line":991,"in_reply_to":"39235209_1b8e7643","updated":"2023-10-20 16:03:05.000000000","message":"That you limit the number is not my concern. That you change the limit value is. (At least that you do it as part of this patch, that ideally won\u0027t affect behavior for -as-gw.) If you were to choose \"5\" as the limit, there wouldn\u0027t be a change in behavior. And if there\u0027s a justification to switch from 5 to 3, then let\u0027s have this discussion elsewhere, not in this patch.","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97145c01d2a9f24d687c5eebf247bab435cdf36","unresolved":true,"context_lines":[{"line_number":988,"context_line":"        ha_ch_grp_cmd \u003d txn.add(nb_idl.ha_chassis_group_add("},{"line_number":989,"context_line":"            ha_ch_grp_name, may_exist\u003dTrue, external_ids\u003dext_ids))"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    max_chassis_number \u003d constants.MAX_CHASSIS_IN_HA_GROUP"},{"line_number":992,"context_line":"    priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"},{"line_number":993,"context_line":"    if ha_ch_grp:"},{"line_number":994,"context_line":"        # Remove any chassis that no longer belongs to the AZ hints"}],"source_content_type":"text/x-python","patch_set":9,"id":"39235209_1b8e7643","line":991,"in_reply_to":"5aa2cbd7_9dccbdda","updated":"2023-10-13 09:38:41.000000000","message":"Yes it was intentional, I\u0027m limiting the number because I want to limit the BFD mash for each group (as stated in a previous comment in the constants.py file)\n\nAs for limiting both cases: The code is simpler if we do not discriminate between each case, so there\u0027s always 3 per group and in the case of the existing -as-gw, these were controller nodes which in most deployment cases the number is already 3.\n\nI will update the release note. Let me know if this rationale makes sense for you too.","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":true,"context_lines":[{"line_number":988,"context_line":"        ha_ch_grp_cmd \u003d txn.add(nb_idl.ha_chassis_group_add("},{"line_number":989,"context_line":"            ha_ch_grp_name, may_exist\u003dTrue, external_ids\u003dext_ids))"},{"line_number":990,"context_line":""},{"line_number":991,"context_line":"    max_chassis_number \u003d constants.MAX_CHASSIS_IN_HA_GROUP"},{"line_number":992,"context_line":"    priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"},{"line_number":993,"context_line":"    if ha_ch_grp:"},{"line_number":994,"context_line":"        # Remove any chassis that no longer belongs to the AZ hints"}],"source_content_type":"text/x-python","patch_set":9,"id":"2b87feef_23e9399a","line":991,"in_reply_to":"75f9832b_661977d9","updated":"2023-10-25 10:37:41.000000000","message":"The 5 never applied to this code. The 5 only applied to gw router ports and not external ports. Before external ports number limit was based on the number of chassis marked with the \"enable-chassis-as-gw\" configuration, if 10 chassis were tagged with this configuration the HA Chassis Group for the external ports would have 10 members.","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5e426fb86ed2325cfba3dfc6defda6fe7837f4f0","unresolved":true,"context_lines":[{"line_number":1008,"context_line":"        priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"},{"line_number":1009,"context_line":"        if (high_prio_ch and"},{"line_number":1010,"context_line":"                high_prio_ch.chassis_name in az_chassis and"},{"line_number":1011,"context_line":"                high_prio_ch.chassis_name !\u003d ignore_chassis):"},{"line_number":1012,"context_line":"            txn.add(nb_idl.ha_chassis_group_add_chassis("},{"line_number":1013,"context_line":"                ha_ch_grp_name, high_prio_ch.chassis_name,"},{"line_number":1014,"context_line":"                priority\u003dpriority))"}],"source_content_type":"text/x-python","patch_set":9,"id":"701373da_6a1299f3","line":1011,"updated":"2023-10-12 17:10:08.000000000","message":"I think `az_chassis` at this point doesn\u0027t contain `ignore_chassis` (we removed it in line 976), so this additional `!\u003d` condition seems to be a no-op. Am I missing something?\n\n(btw if this is indeed redundant and you adopt my suggestion from line 976, then you can also get rid of `ignore_chassis` variable.)","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"932a2770d84c6cd8a9eb86afa6dd71d6f947ac60","unresolved":false,"context_lines":[{"line_number":1008,"context_line":"        priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"},{"line_number":1009,"context_line":"        if (high_prio_ch and"},{"line_number":1010,"context_line":"                high_prio_ch.chassis_name in az_chassis and"},{"line_number":1011,"context_line":"                high_prio_ch.chassis_name !\u003d ignore_chassis):"},{"line_number":1012,"context_line":"            txn.add(nb_idl.ha_chassis_group_add_chassis("},{"line_number":1013,"context_line":"                ha_ch_grp_name, high_prio_ch.chassis_name,"},{"line_number":1014,"context_line":"                priority\u003dpriority))"}],"source_content_type":"text/x-python","patch_set":9,"id":"e6c215a0_a4f61adf","line":1011,"in_reply_to":"040b5a1d_9da51973","updated":"2023-10-31 10:19:27.000000000","message":"Done","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97145c01d2a9f24d687c5eebf247bab435cdf36","unresolved":true,"context_lines":[{"line_number":1008,"context_line":"        priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"},{"line_number":1009,"context_line":"        if (high_prio_ch and"},{"line_number":1010,"context_line":"                high_prio_ch.chassis_name in az_chassis and"},{"line_number":1011,"context_line":"                high_prio_ch.chassis_name !\u003d ignore_chassis):"},{"line_number":1012,"context_line":"            txn.add(nb_idl.ha_chassis_group_add_chassis("},{"line_number":1013,"context_line":"                ha_ch_grp_name, high_prio_ch.chassis_name,"},{"line_number":1014,"context_line":"                priority\u003dpriority))"}],"source_content_type":"text/x-python","patch_set":9,"id":"040b5a1d_9da51973","line":1011,"in_reply_to":"701373da_6a1299f3","updated":"2023-10-13 09:38:41.000000000","message":"In this case we need to keep the ignore_chassis because it\u0027s listing the chassis in an HA Chassis Group that already exist (and not the ch_list from ~L950), so if the VM moved to another compute there\u0027s a possibility that it moved to a compute that is present in this group, therefore we should check for it.\n\nJust as a context, the idea of finding the highest priority chassis in the group and keep it as the highest priority during update is to prevent the port from moving around and causing disruption.","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5e426fb86ed2325cfba3dfc6defda6fe7837f4f0","unresolved":true,"context_lines":[{"line_number":1021,"context_line":""},{"line_number":1022,"context_line":"    # random.sample() second parameter needs to be \u003c\u003d the list size,"},{"line_number":1023,"context_line":"    # that\u0027s why we need to check for the max value here"},{"line_number":1024,"context_line":"    max_chassis_number \u003d (max_chassis_number if max_chassis_number \u003c"},{"line_number":1025,"context_line":"                          len(az_chassis) else len(az_chassis))"},{"line_number":1026,"context_line":"    # Randomize the order so that networks belonging to the same"},{"line_number":1027,"context_line":"    # availability zones do not necessarily end up with the same"},{"line_number":1028,"context_line":"    # Chassis as the highest priority one."}],"source_content_type":"text/x-python","patch_set":9,"id":"9a63bfc3_583e80e3","line":1025,"range":{"start_line":1024,"start_character":25,"end_line":1025,"end_character":63},"updated":"2023-10-12 17:10:08.000000000","message":"`min(max_chassis_number, len(az_chassis))`?","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97145c01d2a9f24d687c5eebf247bab435cdf36","unresolved":false,"context_lines":[{"line_number":1021,"context_line":""},{"line_number":1022,"context_line":"    # random.sample() second parameter needs to be \u003c\u003d the list size,"},{"line_number":1023,"context_line":"    # that\u0027s why we need to check for the max value here"},{"line_number":1024,"context_line":"    max_chassis_number \u003d (max_chassis_number if max_chassis_number \u003c"},{"line_number":1025,"context_line":"                          len(az_chassis) else len(az_chassis))"},{"line_number":1026,"context_line":"    # Randomize the order so that networks belonging to the same"},{"line_number":1027,"context_line":"    # availability zones do not necessarily end up with the same"},{"line_number":1028,"context_line":"    # Chassis as the highest priority one."}],"source_content_type":"text/x-python","patch_set":9,"id":"1096bb39_cdff8839","line":1025,"range":{"start_line":1024,"start_character":25,"end_line":1025,"end_character":63},"in_reply_to":"9a63bfc3_583e80e3","updated":"2023-10-13 09:38:41.000000000","message":"Done","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5e426fb86ed2325cfba3dfc6defda6fe7837f4f0","unresolved":true,"context_lines":[{"line_number":1023,"context_line":"    # that\u0027s why we need to check for the max value here"},{"line_number":1024,"context_line":"    max_chassis_number \u003d (max_chassis_number if max_chassis_number \u003c"},{"line_number":1025,"context_line":"                          len(az_chassis) else len(az_chassis))"},{"line_number":1026,"context_line":"    # Randomize the order so that networks belonging to the same"},{"line_number":1027,"context_line":"    # availability zones do not necessarily end up with the same"},{"line_number":1028,"context_line":"    # Chassis as the highest priority one."},{"line_number":1029,"context_line":"    for ch in random.sample(list(az_chassis), max_chassis_number):"}],"source_content_type":"text/x-python","patch_set":9,"id":"c9324019_831d4083","line":1026,"updated":"2023-10-12 17:10:08.000000000","message":"This comment is now not entirely correct. Yes, it randomizes but also - it limits the number of entries in the group.","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97145c01d2a9f24d687c5eebf247bab435cdf36","unresolved":false,"context_lines":[{"line_number":1023,"context_line":"    # that\u0027s why we need to check for the max value here"},{"line_number":1024,"context_line":"    max_chassis_number \u003d (max_chassis_number if max_chassis_number \u003c"},{"line_number":1025,"context_line":"                          len(az_chassis) else len(az_chassis))"},{"line_number":1026,"context_line":"    # Randomize the order so that networks belonging to the same"},{"line_number":1027,"context_line":"    # availability zones do not necessarily end up with the same"},{"line_number":1028,"context_line":"    # Chassis as the highest priority one."},{"line_number":1029,"context_line":"    for ch in random.sample(list(az_chassis), max_chassis_number):"}],"source_content_type":"text/x-python","patch_set":9,"id":"5e4467e6_5dd9e5a1","line":1026,"in_reply_to":"c9324019_831d4083","updated":"2023-10-13 09:38:41.000000000","message":"Good point, done","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":681,"context_line":""},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"def is_extport_host_chassis(chassis):"},{"line_number":684,"context_line":"    \"\"\"Check if the given Chassis is eligible to host external ports\"\"\""},{"line_number":685,"context_line":"    return (constants.CMS_OPT_CHASSIS_AS_EXTPORT_HOST in"},{"line_number":686,"context_line":"            get_ovn_cms_options(chassis))"},{"line_number":687,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"84182059_e193a50f","line":684,"range":{"start_line":684,"start_character":34,"end_line":684,"end_character":68},"updated":"2023-10-20 16:03:05.000000000","message":"This comment is slightly misleading.\n\nThe way the code is implemented, a chassis is eligible to host external ports when:\n\n1. as-extport-host is explicitly set in its ovsdb config\n2. as-gw is explicitly set in its ovsdb config, and no other chassis has as-extport-host set\n3. if not other chassis has either as-extport-host or as-gw set\n\nWhat this function tests is not whether the chassis is \"eligible to host\" but whether it\u0027s explicitly marked as one of the hosts to host external ports. Which is different.","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":false,"context_lines":[{"line_number":681,"context_line":""},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"def is_extport_host_chassis(chassis):"},{"line_number":684,"context_line":"    \"\"\"Check if the given Chassis is eligible to host external ports\"\"\""},{"line_number":685,"context_line":"    return (constants.CMS_OPT_CHASSIS_AS_EXTPORT_HOST in"},{"line_number":686,"context_line":"            get_ovn_cms_options(chassis))"},{"line_number":687,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"5722190d_49b3fc4e","line":684,"range":{"start_line":684,"start_character":34,"end_line":684,"end_character":68},"in_reply_to":"84182059_e193a50f","updated":"2023-10-25 10:37:41.000000000","message":"True, needs to be explicitly marked.\n\nThe option 3. is not an option for external ports. If no nodes are marked we won\u0027t schedule it. Perhaps even a LOG about that would be useful. I will add.","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":928,"context_line":"    :param sb_idl:     OVN SB IDL"},{"line_number":929,"context_line":"    :returns:          An instance of HAChassisGroupInfo"},{"line_number":930,"context_line":"    \"\"\""},{"line_number":931,"context_line":"    # If there are Chassis eligible for hosting external ports create a HA"},{"line_number":932,"context_line":"    # Chassis Group per external port, otherwise do it at the network level"},{"line_number":933,"context_line":"    group_name \u003d ovn_extport_chassis_group_name(port_id)"},{"line_number":934,"context_line":"    chassis_list \u003d sb_idl.get_extport_chassis_from_cms_options()"}],"source_content_type":"text/x-python","patch_set":10,"id":"1f8ba4a6_5a7919bf","line":931,"range":{"start_line":931,"start_character":27,"end_line":931,"end_character":35},"updated":"2023-10-20 16:03:05.000000000","message":"-\u003e explicitly marked / selected to host external ports... ?","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":false,"context_lines":[{"line_number":928,"context_line":"    :param sb_idl:     OVN SB IDL"},{"line_number":929,"context_line":"    :returns:          An instance of HAChassisGroupInfo"},{"line_number":930,"context_line":"    \"\"\""},{"line_number":931,"context_line":"    # If there are Chassis eligible for hosting external ports create a HA"},{"line_number":932,"context_line":"    # Chassis Group per external port, otherwise do it at the network level"},{"line_number":933,"context_line":"    group_name \u003d ovn_extport_chassis_group_name(port_id)"},{"line_number":934,"context_line":"    chassis_list \u003d sb_idl.get_extport_chassis_from_cms_options()"}],"source_content_type":"text/x-python","patch_set":10,"id":"79d3b26f_433d2284","line":931,"range":{"start_line":931,"start_character":27,"end_line":931,"end_character":35},"in_reply_to":"1f8ba4a6_5a7919bf","updated":"2023-10-25 10:37:41.000000000","message":"++","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":930,"context_line":"    \"\"\""},{"line_number":931,"context_line":"    # If there are Chassis eligible for hosting external ports create a HA"},{"line_number":932,"context_line":"    # Chassis Group per external port, otherwise do it at the network level"},{"line_number":933,"context_line":"    group_name \u003d ovn_extport_chassis_group_name(port_id)"},{"line_number":934,"context_line":"    chassis_list \u003d sb_idl.get_extport_chassis_from_cms_options()"},{"line_number":935,"context_line":"    extport_host_present \u003d True"},{"line_number":936,"context_line":"    if not chassis_list:"}],"source_content_type":"text/x-python","patch_set":10,"id":"2cbadcf5_f13454f2","line":933,"updated":"2023-10-20 16:03:05.000000000","message":"nit: you could avoid calling it here if you construct lines 933-940 as if-else pair of branches. (This would also avoid double assignment for extport_host_present.)\n\n```\nchassis_list \u003d ...\nif chassis_list:\n  group_name \u003d ...\n  extport_host_present \u003d ...\nelse:\n  chassis_list \u003d ...\n  group_name \u003d ...\n  extport_host_present \u003d ...\n```","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":false,"context_lines":[{"line_number":930,"context_line":"    \"\"\""},{"line_number":931,"context_line":"    # If there are Chassis eligible for hosting external ports create a HA"},{"line_number":932,"context_line":"    # Chassis Group per external port, otherwise do it at the network level"},{"line_number":933,"context_line":"    group_name \u003d ovn_extport_chassis_group_name(port_id)"},{"line_number":934,"context_line":"    chassis_list \u003d sb_idl.get_extport_chassis_from_cms_options()"},{"line_number":935,"context_line":"    extport_host_present \u003d True"},{"line_number":936,"context_line":"    if not chassis_list:"}],"source_content_type":"text/x-python","patch_set":10,"id":"b9bf3695_67e397cd","line":933,"in_reply_to":"2cbadcf5_f13454f2","updated":"2023-10-25 10:37:41.000000000","message":"Good point, will change","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":944,"context_line":"        # Check if the port is bound to a chassis and if so, ignore that"},{"line_number":945,"context_line":"        # chassis when building the HA Chassis Group to ensure the"},{"line_number":946,"context_line":"        # external port is bound to a different chassis than the VM"},{"line_number":947,"context_line":"        chassis_host \u003d sb_idl.get_chassis_host_for_port(port_id)"},{"line_number":948,"context_line":"        if chassis_host:"},{"line_number":949,"context_line":"            ignore_chassis \u003d chassis_host[0].name"},{"line_number":950,"context_line":"        LOG.debug(\u0027HA Chassis Group %s is based on external port %s \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"d4bca419_3a94711d","line":947,"updated":"2023-10-20 16:03:05.000000000","message":"A LSP may be bound to multiple chassis (see my comment in idl module for details.) You may have to convert `ignore_chassis` into an iterable. (Perhaps also rename it into e.g. `chassis_blacklist` while at it?)","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":false,"context_lines":[{"line_number":944,"context_line":"        # Check if the port is bound to a chassis and if so, ignore that"},{"line_number":945,"context_line":"        # chassis when building the HA Chassis Group to ensure the"},{"line_number":946,"context_line":"        # external port is bound to a different chassis than the VM"},{"line_number":947,"context_line":"        chassis_host \u003d sb_idl.get_chassis_host_for_port(port_id)"},{"line_number":948,"context_line":"        if chassis_host:"},{"line_number":949,"context_line":"            ignore_chassis \u003d chassis_host[0].name"},{"line_number":950,"context_line":"        LOG.debug(\u0027HA Chassis Group %s is based on external port %s \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"6f0a99e7_46539a16","line":947,"in_reply_to":"d4bca419_3a94711d","updated":"2023-10-25 10:37:41.000000000","message":"Ah, fair point! Will do","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":969,"context_line":"    Filter a list of chassis candidates for a given HA Chassis Group taking"},{"line_number":970,"context_line":"    in consideration availability zones if present."},{"line_number":971,"context_line":""},{"line_number":972,"context_line":"    :param hcg_info: A instance of HAChassisGroupInfo"},{"line_number":973,"context_line":"    \"\"\""},{"line_number":974,"context_line":"    candidates \u003d set()"},{"line_number":975,"context_line":"    if not hcg_info.az_hints:"}],"source_content_type":"text/x-python","patch_set":10,"id":"439a9840_fc60d232","line":972,"updated":"2023-10-20 16:03:05.000000000","message":"returns?","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":false,"context_lines":[{"line_number":969,"context_line":"    Filter a list of chassis candidates for a given HA Chassis Group taking"},{"line_number":970,"context_line":"    in consideration availability zones if present."},{"line_number":971,"context_line":""},{"line_number":972,"context_line":"    :param hcg_info: A instance of HAChassisGroupInfo"},{"line_number":973,"context_line":"    \"\"\""},{"line_number":974,"context_line":"    candidates \u003d set()"},{"line_number":975,"context_line":"    if not hcg_info.az_hints:"}],"source_content_type":"text/x-python","patch_set":10,"id":"fddf5a02_5f0b1a63","line":972,"in_reply_to":"439a9840_fc60d232","updated":"2023-10-25 10:37:41.000000000","message":"Adding","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":971,"context_line":""},{"line_number":972,"context_line":"    :param hcg_info: A instance of HAChassisGroupInfo"},{"line_number":973,"context_line":"    \"\"\""},{"line_number":974,"context_line":"    candidates \u003d set()"},{"line_number":975,"context_line":"    if not hcg_info.az_hints:"},{"line_number":976,"context_line":"        candidates \u003d get_chassis_without_azs(hcg_info.chassis_list)"},{"line_number":977,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":10,"id":"96d30edf_4df0a692","line":974,"updated":"2023-10-20 16:03:05.000000000","message":"nit: redundant line","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":false,"context_lines":[{"line_number":971,"context_line":""},{"line_number":972,"context_line":"    :param hcg_info: A instance of HAChassisGroupInfo"},{"line_number":973,"context_line":"    \"\"\""},{"line_number":974,"context_line":"    candidates \u003d set()"},{"line_number":975,"context_line":"    if not hcg_info.az_hints:"},{"line_number":976,"context_line":"        candidates \u003d get_chassis_without_azs(hcg_info.chassis_list)"},{"line_number":977,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":10,"id":"42dff116_b3dff468","line":974,"in_reply_to":"96d30edf_4df0a692","updated":"2023-10-25 10:37:41.000000000","message":"Done","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":972,"context_line":"    :param hcg_info: A instance of HAChassisGroupInfo"},{"line_number":973,"context_line":"    \"\"\""},{"line_number":974,"context_line":"    candidates \u003d set()"},{"line_number":975,"context_line":"    if not hcg_info.az_hints:"},{"line_number":976,"context_line":"        candidates \u003d get_chassis_without_azs(hcg_info.chassis_list)"},{"line_number":977,"context_line":"    else:"},{"line_number":978,"context_line":"        candidates \u003d get_chassis_in_azs(hcg_info.chassis_list,"}],"source_content_type":"text/x-python","patch_set":10,"id":"e0222984_f919b97e","line":975,"updated":"2023-10-20 16:03:05.000000000","message":"nit: I find `if not X` as the first conditional in `if-else` pair slightly less intelligible than `if X`.","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":false,"context_lines":[{"line_number":972,"context_line":"    :param hcg_info: A instance of HAChassisGroupInfo"},{"line_number":973,"context_line":"    \"\"\""},{"line_number":974,"context_line":"    candidates \u003d set()"},{"line_number":975,"context_line":"    if not hcg_info.az_hints:"},{"line_number":976,"context_line":"        candidates \u003d get_chassis_without_azs(hcg_info.chassis_list)"},{"line_number":977,"context_line":"    else:"},{"line_number":978,"context_line":"        candidates \u003d get_chassis_in_azs(hcg_info.chassis_list,"}],"source_content_type":"text/x-python","patch_set":10,"id":"1ed3b0d6_79e79df7","line":975,"in_reply_to":"e0222984_f919b97e","updated":"2023-10-25 10:37:41.000000000","message":"hehe will do it the other way around","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":986,"context_line":"        candidates.remove(hcg_info.ignore_chassis)"},{"line_number":987,"context_line":"        LOG.debug(\u0027Ignoring chassis %s for HA Chassis Group %s\u0027,"},{"line_number":988,"context_line":"                  hcg_info.ignore_chassis, hcg_info.group_name)"},{"line_number":989,"context_line":"    except KeyError:"},{"line_number":990,"context_line":"        pass"},{"line_number":991,"context_line":""},{"line_number":992,"context_line":"    return candidates"}],"source_content_type":"text/x-python","patch_set":10,"id":"655f2e0d_e36dbad0","line":989,"updated":"2023-10-20 16:03:05.000000000","message":"btw you could set().discard() instead because it handles the case of missing entry in the set.","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":false,"context_lines":[{"line_number":986,"context_line":"        candidates.remove(hcg_info.ignore_chassis)"},{"line_number":987,"context_line":"        LOG.debug(\u0027Ignoring chassis %s for HA Chassis Group %s\u0027,"},{"line_number":988,"context_line":"                  hcg_info.ignore_chassis, hcg_info.group_name)"},{"line_number":989,"context_line":"    except KeyError:"},{"line_number":990,"context_line":"        pass"},{"line_number":991,"context_line":""},{"line_number":992,"context_line":"    return candidates"}],"source_content_type":"text/x-python","patch_set":10,"id":"3eb789eb_cc385fe4","line":989,"in_reply_to":"655f2e0d_e36dbad0","updated":"2023-10-25 10:37:41.000000000","message":"Done","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":1007,"context_line":"    :param txn: The ovsdbapp transaction object"},{"line_number":1008,"context_line":"    :returns: The HA Chassis Group UUID or the HA Chassis Group command object"},{"line_number":1009,"context_line":"    \"\"\""},{"line_number":1010,"context_line":"    # If there are Chassis eligible for hosting external ports create a HA"},{"line_number":1011,"context_line":"    # Chassis Group per external port, otherwise do it at the network level"},{"line_number":1012,"context_line":"    hcg_info \u003d _get_info_for_ha_chassis_group(context, port_id, network_id,"},{"line_number":1013,"context_line":"                                              sb_idl)"}],"source_content_type":"text/x-python","patch_set":10,"id":"e954619a_297ad4c7","line":1010,"range":{"start_line":1010,"start_character":27,"end_line":1010,"end_character":35},"updated":"2023-10-20 16:03:05.000000000","message":"-\u003e explicitly marked to host... ? that said, this comment is duplicate and can probably be dropped since it explains mechanics of get_info.","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":false,"context_lines":[{"line_number":1007,"context_line":"    :param txn: The ovsdbapp transaction object"},{"line_number":1008,"context_line":"    :returns: The HA Chassis Group UUID or the HA Chassis Group command object"},{"line_number":1009,"context_line":"    \"\"\""},{"line_number":1010,"context_line":"    # If there are Chassis eligible for hosting external ports create a HA"},{"line_number":1011,"context_line":"    # Chassis Group per external port, otherwise do it at the network level"},{"line_number":1012,"context_line":"    hcg_info \u003d _get_info_for_ha_chassis_group(context, port_id, network_id,"},{"line_number":1013,"context_line":"                                              sb_idl)"}],"source_content_type":"text/x-python","patch_set":10,"id":"7783510d_cd958133","line":1010,"range":{"start_line":1010,"start_character":27,"end_line":1010,"end_character":35},"in_reply_to":"e954619a_297ad4c7","updated":"2023-10-25 10:37:41.000000000","message":"Done","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":1022,"context_line":"        ext_ids \u003d {constants.OVN_AZ_HINTS_EXT_ID_KEY: \u0027,\u0027.join("},{"line_number":1023,"context_line":"            hcg_info.az_hints)}"},{"line_number":1024,"context_line":"        ha_ch_grp_cmd \u003d txn.add(nb_idl.ha_chassis_group_add("},{"line_number":1025,"context_line":"            hcg_info.group_name, may_exist\u003dTrue, external_ids\u003dext_ids))"},{"line_number":1026,"context_line":""},{"line_number":1027,"context_line":"    max_chassis_number \u003d constants.MAX_CHASSIS_IN_HA_GROUP"},{"line_number":1028,"context_line":"    priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"}],"source_content_type":"text/x-python","patch_set":10,"id":"d72fb662_53a1c28b","line":1025,"range":{"start_line":1025,"start_character":33,"end_line":1025,"end_character":47},"updated":"2023-10-20 16:03:05.000000000","message":"Do I understand it correctly that this means that `ha_ch_grp_cmd` may - in an unfortunate situation - be a `rowview.RowView` here? (In case when another worker just created the group between lines 1021 and 1024) In this case, `ha_ch_grp` will still be `None`, and in line 1072, we\u0027ll then return `RowView`, despite what line 1008 promises.\n\n(The code would then also not adjust the membership for the HA group, as implemented in lines 1030+.)\n\nI think this race scenario is now possible because of the change in the approach to fetch-or-create the group, namely - now you get, then create; but before, the code was get-but-create-if-missing (as an atomic step).\n\nThoughts?","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebdf6ec6d615ea07dce6b5e0256f4dd3660ab78d","unresolved":false,"context_lines":[{"line_number":1022,"context_line":"        ext_ids \u003d {constants.OVN_AZ_HINTS_EXT_ID_KEY: \u0027,\u0027.join("},{"line_number":1023,"context_line":"            hcg_info.az_hints)}"},{"line_number":1024,"context_line":"        ha_ch_grp_cmd \u003d txn.add(nb_idl.ha_chassis_group_add("},{"line_number":1025,"context_line":"            hcg_info.group_name, may_exist\u003dTrue, external_ids\u003dext_ids))"},{"line_number":1026,"context_line":""},{"line_number":1027,"context_line":"    max_chassis_number \u003d constants.MAX_CHASSIS_IN_HA_GROUP"},{"line_number":1028,"context_line":"    priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"}],"source_content_type":"text/x-python","patch_set":10,"id":"6c48db1f_04785492","line":1025,"range":{"start_line":1025,"start_character":33,"end_line":1025,"end_character":47},"in_reply_to":"b40f3dc6_4c91df3a","updated":"2023-10-31 11:45:03.000000000","message":"Done","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":true,"context_lines":[{"line_number":1022,"context_line":"        ext_ids \u003d {constants.OVN_AZ_HINTS_EXT_ID_KEY: \u0027,\u0027.join("},{"line_number":1023,"context_line":"            hcg_info.az_hints)}"},{"line_number":1024,"context_line":"        ha_ch_grp_cmd \u003d txn.add(nb_idl.ha_chassis_group_add("},{"line_number":1025,"context_line":"            hcg_info.group_name, may_exist\u003dTrue, external_ids\u003dext_ids))"},{"line_number":1026,"context_line":""},{"line_number":1027,"context_line":"    max_chassis_number \u003d constants.MAX_CHASSIS_IN_HA_GROUP"},{"line_number":1028,"context_line":"    priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"}],"source_content_type":"text/x-python","patch_set":10,"id":"e58a38b8_fe2c1067","line":1025,"range":{"start_line":1025,"start_character":33,"end_line":1025,"end_character":47},"in_reply_to":"d72fb662_53a1c28b","updated":"2023-10-25 10:37:41.000000000","message":"I will test it again, in my tests the previous approach wasn\u0027t working as expected and was returning None when it should be returning a RowView, idk if it was a race condition. Changing it to get() first and create if not found has given consistent results in my tests. But I will re-test the previous approach.\n\nPrior from that, I had a subtranscation that would create the group always and then fetch it but it was changed over time.","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4cf999e0500a7b00bd3104e29e642eb035bb6ec2","unresolved":true,"context_lines":[{"line_number":1022,"context_line":"        ext_ids \u003d {constants.OVN_AZ_HINTS_EXT_ID_KEY: \u0027,\u0027.join("},{"line_number":1023,"context_line":"            hcg_info.az_hints)}"},{"line_number":1024,"context_line":"        ha_ch_grp_cmd \u003d txn.add(nb_idl.ha_chassis_group_add("},{"line_number":1025,"context_line":"            hcg_info.group_name, may_exist\u003dTrue, external_ids\u003dext_ids))"},{"line_number":1026,"context_line":""},{"line_number":1027,"context_line":"    max_chassis_number \u003d constants.MAX_CHASSIS_IN_HA_GROUP"},{"line_number":1028,"context_line":"    priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"}],"source_content_type":"text/x-python","patch_set":10,"id":"b40f3dc6_4c91df3a","line":1025,"range":{"start_line":1025,"start_character":33,"end_line":1025,"end_character":47},"in_reply_to":"e58a38b8_fe2c1067","updated":"2023-10-25 14:05:46.000000000","message":"Just tested changing the code to use the result of the ha_chassis_group_add() as an indicator to know if the ha chassis group existed before or not and it does not work, perhaps it\u0027s a bug in ovsdbapp, I have not dug much into it at this point.\n\nBasically this method would be called twice: First when the external port is created and then when it gets bound to a chassis. In the second call this code should remove the chassis the port was bound to from the list of candidates. Both times the result of ha_chassis_group_add() is None. So the port never gets removed from the ha chassis group.\n\nHere\u0027s the diff: https://paste.opendev.org/show/baSEKV1DRhqbsdSpX0zA/\n\nHere\u0027s the result of the ha_chassis_group_add command I appended to a file while testing (see diff):\n```\nstack@central:~/neutron$ cat /tmp/ha_ch_grp_value \nNone\nNone\n```\n\nstack@central:~/neutron$ sudo pip3 list|  grep ovsdbapp\novsdbapp               2.4.1","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":1025,"context_line":"            hcg_info.group_name, may_exist\u003dTrue, external_ids\u003dext_ids))"},{"line_number":1026,"context_line":""},{"line_number":1027,"context_line":"    max_chassis_number \u003d constants.MAX_CHASSIS_IN_HA_GROUP"},{"line_number":1028,"context_line":"    priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"},{"line_number":1029,"context_line":"    if ha_ch_grp:"},{"line_number":1030,"context_line":"        # Remove any chassis that no longer belongs to the AZ hints"},{"line_number":1031,"context_line":"        # or is ignored"}],"source_content_type":"text/x-python","patch_set":10,"id":"c58ab197_8bb6954f","line":1028,"updated":"2023-10-20 16:03:05.000000000","message":"(No action required) this could also be a function in its own right that would \"return the final list of group members\". (You would then only make sure that add_chassis IDL step is not part of it but left for later simple for-loop over the final list, that would handle the add_chassis IDL part.)","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":false,"context_lines":[{"line_number":1025,"context_line":"            hcg_info.group_name, may_exist\u003dTrue, external_ids\u003dext_ids))"},{"line_number":1026,"context_line":""},{"line_number":1027,"context_line":"    max_chassis_number \u003d constants.MAX_CHASSIS_IN_HA_GROUP"},{"line_number":1028,"context_line":"    priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"},{"line_number":1029,"context_line":"    if ha_ch_grp:"},{"line_number":1030,"context_line":"        # Remove any chassis that no longer belongs to the AZ hints"},{"line_number":1031,"context_line":"        # or is ignored"}],"source_content_type":"text/x-python","patch_set":10,"id":"aa5e94fc_4db8c27c","line":1028,"in_reply_to":"c58ab197_8bb6954f","updated":"2023-10-25 10:37:41.000000000","message":"Ack","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":1040,"context_line":"        # highest priority in the group to avoid ports already bound to it"},{"line_number":1041,"context_line":"        # from moving to another chassis"},{"line_number":1042,"context_line":"        high_prio_ch \u003d max(ha_ch_grp.ha_chassis, key\u003dlambda x: x.priority,"},{"line_number":1043,"context_line":"                           default\u003dNone)"},{"line_number":1044,"context_line":"        priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"},{"line_number":1045,"context_line":"        if (high_prio_ch and"},{"line_number":1046,"context_line":"                high_prio_ch.chassis_name in candidates and"}],"source_content_type":"text/x-python","patch_set":10,"id":"ea22ed65_231ec9e8","line":1043,"range":{"start_line":1043,"start_character":27,"end_line":1043,"end_character":39},"updated":"2023-10-20 16:03:05.000000000","message":"(No action required) default\u003dNone could be removed if you\u0027d replace the `if high_prio_ch` conditional in line 1045 with `if ha_ch_grp.ha_chassis`.","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"eb121eb38af3d0d432ccb6d8b41020fe930b9728","unresolved":false,"context_lines":[{"line_number":1040,"context_line":"        # highest priority in the group to avoid ports already bound to it"},{"line_number":1041,"context_line":"        # from moving to another chassis"},{"line_number":1042,"context_line":"        high_prio_ch \u003d max(ha_ch_grp.ha_chassis, key\u003dlambda x: x.priority,"},{"line_number":1043,"context_line":"                           default\u003dNone)"},{"line_number":1044,"context_line":"        priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"},{"line_number":1045,"context_line":"        if (high_prio_ch and"},{"line_number":1046,"context_line":"                high_prio_ch.chassis_name in candidates and"}],"source_content_type":"text/x-python","patch_set":10,"id":"a8352caa_91ceb377","line":1043,"range":{"start_line":1043,"start_character":27,"end_line":1043,"end_character":39},"in_reply_to":"b5ecc2fc_60ce9bec","updated":"2023-10-27 15:08:14.000000000","message":"\u003e We need the default because\n\nNot if you would structure the code as:\n\n```\nif ha_ch_grp.ha_chassis:\n  high_prio_ch \u003d max(ha_ch_grp.ha_chassis, key\u003d...)\n  if high_prio_ch.chassis_name in candidates:\n    ...\n```\n\nBut as I said, it\u0027s a minor point and more of a \"preference\".","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e24f433f881bcfb860cb1473bf4ba0bb0f364f22","unresolved":false,"context_lines":[{"line_number":1040,"context_line":"        # highest priority in the group to avoid ports already bound to it"},{"line_number":1041,"context_line":"        # from moving to another chassis"},{"line_number":1042,"context_line":"        high_prio_ch \u003d max(ha_ch_grp.ha_chassis, key\u003dlambda x: x.priority,"},{"line_number":1043,"context_line":"                           default\u003dNone)"},{"line_number":1044,"context_line":"        priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"},{"line_number":1045,"context_line":"        if (high_prio_ch and"},{"line_number":1046,"context_line":"                high_prio_ch.chassis_name in candidates and"}],"source_content_type":"text/x-python","patch_set":10,"id":"b5ecc2fc_60ce9bec","line":1043,"range":{"start_line":1043,"start_character":27,"end_line":1043,"end_character":39},"in_reply_to":"d0a3ca9d_ed3dbc47","updated":"2023-10-25 14:07:46.000000000","message":"Done","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":true,"context_lines":[{"line_number":1040,"context_line":"        # highest priority in the group to avoid ports already bound to it"},{"line_number":1041,"context_line":"        # from moving to another chassis"},{"line_number":1042,"context_line":"        high_prio_ch \u003d max(ha_ch_grp.ha_chassis, key\u003dlambda x: x.priority,"},{"line_number":1043,"context_line":"                           default\u003dNone)"},{"line_number":1044,"context_line":"        priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"},{"line_number":1045,"context_line":"        if (high_prio_ch and"},{"line_number":1046,"context_line":"                high_prio_ch.chassis_name in candidates and"}],"source_content_type":"text/x-python","patch_set":10,"id":"d0a3ca9d_ed3dbc47","line":1043,"range":{"start_line":1043,"start_character":27,"end_line":1043,"end_character":39},"in_reply_to":"ea22ed65_231ec9e8","updated":"2023-10-25 10:37:41.000000000","message":"We need the default because if for whatever reason ha_chassis is empty the max() method will raise ValueError:\n\n```\nValueError: max() arg is an empty sequence\n```","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":1044,"context_line":"        priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"},{"line_number":1045,"context_line":"        if (high_prio_ch and"},{"line_number":1046,"context_line":"                high_prio_ch.chassis_name in candidates and"},{"line_number":1047,"context_line":"                high_prio_ch.chassis_name !\u003d hcg_info.ignore_chassis):"},{"line_number":1048,"context_line":"            txn.add(nb_idl.ha_chassis_group_add_chassis("},{"line_number":1049,"context_line":"                hcg_info.group_name, high_prio_ch.chassis_name,"},{"line_number":1050,"context_line":"                priority\u003dpriority))"}],"source_content_type":"text/x-python","patch_set":10,"id":"d8549912_aa69a8da","line":1047,"range":{"start_line":1047,"start_character":16,"end_line":1047,"end_character":68},"updated":"2023-10-20 16:03:05.000000000","message":"I\u0027m still puzzled by this conditional. Don\u0027t we guarantee that `candidates` do NOT contain `ignore_chassis`? I think we do, in line 986.\n\nIf so, this conditional can be removed since it doesn\u0027t add anything to the other condition in line 1046 above. What do I miss?","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":false,"context_lines":[{"line_number":1044,"context_line":"        priority \u003d constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY"},{"line_number":1045,"context_line":"        if (high_prio_ch and"},{"line_number":1046,"context_line":"                high_prio_ch.chassis_name in candidates and"},{"line_number":1047,"context_line":"                high_prio_ch.chassis_name !\u003d hcg_info.ignore_chassis):"},{"line_number":1048,"context_line":"            txn.add(nb_idl.ha_chassis_group_add_chassis("},{"line_number":1049,"context_line":"                hcg_info.group_name, high_prio_ch.chassis_name,"},{"line_number":1050,"context_line":"                priority\u003dpriority))"}],"source_content_type":"text/x-python","patch_set":10,"id":"d847143a_0fa267ea","line":1047,"range":{"start_line":1047,"start_character":16,"end_line":1047,"end_character":68},"in_reply_to":"d8549912_aa69a8da","updated":"2023-10-25 10:37:41.000000000","message":"True, perhaps this can be removed. Because the condition before it should guarantee that this chassis is not in the candidates.","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":1062,"context_line":"    # even if they belonging to the same availability zones do not"},{"line_number":1063,"context_line":"    # necessarily end up with the same Chassis as the highest priority one."},{"line_number":1064,"context_line":"    for ch in random.sample(list(candidates), max_chassis_number):"},{"line_number":1065,"context_line":"        txn.add(nb_idl.ha_chassis_group_add_chassis("},{"line_number":1066,"context_line":"            hcg_info.group_name, ch, priority\u003dpriority))"},{"line_number":1067,"context_line":"        priority -\u003d 1"},{"line_number":1068,"context_line":""},{"line_number":1069,"context_line":"    LOG.info(\u0027HA Chassis Group %s synchronized\u0027, hcg_info.group_name)"},{"line_number":1070,"context_line":"    # Return the existing register UUID or the HA chassis group creation"}],"source_content_type":"text/x-python","patch_set":10,"id":"7941b09b_5fed21da","line":1067,"range":{"start_line":1065,"start_character":0,"end_line":1067,"end_character":21},"updated":"2023-10-20 16:03:05.000000000","message":"these lines are the same as in lines 1048-1050 and line 1052 above. Please introduce a helper closure and reuse in both places. E.g.\n\n```\npriority \u003d ...\ngroup_name \u003d hcg_info.group_name\n\ndef add_chassis(chassis_name):\n  txn.add(...(group_name, chassis_name, priority\u003dpriority))\n  priority -\u003d 1\n```\n\nThat said, I find the fact that we spread the logic that modifies database over two separate branches unfortunate. It would be better to have separate steps that\n\n- first, calculate the final list of members\n- add members from the final list","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"932a2770d84c6cd8a9eb86afa6dd71d6f947ac60","unresolved":true,"context_lines":[{"line_number":1062,"context_line":"    # even if they belonging to the same availability zones do not"},{"line_number":1063,"context_line":"    # necessarily end up with the same Chassis as the highest priority one."},{"line_number":1064,"context_line":"    for ch in random.sample(list(candidates), max_chassis_number):"},{"line_number":1065,"context_line":"        txn.add(nb_idl.ha_chassis_group_add_chassis("},{"line_number":1066,"context_line":"            hcg_info.group_name, ch, priority\u003dpriority))"},{"line_number":1067,"context_line":"        priority -\u003d 1"},{"line_number":1068,"context_line":""},{"line_number":1069,"context_line":"    LOG.info(\u0027HA Chassis Group %s synchronized\u0027, hcg_info.group_name)"},{"line_number":1070,"context_line":"    # Return the existing register UUID or the HA chassis group creation"}],"source_content_type":"text/x-python","patch_set":10,"id":"67feea50_63e10df5","line":1067,"range":{"start_line":1065,"start_character":0,"end_line":1067,"end_character":21},"in_reply_to":"38a4cefb_37bcd152","updated":"2023-10-31 10:19:27.000000000","message":"Thanks for the suggestion. I agree it\u0027s subjective, cause I don\u0027t find this code that bad, to me it flows fine into the next part where the rest of the chassis are added. I mean, I think the main problem in breaking that flow down is the control variables such as priority, candidates and max_chassis_number that are subtracted in case there\u0027s highest priority chassis prior in the group","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":true,"context_lines":[{"line_number":1062,"context_line":"    # even if they belonging to the same availability zones do not"},{"line_number":1063,"context_line":"    # necessarily end up with the same Chassis as the highest priority one."},{"line_number":1064,"context_line":"    for ch in random.sample(list(candidates), max_chassis_number):"},{"line_number":1065,"context_line":"        txn.add(nb_idl.ha_chassis_group_add_chassis("},{"line_number":1066,"context_line":"            hcg_info.group_name, ch, priority\u003dpriority))"},{"line_number":1067,"context_line":"        priority -\u003d 1"},{"line_number":1068,"context_line":""},{"line_number":1069,"context_line":"    LOG.info(\u0027HA Chassis Group %s synchronized\u0027, hcg_info.group_name)"},{"line_number":1070,"context_line":"    # Return the existing register UUID or the HA chassis group creation"}],"source_content_type":"text/x-python","patch_set":10,"id":"d1a4aa8d_f4852649","line":1067,"range":{"start_line":1065,"start_character":0,"end_line":1067,"end_character":21},"in_reply_to":"7941b09b_5fed21da","updated":"2023-10-25 10:37:41.000000000","message":"I tried to add it but, I think it just looks uglier. This new add_chassis() will need to be a sub-method within the sync_ha_chassis_group() to have access to the txn, group_name, chassis_name and priority variables.\n\nUnless I pass everything via parameters but if I subtract the priority within the new method I would need to return the new value too. If I don\u0027t I would end up with a method called add_chassis() that all it does is  txn.add(ha_chassis_group_add_chassis(...).\n\nThanks for the suggestion but I will keep it as-is for now.","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"eb121eb38af3d0d432ccb6d8b41020fe930b9728","unresolved":true,"context_lines":[{"line_number":1062,"context_line":"    # even if they belonging to the same availability zones do not"},{"line_number":1063,"context_line":"    # necessarily end up with the same Chassis as the highest priority one."},{"line_number":1064,"context_line":"    for ch in random.sample(list(candidates), max_chassis_number):"},{"line_number":1065,"context_line":"        txn.add(nb_idl.ha_chassis_group_add_chassis("},{"line_number":1066,"context_line":"            hcg_info.group_name, ch, priority\u003dpriority))"},{"line_number":1067,"context_line":"        priority -\u003d 1"},{"line_number":1068,"context_line":""},{"line_number":1069,"context_line":"    LOG.info(\u0027HA Chassis Group %s synchronized\u0027, hcg_info.group_name)"},{"line_number":1070,"context_line":"    # Return the existing register UUID or the HA chassis group creation"}],"source_content_type":"text/x-python","patch_set":10,"id":"38a4cefb_37bcd152","line":1067,"range":{"start_line":1065,"start_character":0,"end_line":1067,"end_character":21},"in_reply_to":"d1a4aa8d_f4852649","updated":"2023-10-27 15:08:14.000000000","message":"Closures are fine to my taste, but I understand others may feel different.\n\nWhat I find a bit more of a problem is that we insert new group members in two separate places (one for \"high priority chassis\" and one for \"the rest*\".)\n\nI\u0027d prefer if we have two separate sections / functions that\n- form the final list of candidates (incl. the \"retain old high priority member\" logic)\n- implement the list in txn (should be a simple for).\n\nAgain, I realize that this may be subjective and in the end is not the issue that your code introduced, so you are welcome to ignore.\n\n*: reduced by 1 due to high priority entry.","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"10fb8700d93ec0eff20c9fd12a17f10f6932a5bd","unresolved":true,"context_lines":[{"line_number":165,"context_line":""},{"line_number":166,"context_line":"def ovn_extport_chassis_group_name(port_id):"},{"line_number":167,"context_line":"    # The name of the HA Chassis Group entry will be neutron-extport-\u003cUUID\u003e"},{"line_number":168,"context_line":"    return \u0027neutron-extport-%s\u0027 % port_id"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"def ovn_vhu_sockpath(sock_dir, port_id):"}],"source_content_type":"text/x-python","patch_set":12,"id":"805d1db7_4a625d2c","line":168,"updated":"2023-11-03 15:56:16.000000000","message":"Would be good to define this prefix in constants as above, i\u0027m just assuming it\u0027s used elsewhere?","commit_id":"7f021319ac4487dbf7bac1fa74f4e38b5630e3a2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"aa4cfd4cb18e24babb26664ccee4d3c90ee245bb","unresolved":true,"context_lines":[{"line_number":165,"context_line":""},{"line_number":166,"context_line":"def ovn_extport_chassis_group_name(port_id):"},{"line_number":167,"context_line":"    # The name of the HA Chassis Group entry will be neutron-extport-\u003cUUID\u003e"},{"line_number":168,"context_line":"    return \u0027neutron-extport-%s\u0027 % port_id"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"def ovn_vhu_sockpath(sock_dir, port_id):"}],"source_content_type":"text/x-python","patch_set":12,"id":"b043f793_f1dca15d","line":168,"in_reply_to":"805d1db7_4a625d2c","updated":"2023-11-03 16:26:50.000000000","message":"Indeed. In this file we have a mix in some cases we have these prefix as constants and others do not, but, probably the best way is to always use a constant to avoid the risk of a typo. Will do.","commit_id":"7f021319ac4487dbf7bac1fa74f4e38b5630e3a2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"920337fef7c6265d6794ac9cf1f6d46b74336e30","unresolved":false,"context_lines":[{"line_number":165,"context_line":""},{"line_number":166,"context_line":"def ovn_extport_chassis_group_name(port_id):"},{"line_number":167,"context_line":"    # The name of the HA Chassis Group entry will be neutron-extport-\u003cUUID\u003e"},{"line_number":168,"context_line":"    return \u0027neutron-extport-%s\u0027 % port_id"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"def ovn_vhu_sockpath(sock_dir, port_id):"}],"source_content_type":"text/x-python","patch_set":12,"id":"92f7b352_ad6d21ff","line":168,"in_reply_to":"b043f793_f1dca15d","updated":"2023-11-08 09:32:44.000000000","message":"Done","commit_id":"7f021319ac4487dbf7bac1fa74f4e38b5630e3a2"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"1c3c2cf8af4c5d9c85ccec5181d69b4c10c6aff9","unresolved":true,"context_lines":[{"line_number":963,"context_line":"    # If there are Chassis marked for hosting external ports create a HA"},{"line_number":964,"context_line":"    # Chassis Group per external port, otherwise do it at the network level"},{"line_number":965,"context_line":"    chassis_list \u003d sb_idl.get_extport_chassis_from_cms_options()"},{"line_number":966,"context_line":"    if chassis_list:"},{"line_number":967,"context_line":"        group_name \u003d ovn_extport_chassis_group_name(port_id)"},{"line_number":968,"context_line":"        extport_host_present \u003d True"},{"line_number":969,"context_line":"    else:"},{"line_number":970,"context_line":"        chassis_list \u003d sb_idl.get_gateway_chassis_from_cms_options("},{"line_number":971,"context_line":"            name_only\u003dFalse)"},{"line_number":972,"context_line":"        group_name \u003d ovn_name(network_id)"},{"line_number":973,"context_line":"        extport_host_present \u003d False"},{"line_number":974,"context_line":""},{"line_number":975,"context_line":"    ignore_chassis \u003d set()"},{"line_number":976,"context_line":"    if extport_host_present:"},{"line_number":977,"context_line":"        # Check if the port is bound to a chassis and if so, ignore that"},{"line_number":978,"context_line":"        # chassis when building the HA Chassis Group to ensure the"},{"line_number":979,"context_line":"        # external port is bound to a different chassis than the VM"},{"line_number":980,"context_line":"        ignore_chassis \u003d sb_idl.get_chassis_host_for_port(port_id)"},{"line_number":981,"context_line":"        LOG.debug(\u0027HA Chassis Group %s is based on external port %s \u0027"},{"line_number":982,"context_line":"                  \u0027(network %s)\u0027, group_name, port_id, network_id)"},{"line_number":983,"context_line":"    else:"},{"line_number":984,"context_line":"        LOG.debug(\u0027HA Chassis Group %s is based on network %s\u0027,"},{"line_number":985,"context_line":"                  group_name, network_id)"},{"line_number":986,"context_line":""},{"line_number":987,"context_line":"    # Get the Availability Zones hints"},{"line_number":988,"context_line":"    plugin \u003d directory.get_plugin()"}],"source_content_type":"text/x-python","patch_set":14,"id":"2e3652e7_ff001a43","line":985,"range":{"start_line":966,"start_character":0,"end_line":985,"end_character":41},"updated":"2023-11-13 10:58:05.000000000","message":"is this \"extport_host_present\" really needed? IMO it can all be done in one single \"if...else\" block.","commit_id":"e8aa91eb47157e75ab7ae9c49ac8dd91b3b970a9"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"c34253b2f387476a702fa8d9b812971a293683e2","unresolved":true,"context_lines":[{"line_number":963,"context_line":"    # If there are Chassis marked for hosting external ports create a HA"},{"line_number":964,"context_line":"    # Chassis Group per external port, otherwise do it at the network level"},{"line_number":965,"context_line":"    chassis_list \u003d sb_idl.get_extport_chassis_from_cms_options()"},{"line_number":966,"context_line":"    if chassis_list:"},{"line_number":967,"context_line":"        group_name \u003d ovn_extport_chassis_group_name(port_id)"},{"line_number":968,"context_line":"        extport_host_present \u003d True"},{"line_number":969,"context_line":"    else:"},{"line_number":970,"context_line":"        chassis_list \u003d sb_idl.get_gateway_chassis_from_cms_options("},{"line_number":971,"context_line":"            name_only\u003dFalse)"},{"line_number":972,"context_line":"        group_name \u003d ovn_name(network_id)"},{"line_number":973,"context_line":"        extport_host_present \u003d False"},{"line_number":974,"context_line":""},{"line_number":975,"context_line":"    ignore_chassis \u003d set()"},{"line_number":976,"context_line":"    if extport_host_present:"},{"line_number":977,"context_line":"        # Check if the port is bound to a chassis and if so, ignore that"},{"line_number":978,"context_line":"        # chassis when building the HA Chassis Group to ensure the"},{"line_number":979,"context_line":"        # external port is bound to a different chassis than the VM"},{"line_number":980,"context_line":"        ignore_chassis \u003d sb_idl.get_chassis_host_for_port(port_id)"},{"line_number":981,"context_line":"        LOG.debug(\u0027HA Chassis Group %s is based on external port %s \u0027"},{"line_number":982,"context_line":"                  \u0027(network %s)\u0027, group_name, port_id, network_id)"},{"line_number":983,"context_line":"    else:"},{"line_number":984,"context_line":"        LOG.debug(\u0027HA Chassis Group %s is based on network %s\u0027,"},{"line_number":985,"context_line":"                  group_name, network_id)"},{"line_number":986,"context_line":""},{"line_number":987,"context_line":"    # Get the Availability Zones hints"},{"line_number":988,"context_line":"    plugin \u003d directory.get_plugin()"}],"source_content_type":"text/x-python","patch_set":14,"id":"f73226fa_1e8cd1e8","line":985,"range":{"start_line":966,"start_character":0,"end_line":985,"end_character":41},"in_reply_to":"2e3652e7_ff001a43","updated":"2023-11-13 11:07:04.000000000","message":"Indeed I could just use the if..else above based on chassis_list. If there\u0027s a need for a new patch-set I can group it all in one if..else","commit_id":"e8aa91eb47157e75ab7ae9c49ac8dd91b3b970a9"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"6f67021e402624b60a32d356c7e5b3874a85f67a","unresolved":false,"context_lines":[{"line_number":963,"context_line":"    # If there are Chassis marked for hosting external ports create a HA"},{"line_number":964,"context_line":"    # Chassis Group per external port, otherwise do it at the network level"},{"line_number":965,"context_line":"    chassis_list \u003d sb_idl.get_extport_chassis_from_cms_options()"},{"line_number":966,"context_line":"    if chassis_list:"},{"line_number":967,"context_line":"        group_name \u003d ovn_extport_chassis_group_name(port_id)"},{"line_number":968,"context_line":"        extport_host_present \u003d True"},{"line_number":969,"context_line":"    else:"},{"line_number":970,"context_line":"        chassis_list \u003d sb_idl.get_gateway_chassis_from_cms_options("},{"line_number":971,"context_line":"            name_only\u003dFalse)"},{"line_number":972,"context_line":"        group_name \u003d ovn_name(network_id)"},{"line_number":973,"context_line":"        extport_host_present \u003d False"},{"line_number":974,"context_line":""},{"line_number":975,"context_line":"    ignore_chassis \u003d set()"},{"line_number":976,"context_line":"    if extport_host_present:"},{"line_number":977,"context_line":"        # Check if the port is bound to a chassis and if so, ignore that"},{"line_number":978,"context_line":"        # chassis when building the HA Chassis Group to ensure the"},{"line_number":979,"context_line":"        # external port is bound to a different chassis than the VM"},{"line_number":980,"context_line":"        ignore_chassis \u003d sb_idl.get_chassis_host_for_port(port_id)"},{"line_number":981,"context_line":"        LOG.debug(\u0027HA Chassis Group %s is based on external port %s \u0027"},{"line_number":982,"context_line":"                  \u0027(network %s)\u0027, group_name, port_id, network_id)"},{"line_number":983,"context_line":"    else:"},{"line_number":984,"context_line":"        LOG.debug(\u0027HA Chassis Group %s is based on network %s\u0027,"},{"line_number":985,"context_line":"                  group_name, network_id)"},{"line_number":986,"context_line":""},{"line_number":987,"context_line":"    # Get the Availability Zones hints"},{"line_number":988,"context_line":"    plugin \u003d directory.get_plugin()"}],"source_content_type":"text/x-python","patch_set":14,"id":"48c19942_59b4703c","line":985,"range":{"start_line":966,"start_character":0,"end_line":985,"end_character":41},"in_reply_to":"f73226fa_1e8cd1e8","updated":"2023-11-13 14:13:38.000000000","message":"Done","commit_id":"e8aa91eb47157e75ab7ae9c49ac8dd91b3b970a9"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5e426fb86ed2325cfba3dfc6defda6fe7837f4f0","unresolved":true,"context_lines":[{"line_number":1166,"context_line":"            # same chassis as the VM"},{"line_number":1167,"context_line":"            if (ovn_utils.is_port_external(db_port) and"},{"line_number":1168,"context_line":"                    self.sb_ovn.get_extport_chassis_from_cms_options()):"},{"line_number":1169,"context_line":"                with self.nb_ovn.transaction(check_error\u003dTrue) as txn:"},{"line_number":1170,"context_line":"                    ovn_utils.sync_ha_chassis_group("},{"line_number":1171,"context_line":"                        admin_context, db_port[\u0027id\u0027], db_port[\u0027network_id\u0027],"},{"line_number":1172,"context_line":"                        self.nb_ovn, self.sb_ovn, txn)"}],"source_content_type":"text/x-python","patch_set":9,"id":"b0f44e42_43fc30ce","line":1169,"updated":"2023-10-12 17:10:08.000000000","message":"(Please bear with me)\n\nSo if transaction fails, will we return the ovsdbapp exception to the caller? (Which is ovsdb-monitor, which will make us lose the event?)","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97145c01d2a9f24d687c5eebf247bab435cdf36","unresolved":false,"context_lines":[{"line_number":1166,"context_line":"            # same chassis as the VM"},{"line_number":1167,"context_line":"            if (ovn_utils.is_port_external(db_port) and"},{"line_number":1168,"context_line":"                    self.sb_ovn.get_extport_chassis_from_cms_options()):"},{"line_number":1169,"context_line":"                with self.nb_ovn.transaction(check_error\u003dTrue) as txn:"},{"line_number":1170,"context_line":"                    ovn_utils.sync_ha_chassis_group("},{"line_number":1171,"context_line":"                        admin_context, db_port[\u0027id\u0027], db_port[\u0027network_id\u0027],"},{"line_number":1172,"context_line":"                        self.nb_ovn, self.sb_ovn, txn)"}],"source_content_type":"text/x-python","patch_set":9,"id":"4ba0564c_cb26e56c","line":1169,"in_reply_to":"b0f44e42_43fc30ce","updated":"2023-10-13 09:38:41.000000000","message":"Yes it will bubble up, perhaps not the best. I think I will add a try..except around this code block and log a message in case something goes wrong here.\n\nThis code seems fragile as-is, the same could happen with the update_lsp_host_info() from L1161. But that doesn\u0027t justify making it more fragile by adding another call that can fail and bubble up.\n\nI will change it.","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"19c7b17370e4ae0a5b3826c1bf62dc7913c7bb0a","unresolved":true,"context_lines":[{"line_number":1175,"context_line":"                        admin_context, db_port[\u0027id\u0027], db_port[\u0027network_id\u0027],"},{"line_number":1176,"context_line":"                        self.nb_ovn, self.sb_ovn, txn)"},{"line_number":1177,"context_line":"            except Exception as e:"},{"line_number":1178,"context_line":"                LOG.error(\u0027Error while syncing the HA Chassis Group for the\u0027"},{"line_number":1179,"context_line":"                          \u0027external port %s during set port status up. \u0027"},{"line_number":1180,"context_line":"                          \u0027Error: %s\u0027, db_port[\u0027id\u0027], e)"},{"line_number":1181,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"9e997fae_32b093c4","line":1178,"range":{"start_line":1178,"start_character":72,"end_line":1178,"end_character":75},"updated":"2023-11-13 17:50:02.000000000","message":"missing trailing space so message will be \u0027theexternal\u0027 :(","commit_id":"973c500d4b4ed67e4ab0ece192fa01c14523eda1"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"2931509b47df66afafb8d684b3a83f22175a3da5","unresolved":false,"context_lines":[{"line_number":1175,"context_line":"                        admin_context, db_port[\u0027id\u0027], db_port[\u0027network_id\u0027],"},{"line_number":1176,"context_line":"                        self.nb_ovn, self.sb_ovn, txn)"},{"line_number":1177,"context_line":"            except Exception as e:"},{"line_number":1178,"context_line":"                LOG.error(\u0027Error while syncing the HA Chassis Group for the\u0027"},{"line_number":1179,"context_line":"                          \u0027external port %s during set port status up. \u0027"},{"line_number":1180,"context_line":"                          \u0027Error: %s\u0027, db_port[\u0027id\u0027], e)"},{"line_number":1181,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"0c8e1cde_9ee5b046","line":1178,"range":{"start_line":1178,"start_character":72,"end_line":1178,"end_character":75},"in_reply_to":"9e997fae_32b093c4","updated":"2023-11-14 08:54:10.000000000","message":"Oops... will fix","commit_id":"973c500d4b4ed67e4ab0ece192fa01c14523eda1"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c591f83b1ab660102540ddcc71423fe03185b4ff","unresolved":true,"context_lines":[{"line_number":670,"context_line":"           external_ids:ovn-cms-options\u003d\"enable-chassis-as-extport-host\""},{"line_number":671,"context_line":"        In this function, we parse ovn-cms-options and return these chassis"},{"line_number":672,"context_line":""},{"line_number":673,"context_line":"        :param name_only: Return only the chassis names instead of"},{"line_number":674,"context_line":"                          objects. Defaults to True."},{"line_number":675,"context_line":"        :returns: List with chassis."},{"line_number":676,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"a414fe9a_52e660cc","line":673,"updated":"2023-09-21 20:00:47.000000000","message":"this is another shapeshifter that I\u0027d advise against. You can have a separate method for each of the modes.","commit_id":"32238978ea8df28510601395bd889aebbfefda1b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"404e2df7e3ffc64e446b8209248b9477732e2562","unresolved":true,"context_lines":[{"line_number":670,"context_line":"           external_ids:ovn-cms-options\u003d\"enable-chassis-as-extport-host\""},{"line_number":671,"context_line":"        In this function, we parse ovn-cms-options and return these chassis"},{"line_number":672,"context_line":""},{"line_number":673,"context_line":"        :param name_only: Return only the chassis names instead of"},{"line_number":674,"context_line":"                          objects. Defaults to True."},{"line_number":675,"context_line":"        :returns: List with chassis."},{"line_number":676,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"f2cb734e_8f1b7cd0","line":673,"in_reply_to":"a414fe9a_52e660cc","updated":"2023-09-22 09:38:30.000000000","message":"I think for my patch I am never changing the default value for this parameter, so I think we can get rid of it and always return the names","commit_id":"32238978ea8df28510601395bd889aebbfefda1b"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"cff39ea4493063d4ac24f34c68cdc03f840f2e38","unresolved":false,"context_lines":[{"line_number":670,"context_line":"           external_ids:ovn-cms-options\u003d\"enable-chassis-as-extport-host\""},{"line_number":671,"context_line":"        In this function, we parse ovn-cms-options and return these chassis"},{"line_number":672,"context_line":""},{"line_number":673,"context_line":"        :param name_only: Return only the chassis names instead of"},{"line_number":674,"context_line":"                          objects. Defaults to True."},{"line_number":675,"context_line":"        :returns: List with chassis."},{"line_number":676,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"d647fac8_945d2a9d","line":673,"in_reply_to":"f2cb734e_8f1b7cd0","updated":"2023-09-22 13:33:38.000000000","message":"Done","commit_id":"32238978ea8df28510601395bd889aebbfefda1b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5e426fb86ed2325cfba3dfc6defda6fe7837f4f0","unresolved":true,"context_lines":[{"line_number":694,"context_line":"        \"\"\"Return the Chassis hosting the port"},{"line_number":695,"context_line":""},{"line_number":696,"context_line":"        :param port_id:     The port ID"},{"line_number":697,"context_line":"        :type chassis_type: string"},{"line_number":698,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"9e642124_add5b287","line":697,"updated":"2023-10-12 17:10:08.000000000","message":"wrong param name?","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97145c01d2a9f24d687c5eebf247bab435cdf36","unresolved":false,"context_lines":[{"line_number":694,"context_line":"        \"\"\"Return the Chassis hosting the port"},{"line_number":695,"context_line":""},{"line_number":696,"context_line":"        :param port_id:     The port ID"},{"line_number":697,"context_line":"        :type chassis_type: string"},{"line_number":698,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"cd078bc8_fcc45c87","line":697,"in_reply_to":"9e642124_add5b287","updated":"2023-10-13 09:38:41.000000000","message":"Ops, good catch","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":977,"context_line":"            ret \u003d cmd.execute(check_error\u003dTrue)[0]"},{"line_number":978,"context_line":"        except IndexError:"},{"line_number":979,"context_line":"            return"},{"line_number":980,"context_line":"        return ret.chassis"}],"source_content_type":"text/x-python","patch_set":10,"id":"89c14cc2_a06592f0","line":980,"updated":"2023-10-20 16:03:05.000000000","message":"VIF LSP can be bound to multiple chassis. In this case, ret.additional_chassis may list additional chassis. (This is the case when the port is being live migrated.)\n\nI feel we may have to convert the function to return an iterable of chassis that would combine both `.chassis` and .additional_chassis` fields.\n\nThoughts?\n\n(And if you wonder why in this case it is not handled in line 972... It is a bug, and Jakub has a [patch](https://review.opendev.org/c/openstack/neutron/+/895402/8/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py) to fix it.","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":false,"context_lines":[{"line_number":977,"context_line":"            ret \u003d cmd.execute(check_error\u003dTrue)[0]"},{"line_number":978,"context_line":"        except IndexError:"},{"line_number":979,"context_line":"            return"},{"line_number":980,"context_line":"        return ret.chassis"}],"source_content_type":"text/x-python","patch_set":10,"id":"a2e692ea_32656eeb","line":980,"in_reply_to":"89c14cc2_a06592f0","updated":"2023-10-25 10:37:41.000000000","message":"Good point, will convert it into a list of chassis","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"eb121eb38af3d0d432ccb6d8b41020fe930b9728","unresolved":true,"context_lines":[{"line_number":981,"context_line":""},{"line_number":982,"context_line":"    def get_chassis_host_for_port(self, port_id):"},{"line_number":983,"context_line":"        chassis \u003d set()"},{"line_number":984,"context_line":"        additional_chassis_supported \u003d self.is_col_present("},{"line_number":985,"context_line":"            \u0027Port_Binding\u0027, \u0027additional_chassis\u0027)"},{"line_number":986,"context_line":"        cmd \u003d self.db_find_rows(\u0027Port_Binding\u0027, (\u0027logical_port\u0027, \u0027\u003d\u0027, port_id))"},{"line_number":987,"context_line":"        for row in cmd.execute(check_error\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":11,"id":"418ef356_4e5d279d","line":984,"updated":"2023-10-27 15:08:14.000000000","message":"(No action required) this check will be moved to a separate helper in the Jakub\u0027s [patch](https://review.opendev.org/c/openstack/neutron/+/895402/8/neutron/common/ovn/utils.py) and we may want to adopt it later.\n\nRegardless, since this check will eventually not be needed, you may want to tag it here with a TODO to remove after we bump minimal OVN version to 22.06.","commit_id":"90dd48843fde08d3a44e94df9a3adc52167f0925"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"932a2770d84c6cd8a9eb86afa6dd71d6f947ac60","unresolved":false,"context_lines":[{"line_number":981,"context_line":""},{"line_number":982,"context_line":"    def get_chassis_host_for_port(self, port_id):"},{"line_number":983,"context_line":"        chassis \u003d set()"},{"line_number":984,"context_line":"        additional_chassis_supported \u003d self.is_col_present("},{"line_number":985,"context_line":"            \u0027Port_Binding\u0027, \u0027additional_chassis\u0027)"},{"line_number":986,"context_line":"        cmd \u003d self.db_find_rows(\u0027Port_Binding\u0027, (\u0027logical_port\u0027, \u0027\u003d\u0027, port_id))"},{"line_number":987,"context_line":"        for row in cmd.execute(check_error\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":11,"id":"02c13d12_22c41afe","line":984,"in_reply_to":"418ef356_4e5d279d","updated":"2023-10-31 10:19:27.000000000","message":"Will do!","commit_id":"90dd48843fde08d3a44e94df9a3adc52167f0925"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"eb121eb38af3d0d432ccb6d8b41020fe930b9728","unresolved":true,"context_lines":[{"line_number":986,"context_line":"        cmd \u003d self.db_find_rows(\u0027Port_Binding\u0027, (\u0027logical_port\u0027, \u0027\u003d\u0027, port_id))"},{"line_number":987,"context_line":"        for row in cmd.execute(check_error\u003dTrue):"},{"line_number":988,"context_line":"            if not row.chassis:"},{"line_number":989,"context_line":"                continue"},{"line_number":990,"context_line":"            chassis.add(row.chassis[0].name)"},{"line_number":991,"context_line":"            if additional_chassis_supported:"},{"line_number":992,"context_line":"                for ch in row.additional_chassis:"}],"source_content_type":"text/x-python","patch_set":11,"id":"f709ce26_a8b635fd","line":989,"updated":"2023-10-27 15:08:14.000000000","message":"I don\u0027t think you should short-circuit here, but proceed to additional_chassis handling. This is because each ovn-controller manages its relevant entry independently, and there may be a scenario where the \"main\" chassis hasn\u0027t set the `.chassis` field yet, but the \"additional\" chassis did for its `.additional_chassis`.\n\nI think it would be safer to not assume the order.","commit_id":"90dd48843fde08d3a44e94df9a3adc52167f0925"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"932a2770d84c6cd8a9eb86afa6dd71d6f947ac60","unresolved":false,"context_lines":[{"line_number":986,"context_line":"        cmd \u003d self.db_find_rows(\u0027Port_Binding\u0027, (\u0027logical_port\u0027, \u0027\u003d\u0027, port_id))"},{"line_number":987,"context_line":"        for row in cmd.execute(check_error\u003dTrue):"},{"line_number":988,"context_line":"            if not row.chassis:"},{"line_number":989,"context_line":"                continue"},{"line_number":990,"context_line":"            chassis.add(row.chassis[0].name)"},{"line_number":991,"context_line":"            if additional_chassis_supported:"},{"line_number":992,"context_line":"                for ch in row.additional_chassis:"}],"source_content_type":"text/x-python","patch_set":11,"id":"d99f084f_60de2fc2","line":989,"in_reply_to":"f709ce26_a8b635fd","updated":"2023-10-31 10:19:27.000000000","message":"Ah fair enough, I see... I assumed that if Chassis wasn\u0027t present it could be ignored (just like it would prior from making use of the additional_chassis column).","commit_id":"90dd48843fde08d3a44e94df9a3adc52167f0925"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5e426fb86ed2325cfba3dfc6defda6fe7837f4f0","unresolved":true,"context_lines":[{"line_number":777,"context_line":""},{"line_number":778,"context_line":"        # NOTE(lucasagomes): We need to delete the LSP before we attempt"},{"line_number":779,"context_line":"        # to remove the HA Chassis Group or it will fail with a violation"},{"line_number":780,"context_line":"        # error due to the LSP reference in the group"},{"line_number":781,"context_line":"        with self._nb_idl.transaction(check_error\u003dTrue) as txn:"},{"line_number":782,"context_line":"            if ovn_port.type \u003d\u003d ovn_const.LSP_TYPE_EXTERNAL:"},{"line_number":783,"context_line":"                ha_ch_grp_name \u003d utils.ovn_extport_chassis_group_name(port_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"ec243425_ed282408","line":780,"updated":"2023-10-12 17:10:08.000000000","message":"I may misunderstand the comment but isn\u0027t a single transaction supposed to take care of it?","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":false,"context_lines":[{"line_number":777,"context_line":""},{"line_number":778,"context_line":"        # NOTE(lucasagomes): We need to delete the LSP before we attempt"},{"line_number":779,"context_line":"        # to remove the HA Chassis Group or it will fail with a violation"},{"line_number":780,"context_line":"        # error due to the LSP reference in the group"},{"line_number":781,"context_line":"        with self._nb_idl.transaction(check_error\u003dTrue) as txn:"},{"line_number":782,"context_line":"            if ovn_port.type \u003d\u003d ovn_const.LSP_TYPE_EXTERNAL:"},{"line_number":783,"context_line":"                ha_ch_grp_name \u003d utils.ovn_extport_chassis_group_name(port_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"cb272ec8_f1363642","line":780,"in_reply_to":"79bb6d7a_f7a55333","updated":"2023-10-25 10:37:41.000000000","message":"Done","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97145c01d2a9f24d687c5eebf247bab435cdf36","unresolved":true,"context_lines":[{"line_number":777,"context_line":""},{"line_number":778,"context_line":"        # NOTE(lucasagomes): We need to delete the LSP before we attempt"},{"line_number":779,"context_line":"        # to remove the HA Chassis Group or it will fail with a violation"},{"line_number":780,"context_line":"        # error due to the LSP reference in the group"},{"line_number":781,"context_line":"        with self._nb_idl.transaction(check_error\u003dTrue) as txn:"},{"line_number":782,"context_line":"            if ovn_port.type \u003d\u003d ovn_const.LSP_TYPE_EXTERNAL:"},{"line_number":783,"context_line":"                ha_ch_grp_name \u003d utils.ovn_extport_chassis_group_name(port_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"79bb6d7a_f7a55333","line":780,"in_reply_to":"ec243425_ed282408","updated":"2023-10-13 09:38:41.000000000","message":"During my test when I tried to remove the HA Chassis Group in the same transaction that deletes the port it was giving me a violation error, that\u0027s why I separated it and added this note. I will try again.","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"}],"neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"d0ac3835bf5ed0f7041df767e296b9d9ae000b4b","unresolved":true,"context_lines":[{"line_number":2583,"context_line":"        expected_candidates \u003d [ch0.name, ch2.name]"},{"line_number":2584,"context_line":"        self.assertEqual(sorted(expected_candidates), sorted(candidates))"},{"line_number":2585,"context_line":""},{"line_number":2586,"context_line":"    def test_sync_ha_chassis_group(self):"},{"line_number":2587,"context_line":"        self.nb_ovn.ha_chassis_group_get.side_effect \u003d idlutils.RowNotFound"},{"line_number":2588,"context_line":"        fake_txn \u003d mock.MagicMock()"},{"line_number":2589,"context_line":"        net_attrs \u003d {az_def.AZ_HINTS: [\u0027az0\u0027, \u0027az1\u0027, \u0027az2\u0027]}"}],"source_content_type":"text/x-python","patch_set":10,"id":"01b695c7_55a24a4a","line":2586,"updated":"2023-10-21 00:32:01.000000000","message":"I also wonder... now that you so neatly split out the mega-function into smaller pieces, if we could add some new strategic unit test cases targeting the pieces and not the whole.","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"960b85f734533ae4d8418bb1553002cbaa598208","unresolved":false,"context_lines":[{"line_number":2583,"context_line":"        expected_candidates \u003d [ch0.name, ch2.name]"},{"line_number":2584,"context_line":"        self.assertEqual(sorted(expected_candidates), sorted(candidates))"},{"line_number":2585,"context_line":""},{"line_number":2586,"context_line":"    def test_sync_ha_chassis_group(self):"},{"line_number":2587,"context_line":"        self.nb_ovn.ha_chassis_group_get.side_effect \u003d idlutils.RowNotFound"},{"line_number":2588,"context_line":"        fake_txn \u003d mock.MagicMock()"},{"line_number":2589,"context_line":"        net_attrs \u003d {az_def.AZ_HINTS: [\u0027az0\u0027, \u0027az1\u0027, \u0027az2\u0027]}"}],"source_content_type":"text/x-python","patch_set":10,"id":"5dc00109_e6a151cf","line":2586,"in_reply_to":"01b695c7_55a24a4a","updated":"2023-10-26 13:18:11.000000000","message":"Done","commit_id":"886956499b129f3b53394644450f97af5b85d60f"}],"releasenotes/notes/external-port-scheduling-a5419ac51d863087.yaml":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c3e77b01f5a00ad2c243322f8a9e8531c94a85eb","unresolved":true,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    A new ovn-cms-configuration called \"enable-chassis-as-extport-host\" is now"},{"line_number":5,"context_line":"    recoginized by ML2/OVN and is used to identify nodes that are eligible"},{"line_number":6,"context_line":"    for scheduling OVN\u0027s external ports. This feature is backward compatible"},{"line_number":7,"context_line":"    and if no nodes contains this new configuration the external ports will"},{"line_number":8,"context_line":"    continue to be scheduled as before."}],"source_content_type":"text/x-yaml","patch_set":8,"id":"215e2233_86f814f6","line":5,"updated":"2023-10-03 21:07:21.000000000","message":"recognized","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"310fd351d3f7eb6a49b5179849247a241fa762b1","unresolved":false,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    A new ovn-cms-configuration called \"enable-chassis-as-extport-host\" is now"},{"line_number":5,"context_line":"    recoginized by ML2/OVN and is used to identify nodes that are eligible"},{"line_number":6,"context_line":"    for scheduling OVN\u0027s external ports. This feature is backward compatible"},{"line_number":7,"context_line":"    and if no nodes contains this new configuration the external ports will"},{"line_number":8,"context_line":"    continue to be scheduled as before."}],"source_content_type":"text/x-yaml","patch_set":8,"id":"7c81554f_1bb6a72d","line":5,"in_reply_to":"215e2233_86f814f6","updated":"2023-10-04 09:36:07.000000000","message":"Done","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"c3e77b01f5a00ad2c243322f8a9e8531c94a85eb","unresolved":true,"context_lines":[{"line_number":4,"context_line":"    A new ovn-cms-configuration called \"enable-chassis-as-extport-host\" is now"},{"line_number":5,"context_line":"    recoginized by ML2/OVN and is used to identify nodes that are eligible"},{"line_number":6,"context_line":"    for scheduling OVN\u0027s external ports. This feature is backward compatible"},{"line_number":7,"context_line":"    and if no nodes contains this new configuration the external ports will"},{"line_number":8,"context_line":"    continue to be scheduled as before."}],"source_content_type":"text/x-yaml","patch_set":8,"id":"ad0e7200_fcc6ee86","line":7,"updated":"2023-10-03 21:07:21.000000000","message":"no nodes contain","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"310fd351d3f7eb6a49b5179849247a241fa762b1","unresolved":false,"context_lines":[{"line_number":4,"context_line":"    A new ovn-cms-configuration called \"enable-chassis-as-extport-host\" is now"},{"line_number":5,"context_line":"    recoginized by ML2/OVN and is used to identify nodes that are eligible"},{"line_number":6,"context_line":"    for scheduling OVN\u0027s external ports. This feature is backward compatible"},{"line_number":7,"context_line":"    and if no nodes contains this new configuration the external ports will"},{"line_number":8,"context_line":"    continue to be scheduled as before."}],"source_content_type":"text/x-yaml","patch_set":8,"id":"fe0984d3_cdb90a4b","line":7,"in_reply_to":"ad0e7200_fcc6ee86","updated":"2023-10-04 09:36:07.000000000","message":"Done","commit_id":"dc8a9a2b0f0cdc91e6c741a567aad24419a605b3"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5e426fb86ed2325cfba3dfc6defda6fe7837f4f0","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    A new ovn-cms-configuration called \"enable-chassis-as-extport-host\" is now"},{"line_number":5,"context_line":"    recognized by ML2/OVN and is used to identify nodes that are eligible"},{"line_number":6,"context_line":"    for scheduling OVN\u0027s external ports. This feature is backward compatible"},{"line_number":7,"context_line":"    and if no nodes contain this new configuration the external ports will"}],"source_content_type":"text/x-yaml","patch_set":9,"id":"fdc0f5fe_e6501f5c","line":4,"range":{"start_line":4,"start_character":10,"end_line":4,"end_character":31},"updated":"2023-10-12 17:10:08.000000000","message":"What is it? Do you mean `ovn-cms-options option`?","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97145c01d2a9f24d687c5eebf247bab435cdf36","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    A new ovn-cms-configuration called \"enable-chassis-as-extport-host\" is now"},{"line_number":5,"context_line":"    recognized by ML2/OVN and is used to identify nodes that are eligible"},{"line_number":6,"context_line":"    for scheduling OVN\u0027s external ports. This feature is backward compatible"},{"line_number":7,"context_line":"    and if no nodes contain this new configuration the external ports will"}],"source_content_type":"text/x-yaml","patch_set":9,"id":"e7877d6a_674bb940","line":4,"range":{"start_line":4,"start_character":10,"end_line":4,"end_character":31},"in_reply_to":"fdc0f5fe_e6501f5c","updated":"2023-10-13 09:38:41.000000000","message":"Ops, yes!","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5e426fb86ed2325cfba3dfc6defda6fe7837f4f0","unresolved":true,"context_lines":[{"line_number":6,"context_line":"    for scheduling OVN\u0027s external ports. This feature is backward compatible"},{"line_number":7,"context_line":"    and if no nodes contain this new configuration the external ports will"},{"line_number":8,"context_line":"    continue to be scheduled as before."},{"line_number":9,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":9,"id":"899f5828_6905af46","line":9,"updated":"2023-10-12 17:10:08.000000000","message":"nit: spurious newlines","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"e97145c01d2a9f24d687c5eebf247bab435cdf36","unresolved":false,"context_lines":[{"line_number":6,"context_line":"    for scheduling OVN\u0027s external ports. This feature is backward compatible"},{"line_number":7,"context_line":"    and if no nodes contain this new configuration the external ports will"},{"line_number":8,"context_line":"    continue to be scheduled as before."},{"line_number":9,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":9,"id":"1aecdf1f_c48a343b","line":9,"in_reply_to":"899f5828_6905af46","updated":"2023-10-13 09:38:41.000000000","message":"Removing it, thank you!","commit_id":"354a6db7b210125c13dc70bd33bae1eb501c2bbe"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7cdfa26cfbde378cce96306ddef403e4b3bbabd1","unresolved":true,"context_lines":[{"line_number":7,"context_line":"    compatible and if no nodes contain this new option is found the external"},{"line_number":8,"context_line":"    ports will continue to be scheduled using the \"enable-chassis-as-gw\""},{"line_number":9,"context_line":"    option as before."},{"line_number":10,"context_line":"    This change also introduced a limit to the number of members for each"},{"line_number":11,"context_line":"    HA Chassis Group to 3. This is because OVN uses BFD to monitor the"},{"line_number":12,"context_line":"    connectivity of each member and having an unlimited number of members"},{"line_number":13,"context_line":"    could potentially put a lot of stress in OVN."}],"source_content_type":"text/x-yaml","patch_set":10,"id":"c14b7508_a7f959fc","line":10,"updated":"2023-10-20 16:03:05.000000000","message":"I understand there may be a good reason to consolidate the behavior for both cases to a single number, but I think 5 (as used for -as-gw prior to this patch) is as good a number to consolidate on for this matter.\n\nAnd if there\u0027s a justification to reduce the number from 5 to 3 (for both cases), this can - and then should - be done as a separate patch (basically, just changing the constant value(s) from 5 to 3 + release note + justification).\n\nIf you agree that switching from 5 to 3 for -as-gw is not part of the core of this patch, then please split this change out (and remove the additional note here then).","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"960b85f734533ae4d8418bb1553002cbaa598208","unresolved":false,"context_lines":[{"line_number":7,"context_line":"    compatible and if no nodes contain this new option is found the external"},{"line_number":8,"context_line":"    ports will continue to be scheduled using the \"enable-chassis-as-gw\""},{"line_number":9,"context_line":"    option as before."},{"line_number":10,"context_line":"    This change also introduced a limit to the number of members for each"},{"line_number":11,"context_line":"    HA Chassis Group to 3. This is because OVN uses BFD to monitor the"},{"line_number":12,"context_line":"    connectivity of each member and having an unlimited number of members"},{"line_number":13,"context_line":"    could potentially put a lot of stress in OVN."}],"source_content_type":"text/x-yaml","patch_set":10,"id":"18cafe02_57efbcb2","line":10,"in_reply_to":"34b9e80c_d9288c62","updated":"2023-10-26 13:18:11.000000000","message":"Done","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"ebbad3d05b118e818d69e47205bbdcd9b1c4e5d4","unresolved":true,"context_lines":[{"line_number":7,"context_line":"    compatible and if no nodes contain this new option is found the external"},{"line_number":8,"context_line":"    ports will continue to be scheduled using the \"enable-chassis-as-gw\""},{"line_number":9,"context_line":"    option as before."},{"line_number":10,"context_line":"    This change also introduced a limit to the number of members for each"},{"line_number":11,"context_line":"    HA Chassis Group to 3. This is because OVN uses BFD to monitor the"},{"line_number":12,"context_line":"    connectivity of each member and having an unlimited number of members"},{"line_number":13,"context_line":"    could potentially put a lot of stress in OVN."}],"source_content_type":"text/x-yaml","patch_set":10,"id":"34b9e80c_d9288c62","line":10,"in_reply_to":"c14b7508_a7f959fc","updated":"2023-10-25 10:37:41.000000000","message":"I will change it to 5, but, the 5 from the constants was only applicable for GW Router ports and not external ports.","commit_id":"886956499b129f3b53394644450f97af5b85d60f"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"10fb8700d93ec0eff20c9fd12a17f10f6932a5bd","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    A new ovn-cms-options option called \"enable-chassis-as-extport-host\""},{"line_number":5,"context_line":"    is now recognized by ML2/OVN and is used to identify nodes that are"},{"line_number":6,"context_line":"    eligible for scheduling OVN\u0027s external ports. This feature is backward"},{"line_number":7,"context_line":"    compatible and if no nodes contain this new option is found the external"}],"source_content_type":"text/x-yaml","patch_set":12,"id":"12776913_e17701eb","line":4,"range":{"start_line":4,"start_character":41,"end_line":4,"end_character":71},"updated":"2023-11-03 15:56:16.000000000","message":"nit: can you put this in `` `` so it\u0027s bold in the release notes? Same with as-gw below.","commit_id":"7f021319ac4487dbf7bac1fa74f4e38b5630e3a2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"aa4cfd4cb18e24babb26664ccee4d3c90ee245bb","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    A new ovn-cms-options option called \"enable-chassis-as-extport-host\""},{"line_number":5,"context_line":"    is now recognized by ML2/OVN and is used to identify nodes that are"},{"line_number":6,"context_line":"    eligible for scheduling OVN\u0027s external ports. This feature is backward"},{"line_number":7,"context_line":"    compatible and if no nodes contain this new option is found the external"}],"source_content_type":"text/x-yaml","patch_set":12,"id":"db219df8_e2df400f","line":4,"range":{"start_line":4,"start_character":41,"end_line":4,"end_character":71},"in_reply_to":"12776913_e17701eb","updated":"2023-11-03 16:26:50.000000000","message":"Done","commit_id":"7f021319ac4487dbf7bac1fa74f4e38b5630e3a2"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"10fb8700d93ec0eff20c9fd12a17f10f6932a5bd","unresolved":true,"context_lines":[{"line_number":4,"context_line":"    A new ovn-cms-options option called \"enable-chassis-as-extport-host\""},{"line_number":5,"context_line":"    is now recognized by ML2/OVN and is used to identify nodes that are"},{"line_number":6,"context_line":"    eligible for scheduling OVN\u0027s external ports. This feature is backward"},{"line_number":7,"context_line":"    compatible and if no nodes contain this new option is found the external"},{"line_number":8,"context_line":"    ports will continue to be scheduled using the \"enable-chassis-as-gw\""},{"line_number":9,"context_line":"    option as before."},{"line_number":10,"context_line":"    This change also introduced a limit to the number of members for each"}],"source_content_type":"text/x-yaml","patch_set":12,"id":"ecc42b73_9ee40708","line":7,"range":{"start_line":7,"start_character":44,"end_line":7,"end_character":76},"updated":"2023-11-03 15:56:16.000000000","message":"s/new option the external","commit_id":"7f021319ac4487dbf7bac1fa74f4e38b5630e3a2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"aa4cfd4cb18e24babb26664ccee4d3c90ee245bb","unresolved":false,"context_lines":[{"line_number":4,"context_line":"    A new ovn-cms-options option called \"enable-chassis-as-extport-host\""},{"line_number":5,"context_line":"    is now recognized by ML2/OVN and is used to identify nodes that are"},{"line_number":6,"context_line":"    eligible for scheduling OVN\u0027s external ports. This feature is backward"},{"line_number":7,"context_line":"    compatible and if no nodes contain this new option is found the external"},{"line_number":8,"context_line":"    ports will continue to be scheduled using the \"enable-chassis-as-gw\""},{"line_number":9,"context_line":"    option as before."},{"line_number":10,"context_line":"    This change also introduced a limit to the number of members for each"}],"source_content_type":"text/x-yaml","patch_set":12,"id":"e288061b_2693753f","line":7,"range":{"start_line":7,"start_character":44,"end_line":7,"end_character":76},"in_reply_to":"ecc42b73_9ee40708","updated":"2023-11-03 16:26:50.000000000","message":"Done","commit_id":"7f021319ac4487dbf7bac1fa74f4e38b5630e3a2"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"10fb8700d93ec0eff20c9fd12a17f10f6932a5bd","unresolved":true,"context_lines":[{"line_number":7,"context_line":"    compatible and if no nodes contain this new option is found the external"},{"line_number":8,"context_line":"    ports will continue to be scheduled using the \"enable-chassis-as-gw\""},{"line_number":9,"context_line":"    option as before."},{"line_number":10,"context_line":"    This change also introduced a limit to the number of members for each"},{"line_number":11,"context_line":"    HA Chassis Group to 5, matching the limit of gateway router ports"},{"line_number":12,"context_line":"    replicas. This is because OVN uses BFD to monitor the connectivity"},{"line_number":13,"context_line":"    of each member and having an unlimited number of members could"}],"source_content_type":"text/x-yaml","patch_set":12,"id":"8026dbf4_6c0fecd3","line":10,"range":{"start_line":10,"start_character":21,"end_line":10,"end_character":31},"updated":"2023-11-03 15:56:16.000000000","message":"s/introduces","commit_id":"7f021319ac4487dbf7bac1fa74f4e38b5630e3a2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"aa4cfd4cb18e24babb26664ccee4d3c90ee245bb","unresolved":false,"context_lines":[{"line_number":7,"context_line":"    compatible and if no nodes contain this new option is found the external"},{"line_number":8,"context_line":"    ports will continue to be scheduled using the \"enable-chassis-as-gw\""},{"line_number":9,"context_line":"    option as before."},{"line_number":10,"context_line":"    This change also introduced a limit to the number of members for each"},{"line_number":11,"context_line":"    HA Chassis Group to 5, matching the limit of gateway router ports"},{"line_number":12,"context_line":"    replicas. This is because OVN uses BFD to monitor the connectivity"},{"line_number":13,"context_line":"    of each member and having an unlimited number of members could"}],"source_content_type":"text/x-yaml","patch_set":12,"id":"cd8f24a6_0108447b","line":10,"range":{"start_line":10,"start_character":21,"end_line":10,"end_character":31},"in_reply_to":"8026dbf4_6c0fecd3","updated":"2023-11-03 16:26:50.000000000","message":"Done","commit_id":"7f021319ac4487dbf7bac1fa74f4e38b5630e3a2"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"10fb8700d93ec0eff20c9fd12a17f10f6932a5bd","unresolved":true,"context_lines":[{"line_number":8,"context_line":"    ports will continue to be scheduled using the \"enable-chassis-as-gw\""},{"line_number":9,"context_line":"    option as before."},{"line_number":10,"context_line":"    This change also introduced a limit to the number of members for each"},{"line_number":11,"context_line":"    HA Chassis Group to 5, matching the limit of gateway router ports"},{"line_number":12,"context_line":"    replicas. This is because OVN uses BFD to monitor the connectivity"},{"line_number":13,"context_line":"    of each member and having an unlimited number of members could"},{"line_number":14,"context_line":"    potentially put a lot of stress in OVN."}],"source_content_type":"text/x-yaml","patch_set":12,"id":"ce142d26_67a0c8b8","line":11,"range":{"start_line":11,"start_character":64,"end_line":11,"end_character":69},"updated":"2023-11-03 15:56:16.000000000","message":"s/port ?","commit_id":"7f021319ac4487dbf7bac1fa74f4e38b5630e3a2"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"aa4cfd4cb18e24babb26664ccee4d3c90ee245bb","unresolved":false,"context_lines":[{"line_number":8,"context_line":"    ports will continue to be scheduled using the \"enable-chassis-as-gw\""},{"line_number":9,"context_line":"    option as before."},{"line_number":10,"context_line":"    This change also introduced a limit to the number of members for each"},{"line_number":11,"context_line":"    HA Chassis Group to 5, matching the limit of gateway router ports"},{"line_number":12,"context_line":"    replicas. This is because OVN uses BFD to monitor the connectivity"},{"line_number":13,"context_line":"    of each member and having an unlimited number of members could"},{"line_number":14,"context_line":"    potentially put a lot of stress in OVN."}],"source_content_type":"text/x-yaml","patch_set":12,"id":"93f3b9f4_119ec9d5","line":11,"range":{"start_line":11,"start_character":64,"end_line":11,"end_character":69},"in_reply_to":"ce142d26_67a0c8b8","updated":"2023-11-03 16:26:50.000000000","message":"Done","commit_id":"7f021319ac4487dbf7bac1fa74f4e38b5630e3a2"}]}