)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"fc2a66868e51db3f0ce17ef0c3dbeb535b1cb8fc","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Roberto Bartzen Acosta \u003crbartzen@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2023-10-31 16:05:10 -0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[ML2/OVN] Add gateway_port support for FIP"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The OVN changed support for NAT rules including a new column and auto discovery logic (which may not work in some cases) [1][2]."},{"line_number":10,"context_line":"If the OVN backend supports this column in the Northbound DB Schema, set gateway port uuid to any floating IP to prevent North/South traffic issues for floating IPs."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"bb81d0c7_98c9bd28","line":7,"updated":"2023-11-15 10:04:17.000000000","message":"Can we add a release note for this ?","commit_id":"f74bc27794d48f89a3221831508852ff0059395f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"8105fe459eac211874de61177383959733c68766","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Roberto Bartzen Acosta \u003crbartzen@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2023-10-31 16:05:10 -0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[ML2/OVN] Add gateway_port support for FIP"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The OVN changed support for NAT rules including a new column and auto discovery logic (which may not work in some cases) [1][2]."},{"line_number":10,"context_line":"If the OVN backend supports this column in the Northbound DB Schema, set gateway port uuid to any floating IP to prevent North/South traffic issues for floating IPs."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"3a26c90e_d293da12","line":7,"in_reply_to":"9b67d7c3_0656ccba","updated":"2023-11-17 14:59:15.000000000","message":"Cool, looking forward to the update. Other than that this is a +2 for me. Thanks!","commit_id":"f74bc27794d48f89a3221831508852ff0059395f"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"b02d2920c711a9a2e7c0575931cf8950c78a7c39","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Roberto Bartzen Acosta \u003crbartzen@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2023-10-31 16:05:10 -0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[ML2/OVN] Add gateway_port support for FIP"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The OVN changed support for NAT rules including a new column and auto discovery logic (which may not work in some cases) [1][2]."},{"line_number":10,"context_line":"If the OVN backend supports this column in the Northbound DB Schema, set gateway port uuid to any floating IP to prevent North/South traffic issues for floating IPs."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9b67d7c3_0656ccba","line":7,"in_reply_to":"bb81d0c7_98c9bd28","updated":"2023-11-17 12:23:08.000000000","message":"Of course, I\u0027m adding a release note. Thanks for your review Lucas.","commit_id":"f74bc27794d48f89a3221831508852ff0059395f"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":32586,"name":"Elvira García Ruiz","display_name":"Elvira","email":"egarciar@redhat.com","username":"elvira"},"change_message_id":"27a4474211d4db483973971f9bf2a3bbeb09e140","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d2d97506_ecad36d9","updated":"2023-09-20 11:18:06.000000000","message":"LGTM! Thanks","commit_id":"aa0aa023a616b876b20154c328b1224052cc4850"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"e344ccdc8a3e3926fc88a37c61ac81f15ea8e1e4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"ab389e7e_a1b48393","updated":"2023-09-18 15:19:06.000000000","message":"recheck: unrelated failure","commit_id":"aa0aa023a616b876b20154c328b1224052cc4850"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"4258513e629cc9a23a7b5b5ba3bb90207976663f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"7529d438_f9aee112","updated":"2023-10-24 11:22:47.000000000","message":"recheck neutron-functional-with-uwsgi","commit_id":"11092a2cd8168528547a25da1575d207a2dec7b2"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"1b1b0e1e7e2b9b7fea4ef04dfa4538e0198f321a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"1f54454a_988f4a8c","updated":"2023-10-24 19:47:57.000000000","message":"recheck unrelated functional failure","commit_id":"11092a2cd8168528547a25da1575d207a2dec7b2"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"40d2a15e91bb76a302e28f4f5ec79e55ee0d42c7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"05f59fa1_0c2d95a7","updated":"2023-11-15 01:30:08.000000000","message":"I think this looks Ok, just adding Lucas to get more eyes on it.","commit_id":"f74bc27794d48f89a3221831508852ff0059395f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"fc2a66868e51db3f0ce17ef0c3dbeb535b1cb8fc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"3a2afa96_f6c609ee","updated":"2023-11-15 10:04:17.000000000","message":"Thanks for this patch, the code looks great and clean!\n\nThis is a soft -1, I think it would be good to have a release note for this and I left another nit in the code. Other than that it\u0027s a +2 from me.\n\nI also added Ihar as a reviewer as he might know better about this core OVN feature and the use here.","commit_id":"f74bc27794d48f89a3221831508852ff0059395f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"2a5f6bd0e7f815c02e726d06d93edbab23a6d093","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"a373ff6d_a6ea7a73","updated":"2023-11-20 08:50:22.000000000","message":"recheck\n\nfailures are not related to the patch","commit_id":"78b5fe2ff417e176a3ccc356d9428287a0ee3c64"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"295146c5c77babc4f454d09f553121afde375b96","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"20d51ac6_135fa347","updated":"2023-11-20 13:58:30.000000000","message":"recheck openstack-tox-py311-with-sqlalchemy-master","commit_id":"78b5fe2ff417e176a3ccc356d9428287a0ee3c64"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"b77249fea570db14b1e49346b4451213bf6588b8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"bd00ab27_5ff14097","updated":"2023-11-21 15:33:11.000000000","message":"recheck unrelated failures","commit_id":"78b5fe2ff417e176a3ccc356d9428287a0ee3c64"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"fc2a66868e51db3f0ce17ef0c3dbeb535b1cb8fc","unresolved":true,"context_lines":[{"line_number":858,"context_line":"        return self.db_set(\"NB_Global\", \".\", options\u003doptions)"},{"line_number":859,"context_line":""},{"line_number":860,"context_line":"    def is_nat_gateway_port_supported(self):"},{"line_number":861,"context_line":"        return self.is_col_present(\u0027NAT\u0027, \u0027gateway_port\u0027)"},{"line_number":862,"context_line":""},{"line_number":863,"context_line":""},{"line_number":864,"context_line":"class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend):"}],"source_content_type":"text/x-python","patch_set":8,"id":"c882c951_98325fed","line":861,"updated":"2023-11-15 10:04:17.000000000","message":"Can we move this method to the utils.py file ? We already have another helper function like this one there, it\u0027s better to keep them in one place as these are temporary functions that should be removed when we stop supporting older OVN versions that does not contain these columns\n\nHere:\nhttps://github.com/openstack/neutron/blob/5f2777f1374d92a0fe421e51c3e9359198e5f6f2/neutron/common/ovn/utils.py#L1127-L1128","commit_id":"f74bc27794d48f89a3221831508852ff0059395f"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"1bccb6188651b49a3f94b2ea5d0aeaab7a8e7b56","unresolved":false,"context_lines":[{"line_number":858,"context_line":"        return self.db_set(\"NB_Global\", \".\", options\u003doptions)"},{"line_number":859,"context_line":""},{"line_number":860,"context_line":"    def is_nat_gateway_port_supported(self):"},{"line_number":861,"context_line":"        return self.is_col_present(\u0027NAT\u0027, \u0027gateway_port\u0027)"},{"line_number":862,"context_line":""},{"line_number":863,"context_line":""},{"line_number":864,"context_line":"class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend):"}],"source_content_type":"text/x-python","patch_set":8,"id":"6173995e_dc043683","line":861,"in_reply_to":"247f8b9f_3c2092fb","updated":"2023-11-17 19:57:49.000000000","message":"Done","commit_id":"f74bc27794d48f89a3221831508852ff0059395f"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"b02d2920c711a9a2e7c0575931cf8950c78a7c39","unresolved":true,"context_lines":[{"line_number":858,"context_line":"        return self.db_set(\"NB_Global\", \".\", options\u003doptions)"},{"line_number":859,"context_line":""},{"line_number":860,"context_line":"    def is_nat_gateway_port_supported(self):"},{"line_number":861,"context_line":"        return self.is_col_present(\u0027NAT\u0027, \u0027gateway_port\u0027)"},{"line_number":862,"context_line":""},{"line_number":863,"context_line":""},{"line_number":864,"context_line":"class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend):"}],"source_content_type":"text/x-python","patch_set":8,"id":"f261f076_1619391b","line":861,"in_reply_to":"c882c951_98325fed","updated":"2023-11-17 12:23:08.000000000","message":"Sure, I can move it but this new function in utils.py was included last week [1] after my branch was created (I will need to merge it). I had placed it in IDL because I thought it would be the ideal place for these OVSDB IDL purposes and there is also the is_col_present helper there.\n\n[1] https://opendev.org/openstack/neutron/commit/26f173423986ca6c110ee517bae8d5db844d6ad6","commit_id":"f74bc27794d48f89a3221831508852ff0059395f"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"1febfbba8fdabd3bc8ff3d4d7b494e91c6459aa2","unresolved":true,"context_lines":[{"line_number":858,"context_line":"        return self.db_set(\"NB_Global\", \".\", options\u003doptions)"},{"line_number":859,"context_line":""},{"line_number":860,"context_line":"    def is_nat_gateway_port_supported(self):"},{"line_number":861,"context_line":"        return self.is_col_present(\u0027NAT\u0027, \u0027gateway_port\u0027)"},{"line_number":862,"context_line":""},{"line_number":863,"context_line":""},{"line_number":864,"context_line":"class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend):"}],"source_content_type":"text/x-python","patch_set":8,"id":"247f8b9f_3c2092fb","line":861,"in_reply_to":"f261f076_1619391b","updated":"2023-11-17 14:58:51.000000000","message":"Yeah, it\u0027s just to keep things tidy. These methods are ephemeral and they will be deleted in the future so better keep it all in one place IMHO.","commit_id":"f74bc27794d48f89a3221831508852ff0059395f"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py":[{"author":{"_account_id":32586,"name":"Elvira García Ruiz","display_name":"Elvira","email":"egarciar@redhat.com","username":"elvira"},"change_message_id":"27a4474211d4db483973971f9bf2a3bbeb09e140","unresolved":true,"context_lines":[{"line_number":1134,"context_line":"        for router in lrouters:"},{"line_number":1135,"context_line":"            ovn_fips \u003d router[\u0027dnat_and_snats\u0027]"},{"line_number":1136,"context_line":"            for ovn_fip in ovn_fips:"},{"line_number":1137,"context_line":"                # Skip FIPs that have already configured with gateway_port"},{"line_number":1138,"context_line":"                if ovn_fip[\u0027gateway_port\u0027]:"},{"line_number":1139,"context_line":"                    continue"},{"line_number":1140,"context_line":"                fip_id \u003d ovn_fip[\u0027external_ids\u0027].get("}],"source_content_type":"text/x-python","patch_set":3,"id":"5c8b306d_1df45c07","line":1137,"range":{"start_line":1137,"start_character":33,"end_line":1137,"end_character":37},"updated":"2023-09-20 11:18:06.000000000","message":"Just if you need to respin: \"are\" instead of have ?","commit_id":"aa0aa023a616b876b20154c328b1224052cc4850"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"63147c7a2c7960db2bb2a987f9f6b7c6ce6216ab","unresolved":false,"context_lines":[{"line_number":1134,"context_line":"        for router in lrouters:"},{"line_number":1135,"context_line":"            ovn_fips \u003d router[\u0027dnat_and_snats\u0027]"},{"line_number":1136,"context_line":"            for ovn_fip in ovn_fips:"},{"line_number":1137,"context_line":"                # Skip FIPs that have already configured with gateway_port"},{"line_number":1138,"context_line":"                if ovn_fip[\u0027gateway_port\u0027]:"},{"line_number":1139,"context_line":"                    continue"},{"line_number":1140,"context_line":"                fip_id \u003d ovn_fip[\u0027external_ids\u0027].get("}],"source_content_type":"text/x-python","patch_set":3,"id":"7bfb5dbd_bfd8a3dc","line":1137,"range":{"start_line":1137,"start_character":33,"end_line":1137,"end_character":37},"in_reply_to":"5c8b306d_1df45c07","updated":"2023-10-20 14:39:45.000000000","message":"sounds good to me, thanks.","commit_id":"aa0aa023a616b876b20154c328b1224052cc4850"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"1b1b0e1e7e2b9b7fea4ef04dfa4538e0198f321a","unresolved":true,"context_lines":[{"line_number":1122,"context_line":"            lrouter \u003d utils.ovn_name(fip[\u0027router_id\u0027])"},{"line_number":1123,"context_line":"            router_db \u003d self._ovn_client._l3_plugin.get_router(context,"},{"line_number":1124,"context_line":"                fip[\u0027router_id\u0027])"},{"line_number":1125,"context_line":"            gw_port_id \u003d router_db.get(\u0027gw_port_id\u0027)"},{"line_number":1126,"context_line":"            lrp \u003d self._nb_idl.get_lrouter_port(gw_port_id)"},{"line_number":1127,"context_line":"            columns \u003d {\u0027gateway_port\u0027: lrp.uuid}"},{"line_number":1128,"context_line":"            cmds.append(self._nb_idl.set_nat_rule_in_lrouter(lrouter,"}],"source_content_type":"text/x-python","patch_set":5,"id":"58b8ac6e_53e671b7","line":1125,"updated":"2023-10-24 19:47:57.000000000","message":"Just trying to understand how many times this might be making a DB call to get the same router object. We could cache it, or could always move this to the above loop. I realize it only ones once per startup, but could it find things every time?","commit_id":"11092a2cd8168528547a25da1575d207a2dec7b2"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"31d8cdb249982557ed6706932bc91d2c71811d05","unresolved":true,"context_lines":[{"line_number":1122,"context_line":"            lrouter \u003d utils.ovn_name(fip[\u0027router_id\u0027])"},{"line_number":1123,"context_line":"            router_db \u003d self._ovn_client._l3_plugin.get_router(context,"},{"line_number":1124,"context_line":"                fip[\u0027router_id\u0027])"},{"line_number":1125,"context_line":"            gw_port_id \u003d router_db.get(\u0027gw_port_id\u0027)"},{"line_number":1126,"context_line":"            lrp \u003d self._nb_idl.get_lrouter_port(gw_port_id)"},{"line_number":1127,"context_line":"            columns \u003d {\u0027gateway_port\u0027: lrp.uuid}"},{"line_number":1128,"context_line":"            cmds.append(self._nb_idl.set_nat_rule_in_lrouter(lrouter,"}],"source_content_type":"text/x-python","patch_set":5,"id":"608de52e_8ed83919","line":1125,"in_reply_to":"58b8ac6e_53e671b7","updated":"2023-10-25 12:14:45.000000000","message":"gotcha! I like these performance questions!\nIMO it depends on how many FIPs are allocated to each router. I mean, if we have more routers than FIPs per router, it doesn\u0027t make sense to put them in the above loop as unnecessary requests would be made (assuming thousands of routers in the DB, this would be very hard). But if we use the DB requests at the currenty location, and a router uses more than one FIP to its VMs, it can become inefficient when requesting multiple calls to the same router. \n\nPerhaps a cache for the router object in the FIPs loop might be more interesting for performance purposes.\n\nDoes this make sense to you?","commit_id":"11092a2cd8168528547a25da1575d207a2dec7b2"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"13503c31ff9a0c33fdaa7e37bf318db9ec885b7f","unresolved":true,"context_lines":[{"line_number":1122,"context_line":"            lrouter \u003d utils.ovn_name(fip[\u0027router_id\u0027])"},{"line_number":1123,"context_line":"            router_db \u003d self._ovn_client._l3_plugin.get_router(context,"},{"line_number":1124,"context_line":"                fip[\u0027router_id\u0027])"},{"line_number":1125,"context_line":"            gw_port_id \u003d router_db.get(\u0027gw_port_id\u0027)"},{"line_number":1126,"context_line":"            lrp \u003d self._nb_idl.get_lrouter_port(gw_port_id)"},{"line_number":1127,"context_line":"            columns \u003d {\u0027gateway_port\u0027: lrp.uuid}"},{"line_number":1128,"context_line":"            cmds.append(self._nb_idl.set_nat_rule_in_lrouter(lrouter,"}],"source_content_type":"text/x-python","patch_set":5,"id":"8f5b396a_1cfedb48","line":1125,"in_reply_to":"608de52e_8ed83919","updated":"2023-10-25 20:54:44.000000000","message":"Right, if doing in the above loop we\u0027d want to filter the FIPs to at least lessen the possible number of requests, something like:\n\novn_fips \u003d [ovn_fip for ovn_fip in ovn_fips if not ovn_fip[\u0027gateway_port\u0027]]\nrouter db lookup, etc\nfor ovn_fip in ovn_fips:\n...\n\nIt would be easier to just cache the gw_port_id here, indexed by router_id.\n\nAlso, since all you ever care about is gw_port_id, I think you can pass that to get_router() with fields\u003d[\u0027gw_port_id\u0027] to limit the dict returned. It\u0027s not done much but the signature seems to support it.","commit_id":"11092a2cd8168528547a25da1575d207a2dec7b2"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"95e1f4195534c1ffab3cc061705b01a35a9c34b6","unresolved":false,"context_lines":[{"line_number":1122,"context_line":"            lrouter \u003d utils.ovn_name(fip[\u0027router_id\u0027])"},{"line_number":1123,"context_line":"            router_db \u003d self._ovn_client._l3_plugin.get_router(context,"},{"line_number":1124,"context_line":"                fip[\u0027router_id\u0027])"},{"line_number":1125,"context_line":"            gw_port_id \u003d router_db.get(\u0027gw_port_id\u0027)"},{"line_number":1126,"context_line":"            lrp \u003d self._nb_idl.get_lrouter_port(gw_port_id)"},{"line_number":1127,"context_line":"            columns \u003d {\u0027gateway_port\u0027: lrp.uuid}"},{"line_number":1128,"context_line":"            cmds.append(self._nb_idl.set_nat_rule_in_lrouter(lrouter,"}],"source_content_type":"text/x-python","patch_set":5,"id":"a8c9a0c1_ade4896a","line":1125,"in_reply_to":"8f5b396a_1cfedb48","updated":"2023-10-31 13:52:45.000000000","message":"Yeap, cache the gw_port_id and filter the get_router query by fields sounds good to me ;) thanks for the review!","commit_id":"11092a2cd8168528547a25da1575d207a2dec7b2"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py":[{"author":{"_account_id":29074,"name":"Felix Huettner","email":"felix.huettner@digits.schwarz","username":"felix.huettner"},"change_message_id":"2669f1364222e0b37833d122a967e79fa99a6234","unresolved":true,"context_lines":[{"line_number":843,"context_line":"            router_db \u003d self._l3_plugin.get_router(admin_context, router_id)"},{"line_number":844,"context_line":"            if router_db:"},{"line_number":845,"context_line":"                gw_port_id \u003d router_db.get(\u0027gw_port_id\u0027)"},{"line_number":846,"context_line":"                if gw_port_id:"},{"line_number":847,"context_line":"                    lrp \u003d self._nb_idl.get_lrouter_port(gw_port_id)"},{"line_number":848,"context_line":"                    columns[\u0027gateway_port\u0027] \u003d lrp.uuid"},{"line_number":849,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"0ede5693_b01aadf5","line":846,"updated":"2023-09-18 10:09:03.000000000","message":"I\u0027m not sure how good or bad it is to guard `router_db` and `gw_port_id` with a if condition.\nI see no way to have a active floating ip while having no router with a gateway port.\n\nSo i see that at least as quite concerning and would add logs for that, but from my perspective failing is also valid","commit_id":"08c025f6f6f05a5e5d20b42b0db2ebece0fa782c"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"e0b3148a328aa96521a4a2b9ef47ae5371c7b33a","unresolved":false,"context_lines":[{"line_number":843,"context_line":"            router_db \u003d self._l3_plugin.get_router(admin_context, router_id)"},{"line_number":844,"context_line":"            if router_db:"},{"line_number":845,"context_line":"                gw_port_id \u003d router_db.get(\u0027gw_port_id\u0027)"},{"line_number":846,"context_line":"                if gw_port_id:"},{"line_number":847,"context_line":"                    lrp \u003d self._nb_idl.get_lrouter_port(gw_port_id)"},{"line_number":848,"context_line":"                    columns[\u0027gateway_port\u0027] \u003d lrp.uuid"},{"line_number":849,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"b96e9518_55ecebe3","line":846,"in_reply_to":"0ede5693_b01aadf5","updated":"2023-09-18 12:29:20.000000000","message":"Hi Felix, thanks for your feedback. I agree with you, these conditions would not be valid in normal operation (only if something very strange happens with the Neutron database). \nI was only able to simulate the guard use case by generating bug configs in unit tests, but this concern seems very excessive.\n\nI will remove it, thanks.","commit_id":"08c025f6f6f05a5e5d20b42b0db2ebece0fa782c"}]}