)]}'
{"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0d17dd45895d37aae232b38994ae73bb42154522","unresolved":true,"context_lines":[{"line_number":1519,"context_line":"                    cidr \u003d subnet[\u0027cidr\u0027]"},{"line_number":1520,"context_line":"                    break"},{"line_number":1521,"context_line":""},{"line_number":1522,"context_line":"            if router and utils.is_snat_enabled(router) and cidr:"},{"line_number":1523,"context_line":"                self.update_nat_rules("},{"line_number":1524,"context_line":"                    router, networks\u003d[cidr], enable_snat\u003dFalse, txn\u003dtxn)"},{"line_number":1525,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"658a11ba_3208d5cc","line":1522,"range":{"start_line":1522,"start_character":15,"end_line":1522,"end_character":21},"updated":"2021-03-24 13:14:09.000000000","message":"Looks like this can go away as we would have returned early on L1503","commit_id":"60356ba6c80c29e1c49856a84daf5adcb8a012e4"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"9f7255040b54459bafa405e7d3573aeae54b8969","unresolved":true,"context_lines":[{"line_number":1519,"context_line":"                    cidr \u003d subnet[\u0027cidr\u0027]"},{"line_number":1520,"context_line":"                    break"},{"line_number":1521,"context_line":""},{"line_number":1522,"context_line":"            if router and utils.is_snat_enabled(router) and cidr:"},{"line_number":1523,"context_line":"                self.update_nat_rules("},{"line_number":1524,"context_line":"                    router, networks\u003d[cidr], enable_snat\u003dFalse, txn\u003dtxn)"},{"line_number":1525,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"57aba383_50bb5cb4","line":1522,"range":{"start_line":1522,"start_character":15,"end_line":1522,"end_character":21},"in_reply_to":"658a11ba_3208d5cc","updated":"2021-03-24 13:25:24.000000000","message":"Good point, indeed!","commit_id":"60356ba6c80c29e1c49856a84daf5adcb8a012e4"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0d17dd45895d37aae232b38994ae73bb42154522","unresolved":true,"context_lines":[{"line_number":1527,"context_line":"            # delete the router port as the last operation and update the"},{"line_number":1528,"context_line":"            # revision database to ensure consistency"},{"line_number":1529,"context_line":"            if port_removed:"},{"line_number":1530,"context_line":"                self._delete_lrouter_port(context, port_id, router_id, txn\u003dtxn)"},{"line_number":1531,"context_line":"            else:"},{"line_number":1532,"context_line":"                # otherwise, we just update the revision database"},{"line_number":1533,"context_line":"                db_rev.bump_revision("}],"source_content_type":"text/x-python","patch_set":2,"id":"93e53e67_aef97f46","line":1530,"updated":"2021-03-24 13:14:09.000000000","message":"Can this still trigger?  Seems it would always trigger at L1501 now based on the changes, but I could be missing something.","commit_id":"60356ba6c80c29e1c49856a84daf5adcb8a012e4"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"d0ef008cd1587ebe67238d64efa9ff3368d8de7f","unresolved":true,"context_lines":[{"line_number":1527,"context_line":"            # delete the router port as the last operation and update the"},{"line_number":1528,"context_line":"            # revision database to ensure consistency"},{"line_number":1529,"context_line":"            if port_removed:"},{"line_number":1530,"context_line":"                self._delete_lrouter_port(context, port_id, router_id, txn\u003dtxn)"},{"line_number":1531,"context_line":"            else:"},{"line_number":1532,"context_line":"                # otherwise, we just update the revision database"},{"line_number":1533,"context_line":"                db_rev.bump_revision("}],"source_content_type":"text/x-python","patch_set":2,"id":"77f036b5_3a52c965","line":1530,"in_reply_to":"0eab20db_c8697f1c","updated":"2021-03-24 14:13:35.000000000","message":"I think spaghetti was the correct word for this code :)  And I guess you have to wait until here since you might have to update the SNAT rules.","commit_id":"60356ba6c80c29e1c49856a84daf5adcb8a012e4"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"9f7255040b54459bafa405e7d3573aeae54b8969","unresolved":true,"context_lines":[{"line_number":1527,"context_line":"            # delete the router port as the last operation and update the"},{"line_number":1528,"context_line":"            # revision database to ensure consistency"},{"line_number":1529,"context_line":"            if port_removed:"},{"line_number":1530,"context_line":"                self._delete_lrouter_port(context, port_id, router_id, txn\u003dtxn)"},{"line_number":1531,"context_line":"            else:"},{"line_number":1532,"context_line":"                # otherwise, we just update the revision database"},{"line_number":1533,"context_line":"                db_rev.bump_revision("}],"source_content_type":"text/x-python","patch_set":2,"id":"99d5ddd6_28d0bdf4","line":1530,"in_reply_to":"93e53e67_aef97f46","updated":"2021-03-24 13:25:24.000000000","message":"Another good point, I looked at it a few times but yeah, apparently there\u0027s no way to get to this when port_removed is now True.\n\nI will get rid of this conditional. Which is good cause the logic in this method is getting super spaghetty!","commit_id":"60356ba6c80c29e1c49856a84daf5adcb8a012e4"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0a7e8e615f389f4019e392d05321eaa2df2e6957","unresolved":true,"context_lines":[{"line_number":1527,"context_line":"            # delete the router port as the last operation and update the"},{"line_number":1528,"context_line":"            # revision database to ensure consistency"},{"line_number":1529,"context_line":"            if port_removed:"},{"line_number":1530,"context_line":"                self._delete_lrouter_port(context, port_id, router_id, txn\u003dtxn)"},{"line_number":1531,"context_line":"            else:"},{"line_number":1532,"context_line":"                # otherwise, we just update the revision database"},{"line_number":1533,"context_line":"                db_rev.bump_revision("}],"source_content_type":"text/x-python","patch_set":2,"id":"0eab20db_c8697f1c","line":1530,"in_reply_to":"99d5ddd6_28d0bdf4","updated":"2021-03-24 13:46:11.000000000","message":"Oh no I was wrong, still possible if the port has been removed (PortNotFound raised on L1480) but the router is still present","commit_id":"60356ba6c80c29e1c49856a84daf5adcb8a012e4"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"ef8b8afdac3560f902b8c6743e828bfcc5784be8","unresolved":true,"context_lines":[{"line_number":1486,"context_line":"            router_id \u003d router_id or ovn_port.external_ids.get("},{"line_number":1487,"context_line":"                ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY)"},{"line_number":1488,"context_line":"            if port and not router_id:"},{"line_number":1489,"context_line":"                router_id \u003d port.get(\u0027device_id\u0027)"},{"line_number":1490,"context_line":""},{"line_number":1491,"context_line":"            router \u003d None"},{"line_number":1492,"context_line":"            if router_id:"}],"source_content_type":"text/x-python","patch_set":3,"id":"d3380c6f_7e2bc6db","line":1489,"updated":"2021-03-25 08:13:02.000000000","message":"if there can be case when there is port and no router_id, can it happen that router_id is not None and router_id !\u003d port[\u0027device_id\u0027] ? Just asking :)","commit_id":"36b8c684b1c42a0733d0820bcd2e1fe217f23797"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"dab227a8302450431c7b8a804d4244e1c62a1ad5","unresolved":true,"context_lines":[{"line_number":1486,"context_line":"            router_id \u003d router_id or ovn_port.external_ids.get("},{"line_number":1487,"context_line":"                ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY)"},{"line_number":1488,"context_line":"            if port and not router_id:"},{"line_number":1489,"context_line":"                router_id \u003d port.get(\u0027device_id\u0027)"},{"line_number":1490,"context_line":""},{"line_number":1491,"context_line":"            router \u003d None"},{"line_number":1492,"context_line":"            if router_id:"}],"source_content_type":"text/x-python","patch_set":3,"id":"e09a13a9_97c4bc12","line":1489,"in_reply_to":"d3380c6f_7e2bc6db","updated":"2021-03-25 09:11:49.000000000","message":"There\u0027s a case when the port can be set but no router_id because I think we added the router_id to the external_ids later. So many some old entry there hasn\u0027t been updated. \n\nNow all router ports created with the OVN driver will include the router_id in it\u0027s external_ids all the time.\n\nRegarding router_id !\u003d port[\u0027device_id], I do not think it\u0027s possible. Wnless as we said, router_id is None, but if it\u0027s set it should always match port[\u0027device_id\u0027].","commit_id":"36b8c684b1c42a0733d0820bcd2e1fe217f23797"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"35564213e0e5c3cec0d34a3e98ad7b436668cd98","unresolved":true,"context_lines":[{"line_number":1494,"context_line":"                    router \u003d self._l3_plugin.get_router(context, router_id)"},{"line_number":1495,"context_line":"                except l3_exc.RouterNotFound:"},{"line_number":1496,"context_line":"                    # If the router is gone, the router port is also gone"},{"line_number":1497,"context_line":"                    port_removed \u003d True"},{"line_number":1498,"context_line":""},{"line_number":1499,"context_line":"            if not router or not router.get(l3.EXTERNAL_GW_INFO):"},{"line_number":1500,"context_line":"                if port_removed:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9b4cf22a_893cd6fd","line":1497,"range":{"start_line":1497,"start_character":20,"end_line":1497,"end_character":39},"updated":"2021-03-24 14:35:21.000000000","message":"I find the port and port_removed variables redundant. Is there a scenario where port is not None while port_removed is True? Maybe it\u0027s not related to this patch","commit_id":"36b8c684b1c42a0733d0820bcd2e1fe217f23797"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"add96176c6c5e6bb320729f4c596286fdd5bd4bc","unresolved":true,"context_lines":[{"line_number":1494,"context_line":"                    router \u003d self._l3_plugin.get_router(context, router_id)"},{"line_number":1495,"context_line":"                except l3_exc.RouterNotFound:"},{"line_number":1496,"context_line":"                    # If the router is gone, the router port is also gone"},{"line_number":1497,"context_line":"                    port_removed \u003d True"},{"line_number":1498,"context_line":""},{"line_number":1499,"context_line":"            if not router or not router.get(l3.EXTERNAL_GW_INFO):"},{"line_number":1500,"context_line":"                if port_removed:"}],"source_content_type":"text/x-python","patch_set":3,"id":"4f05f49e_6b318882","line":1497,"range":{"start_line":1497,"start_character":20,"end_line":1497,"end_character":39},"in_reply_to":"14889d9a_bfaee226","updated":"2021-03-24 16:38:40.000000000","message":"I\u0027m fine doing a refactor later","commit_id":"36b8c684b1c42a0733d0820bcd2e1fe217f23797"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"1a3ef2dab6ba22f2f3c87a1926d03a617bcd2f9c","unresolved":true,"context_lines":[{"line_number":1494,"context_line":"                    router \u003d self._l3_plugin.get_router(context, router_id)"},{"line_number":1495,"context_line":"                except l3_exc.RouterNotFound:"},{"line_number":1496,"context_line":"                    # If the router is gone, the router port is also gone"},{"line_number":1497,"context_line":"                    port_removed \u003d True"},{"line_number":1498,"context_line":""},{"line_number":1499,"context_line":"            if not router or not router.get(l3.EXTERNAL_GW_INFO):"},{"line_number":1500,"context_line":"                if port_removed:"}],"source_content_type":"text/x-python","patch_set":3,"id":"14889d9a_bfaee226","line":1497,"range":{"start_line":1497,"start_character":20,"end_line":1497,"end_character":39},"in_reply_to":"9b4cf22a_893cd6fd","updated":"2021-03-24 15:04:04.000000000","message":"Hah true, yeah I don\u0027t think there\u0027s a scenario like that. To be honest it would be good to refactor this whole method to make it easier to read.\n\nI would rather do it in another patch to make the backport of this fix easier but, if u guys think it\u0027s better to do it in this patch itself I am can do it too.","commit_id":"36b8c684b1c42a0733d0820bcd2e1fe217f23797"}]}