)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8070946b7d95528ce9493a51ed2006312fff7fea","unresolved":false,"context_lines":[{"line_number":17,"context_line":"Depends-On: https://review.opendev.org/#/c/740993/"},{"line_number":18,"context_line":"Depends-On: https://review.opendev.org/#/c/741000/"},{"line_number":19,"context_line":"Partially-implements: ovn/port_forwarding"},{"line_number":20,"context_line":"Closes-Bug: #1877447"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I019fe11ac1ddcf2304f3f144c62d52667fc11dce"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"bf51134e_a3f85349","line":20,"updated":"2020-07-21 17:16:58.000000000","message":"Let me ask you, are we going to have more patches for this LP bug? If does, please add Partial-Bug","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"3ddcba87d8aa8e3a58f7746948113d4b3ca29c5a","unresolved":false,"context_lines":[{"line_number":17,"context_line":"Depends-On: https://review.opendev.org/#/c/740993/"},{"line_number":18,"context_line":"Depends-On: https://review.opendev.org/#/c/741000/"},{"line_number":19,"context_line":"Partially-implements: ovn/port_forwarding"},{"line_number":20,"context_line":"Closes-Bug: #1877447"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I019fe11ac1ddcf2304f3f144c62d52667fc11dce"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"9f560f44_5320eeb0","line":20,"in_reply_to":"bf51134e_3b9c51be","updated":"2020-07-31 13:08:18.000000000","message":"I broke this up... and made this partial-bug now. ;)","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":17,"context_line":"Depends-On: https://review.opendev.org/#/c/740993/"},{"line_number":18,"context_line":"Depends-On: https://review.opendev.org/#/c/741000/"},{"line_number":19,"context_line":"Partially-implements: ovn/port_forwarding"},{"line_number":20,"context_line":"Closes-Bug: #1877447"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I019fe11ac1ddcf2304f3f144c62d52667fc11dce"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"bf51134e_3b9c51be","line":20,"in_reply_to":"bf51134e_a3f85349","updated":"2020-07-22 22:56:20.000000000","message":"There are other partial patches [1], but this is the last one for lp 1877447.\n\n[1]: https://review.opendev.org/#/q/topic:ovn/port_forwarding+(status:open+OR+status:merged)","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"}],"neutron/common/ovn/utils.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"334ec66b8be9a69f935f49ea1787de4d9f3a0f26","unresolved":false,"context_lines":[{"line_number":554,"context_line":"    for ovn_lb in ovn_rtr_lb_pfs:"},{"line_number":555,"context_line":"        ext_ids \u003d ovn_lb.external_ids"},{"line_number":556,"context_line":"        fip_id \u003d ext_ids.get(constants.OVN_FIP_EXT_ID_KEY)"},{"line_number":557,"context_line":"        protocol \u003d (ovn_lb.protocol[0]"},{"line_number":558,"context_line":"                    if ovn_lb.protocol else ovsdbapp_const.PROTO_TCP)"},{"line_number":559,"context_line":"        fip_dict \u003d result.get(fip_id, {})"},{"line_number":560,"context_line":"        fip_dict_proto \u003d fip_dict.get(protocol, set())"},{"line_number":561,"context_line":"        ovn_vips \u003d ovn_lb.vips"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_67b7d587","line":558,"range":{"start_line":557,"start_character":20,"end_line":558,"end_character":68},"updated":"2020-08-06 22:12:10.000000000","message":"no change needed, just a note that if I need to do a \"non-destructively get the first item off a list, if the list isn\u0027t empty otherwise a default\", I just do something like:\n\n next(iter(ovn_lb.protocol), ovsdbapp_const.PROTO_TCP)\n\nnot sure it\u0027s better, just shorter. :p Crazily, this whole method can be done as a one-liner dict comprehension with nested dict and set comprehensions. (not recommended!)\n\n return {lb.external_ids[constants.OVN_FIP_EXT_ID_KEY]: {next(iter(lb.protocol), ovsdbapp_const.PROTO_TCP): {\"{} {}\".format(vip, ip) for vip, ips in lb    .vips.items() for ip in ips.split(\u0027,\u0027)}} for lb in ovn_rtr_lb_pfs}","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"7ceec569405fa973221129b27e71adac636e9e2e","unresolved":false,"context_lines":[{"line_number":554,"context_line":"    for ovn_lb in ovn_rtr_lb_pfs:"},{"line_number":555,"context_line":"        ext_ids \u003d ovn_lb.external_ids"},{"line_number":556,"context_line":"        fip_id \u003d ext_ids.get(constants.OVN_FIP_EXT_ID_KEY)"},{"line_number":557,"context_line":"        protocol \u003d (ovn_lb.protocol[0]"},{"line_number":558,"context_line":"                    if ovn_lb.protocol else ovsdbapp_const.PROTO_TCP)"},{"line_number":559,"context_line":"        fip_dict \u003d result.get(fip_id, {})"},{"line_number":560,"context_line":"        fip_dict_proto \u003d fip_dict.get(protocol, set())"},{"line_number":561,"context_line":"        ovn_vips \u003d ovn_lb.vips"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_511b5871","line":558,"range":{"start_line":557,"start_character":20,"end_line":558,"end_character":68},"in_reply_to":"9f560f44_67b7d587","updated":"2020-08-07 20:44:42.000000000","message":"Awesome. Noted!","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"eff5178ffd11d780dde628dbfc2b95a2bcb2c02a","unresolved":false,"context_lines":[{"line_number":753,"context_line":"                # balancer table and have well known name."},{"line_number":754,"context_line":"                if self.name.startswith(PORT_FORWARDING_PREFIX):"},{"line_number":755,"context_line":"                    ovn_table \u003d \u0027Load_Balancer\u0027"},{"line_number":756,"context_line":"                    ovn_resource \u003d self.api.lookup(ovn_table, self.name)"},{"line_number":757,"context_line":"                else:"},{"line_number":758,"context_line":"                    ovn_resource \u003d self._get_floatingip()"},{"line_number":759,"context_line":"            elif self.resource_type \u003d\u003d ovn_const.TYPE_SUBNETS:"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_51aa4f84","line":756,"updated":"2020-08-06 16:45:56.000000000","message":"nits:\n\n1) Use \"Load_Balancer\" directly in the call, instead of creating a new variable\n\n2) Implement this inside \"_get_floatingip\"\n\n3) call this method \"_get_floatingip_and_pf\"","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"f64a83230242f6073fe3e62c7145c199d6e4c881","unresolved":false,"context_lines":[{"line_number":753,"context_line":"                # balancer table and have well known name."},{"line_number":754,"context_line":"                if self.name.startswith(PORT_FORWARDING_PREFIX):"},{"line_number":755,"context_line":"                    ovn_table \u003d \u0027Load_Balancer\u0027"},{"line_number":756,"context_line":"                    ovn_resource \u003d self.api.lookup(ovn_table, self.name)"},{"line_number":757,"context_line":"                else:"},{"line_number":758,"context_line":"                    ovn_resource \u003d self._get_floatingip()"},{"line_number":759,"context_line":"            elif self.resource_type \u003d\u003d ovn_const.TYPE_SUBNETS:"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_a206fb50","line":756,"in_reply_to":"9f560f44_51aa4f84","updated":"2020-08-06 19:19:06.000000000","message":"Done","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"aa014fafb62486c0602a1f21fcdbebe2aa2288c5","unresolved":false,"context_lines":[{"line_number":709,"context_line":"        self.resource_type \u003d resource_type"},{"line_number":710,"context_line":"        self.if_exists \u003d if_exists"},{"line_number":711,"context_line":""},{"line_number":712,"context_line":"    def _get_floatingip_and_pf(self):"},{"line_number":713,"context_line":"        # TYPE_FLOATINGIPS: Determine table to use based on name."},{"line_number":714,"context_line":"        # Floating ip port forwarding resources are kept in load"},{"line_number":715,"context_line":"        # balancer table and have a well known name."}],"source_content_type":"text/x-python","patch_set":28,"id":"9f560f44_8b6bddef","line":712,"range":{"start_line":712,"start_character":24,"end_line":712,"end_character":27},"updated":"2020-08-11 15:27:07.000000000","message":"nit: (don\u0027t need to fix, just pointing it out)\n\nSounds more like an OR, when I read the method\u0027s name I thought it would return both the LB and the NAT row instances","commit_id":"3d3c0533062b07c5e9bdafa7768fd1a89bde82b7"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"257a9e5cb1fb3d9568692f098dc728375244b227","unresolved":false,"context_lines":[{"line_number":709,"context_line":"        self.resource_type \u003d resource_type"},{"line_number":710,"context_line":"        self.if_exists \u003d if_exists"},{"line_number":711,"context_line":""},{"line_number":712,"context_line":"    def _get_floatingip_and_pf(self):"},{"line_number":713,"context_line":"        # TYPE_FLOATINGIPS: Determine table to use based on name."},{"line_number":714,"context_line":"        # Floating ip port forwarding resources are kept in load"},{"line_number":715,"context_line":"        # balancer table and have a well known name."}],"source_content_type":"text/x-python","patch_set":28,"id":"9f560f44_263128a4","line":712,"range":{"start_line":712,"start_character":24,"end_line":712,"end_character":27},"in_reply_to":"9f560f44_8b6bddef","updated":"2020-08-11 18:55:14.000000000","message":"yeah, good point. That was the name suggested earlier on but I would like to improve that.\nDone.","commit_id":"3d3c0533062b07c5e9bdafa7768fd1a89bde82b7"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py":[{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":320,"context_line":"                            utils.ovn_name(port[\u0027network_id\u0027]),"},{"line_number":321,"context_line":"                        ovn_const.OVN_SG_IDS_EXT_ID_KEY:"},{"line_number":322,"context_line":"                            port_info.security_group_ids,"},{"line_number":323,"context_line":"                        ovn_const.OVN_REV_NUM_EXT_ID_KEY: str("},{"line_number":324,"context_line":"                            utils.get_revision_number("},{"line_number":325,"context_line":"                                port, ovn_const.TYPE_PORTS))}"},{"line_number":326,"context_line":"        lswitch_name \u003d utils.ovn_name(port[\u0027network_id\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_adfe6068","line":323,"updated":"2020-07-22 22:56:20.000000000","message":"Reference on how revision number is store in ovn db...","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"3ddcba87d8aa8e3a58f7746948113d4b3ca29c5a","unresolved":false,"context_lines":[{"line_number":320,"context_line":"                            utils.ovn_name(port[\u0027network_id\u0027]),"},{"line_number":321,"context_line":"                        ovn_const.OVN_SG_IDS_EXT_ID_KEY:"},{"line_number":322,"context_line":"                            port_info.security_group_ids,"},{"line_number":323,"context_line":"                        ovn_const.OVN_REV_NUM_EXT_ID_KEY: str("},{"line_number":324,"context_line":"                            utils.get_revision_number("},{"line_number":325,"context_line":"                                port, ovn_const.TYPE_PORTS))}"},{"line_number":326,"context_line":"        lswitch_name \u003d utils.ovn_name(port[\u0027network_id\u0027])"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f560f44_33157a4b","line":323,"in_reply_to":"bf51134e_adfe6068","updated":"2020-07-31 13:08:18.000000000","message":"https://review.opendev.org/#/c/740955/6/doc/source/ovn/port_forwarding.rst@79","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8070946b7d95528ce9493a51ed2006312fff7fea","unresolved":false,"context_lines":[{"line_number":871,"context_line":"            LOG.debug(\"FIP %s doesn\u0027t have external_ids.\", fip)"},{"line_number":872,"context_line":"        self._transaction(commands, txn\u003dtxn)"},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"    def _create_or_update_floatingip_pfs(self, context, fip_id, txn):"},{"line_number":875,"context_line":"        self._l3_plugin.port_forwarding.db_sync_create_or_update("},{"line_number":876,"context_line":"            context, fip_id, txn)"},{"line_number":877,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_c648f55d","line":874,"updated":"2020-07-21 17:16:58.000000000","message":"Those methods (create_update and delete) should be public","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":871,"context_line":"            LOG.debug(\"FIP %s doesn\u0027t have external_ids.\", fip)"},{"line_number":872,"context_line":"        self._transaction(commands, txn\u003dtxn)"},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"    def _create_or_update_floatingip_pfs(self, context, fip_id, txn):"},{"line_number":875,"context_line":"        self._l3_plugin.port_forwarding.db_sync_create_or_update("},{"line_number":876,"context_line":"            context, fip_id, txn)"},{"line_number":877,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_8681c4c6","line":874,"in_reply_to":"bf51134e_c648f55d","updated":"2020-07-22 22:56:20.000000000","message":"Done","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8070946b7d95528ce9493a51ed2006312fff7fea","unresolved":false,"context_lines":[{"line_number":879,"context_line":"        self._l3_plugin.port_forwarding.db_sync_delete("},{"line_number":880,"context_line":"            context, fip_id, txn)"},{"line_number":881,"context_line":""},{"line_number":882,"context_line":"    def _ovn_lb_protocol(self, pf_protocol):"},{"line_number":883,"context_line":"        return self._l3_plugin.port_forwarding.ovn_lb_protocol(pf_protocol)"},{"line_number":884,"context_line":""},{"line_number":885,"context_line":"    def update_floatingip_status(self, context, floatingip):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_a6450146","line":882,"updated":"2020-07-21 17:16:58.000000000","message":"and this one","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":879,"context_line":"        self._l3_plugin.port_forwarding.db_sync_delete("},{"line_number":880,"context_line":"            context, fip_id, txn)"},{"line_number":881,"context_line":""},{"line_number":882,"context_line":"    def _ovn_lb_protocol(self, pf_protocol):"},{"line_number":883,"context_line":"        return self._l3_plugin.port_forwarding.ovn_lb_protocol(pf_protocol)"},{"line_number":884,"context_line":""},{"line_number":885,"context_line":"    def update_floatingip_status(self, context, floatingip):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_26639843","line":882,"in_reply_to":"bf51134e_a6450146","updated":"2020-07-22 22:56:20.000000000","message":"Done","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8070946b7d95528ce9493a51ed2006312fff7fea","unresolved":false,"context_lines":[{"line_number":896,"context_line":"    def create_floatingip_and_pf(self, context, floatingip):"},{"line_number":897,"context_line":"        self.create_floatingip(context, floatingip)"},{"line_number":898,"context_line":"        self._l3_plugin.port_forwarding.maintenance_create("},{"line_number":899,"context_line":"            context, floatingip)"},{"line_number":900,"context_line":""},{"line_number":901,"context_line":"    def create_floatingip(self, context, floatingip):"},{"line_number":902,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_e62a597c","line":899,"updated":"2020-07-21 17:16:58.000000000","message":"Why do we need to call the maintenance task from the client?\n\nAre we going to call the maintenance task every time we create, update or delete a FIP?","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":896,"context_line":"    def create_floatingip_and_pf(self, context, floatingip):"},{"line_number":897,"context_line":"        self.create_floatingip(context, floatingip)"},{"line_number":898,"context_line":"        self._l3_plugin.port_forwarding.maintenance_create("},{"line_number":899,"context_line":"            context, floatingip)"},{"line_number":900,"context_line":""},{"line_number":901,"context_line":"    def create_floatingip(self, context, floatingip):"},{"line_number":902,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_46bccc6b","line":899,"in_reply_to":"bf51134e_e62a597c","updated":"2020-07-22 22:56:20.000000000","message":"Oh, definitely not calling maintenance task!\nIt is more like the other way around: this function is called by the maintenance task when checking the revision\nnumbers of the floating ips. Since ovn lb entries used for\nport forwarding use the revision numbers of their respective\nfloating ips [1] these two are combined here. Knowing that, do you propose the change the name of this function to something else?\n\n[1]: https://review.opendev.org/#/c/740955/5/doc/source/ovn/port_forwarding.rst@81","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"aa014fafb62486c0602a1f21fcdbebe2aa2288c5","unresolved":false,"context_lines":[{"line_number":871,"context_line":"            LOG.debug(\"FIP %s doesn\u0027t have external_ids.\", fip)"},{"line_number":872,"context_line":"        self._transaction(commands, txn\u003dtxn)"},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"    def create_or_update_floatingip_pfs(self, context, fip_id, txn):"},{"line_number":875,"context_line":"        self._l3_plugin.port_forwarding.db_sync_create_or_update("},{"line_number":876,"context_line":"            context, fip_id, txn)"},{"line_number":877,"context_line":""},{"line_number":878,"context_line":"    def delete_floatingip_pfs(self, context, fip_id, txn):"},{"line_number":879,"context_line":"        self._l3_plugin.port_forwarding.db_sync_delete("},{"line_number":880,"context_line":"            context, fip_id, txn)"},{"line_number":881,"context_line":""},{"line_number":882,"context_line":"    def ovn_lb_protocol(self, pf_protocol):"},{"line_number":883,"context_line":"        return self._l3_plugin.port_forwarding.ovn_lb_protocol(pf_protocol)"}],"source_content_type":"text/x-python","patch_set":28,"id":"9f560f44_6625e0ab","line":880,"range":{"start_line":874,"start_character":0,"end_line":880,"end_character":33},"updated":"2020-08-11 15:27:07.000000000","message":"If those are specific to the db_sync tool and only used there, I think it would be better to define them in that module instead of OVNClient. The OVNClient is suppose to be generic with reusable methods.","commit_id":"3d3c0533062b07c5e9bdafa7768fd1a89bde82b7"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"257a9e5cb1fb3d9568692f098dc728375244b227","unresolved":false,"context_lines":[{"line_number":871,"context_line":"            LOG.debug(\"FIP %s doesn\u0027t have external_ids.\", fip)"},{"line_number":872,"context_line":"        self._transaction(commands, txn\u003dtxn)"},{"line_number":873,"context_line":""},{"line_number":874,"context_line":"    def create_or_update_floatingip_pfs(self, context, fip_id, txn):"},{"line_number":875,"context_line":"        self._l3_plugin.port_forwarding.db_sync_create_or_update("},{"line_number":876,"context_line":"            context, fip_id, txn)"},{"line_number":877,"context_line":""},{"line_number":878,"context_line":"    def delete_floatingip_pfs(self, context, fip_id, txn):"},{"line_number":879,"context_line":"        self._l3_plugin.port_forwarding.db_sync_delete("},{"line_number":880,"context_line":"            context, fip_id, txn)"},{"line_number":881,"context_line":""},{"line_number":882,"context_line":"    def ovn_lb_protocol(self, pf_protocol):"},{"line_number":883,"context_line":"        return self._l3_plugin.port_forwarding.ovn_lb_protocol(pf_protocol)"}],"source_content_type":"text/x-python","patch_set":28,"id":"9f560f44_1106dc21","line":880,"range":{"start_line":874,"start_character":0,"end_line":880,"end_character":33},"in_reply_to":"9f560f44_6625e0ab","updated":"2020-08-11 18:55:14.000000000","message":"Ack. Make sense!\nRemoving these from here and making the necessary changes in \nhttps://review.opendev.org/#/c/743772/","commit_id":"3d3c0533062b07c5e9bdafa7768fd1a89bde82b7"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"aa014fafb62486c0602a1f21fcdbebe2aa2288c5","unresolved":false,"context_lines":[{"line_number":893,"context_line":"            db_rev.bump_revision("},{"line_number":894,"context_line":"                context, floatingip, ovn_const.TYPE_FLOATINGIPS)"},{"line_number":895,"context_line":""},{"line_number":896,"context_line":"    def create_floatingip_and_pf(self, context, floatingip):"},{"line_number":897,"context_line":"        self.create_floatingip(context, floatingip)"},{"line_number":898,"context_line":"        self._l3_plugin.port_forwarding.maintenance_create("},{"line_number":899,"context_line":"            context, floatingip)"},{"line_number":900,"context_line":""},{"line_number":901,"context_line":"    def create_floatingip(self, context, floatingip):"},{"line_number":902,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":28,"id":"9f560f44_2b3451fb","line":899,"range":{"start_line":896,"start_character":0,"end_line":899,"end_character":32},"updated":"2020-08-11 15:27:07.000000000","message":"Can we move this method as well as the delete_floatingip_and_pf and update_floatingip_and_pf to the maintenance.py module ?\n\nIt seems very specific to the maintenance task so having it in the OVNClient seems a bit off the scope for this class, which is to be generic and reused in other places.\n\nWe have something similar that is done in router ports:\n\nhttps://github.com/openstack/neutron/blob/b425ca45dddae1bfc44ff1fda9e29d8d71fadb4b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py#L189\n\nhttps://github.com/openstack/neutron/blob/b425ca45dddae1bfc44ff1fda9e29d8d71fadb4b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py#L398-L401","commit_id":"3d3c0533062b07c5e9bdafa7768fd1a89bde82b7"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"257a9e5cb1fb3d9568692f098dc728375244b227","unresolved":false,"context_lines":[{"line_number":893,"context_line":"            db_rev.bump_revision("},{"line_number":894,"context_line":"                context, floatingip, ovn_const.TYPE_FLOATINGIPS)"},{"line_number":895,"context_line":""},{"line_number":896,"context_line":"    def create_floatingip_and_pf(self, context, floatingip):"},{"line_number":897,"context_line":"        self.create_floatingip(context, floatingip)"},{"line_number":898,"context_line":"        self._l3_plugin.port_forwarding.maintenance_create("},{"line_number":899,"context_line":"            context, floatingip)"},{"line_number":900,"context_line":""},{"line_number":901,"context_line":"    def create_floatingip(self, context, floatingip):"},{"line_number":902,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":28,"id":"9f560f44_6c73ffb2","line":899,"range":{"start_line":896,"start_character":0,"end_line":899,"end_character":32},"in_reply_to":"9f560f44_2b3451fb","updated":"2020-08-11 18:55:14.000000000","message":"Yes, absolutely. Good point!","commit_id":"3d3c0533062b07c5e9bdafa7768fd1a89bde82b7"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8070946b7d95528ce9493a51ed2006312fff7fea","unresolved":false,"context_lines":[{"line_number":305,"context_line":""},{"line_number":306,"context_line":"        LOG.debug(\u0027ACL-SYNC: finished @ %s\u0027, str(datetime.now()))"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"    def _parse_ovn_lb_pfs(self, ovn_rtr_lb_pfs):"},{"line_number":309,"context_line":"        result \u003d {}"},{"line_number":310,"context_line":"        for ovn_lb in ovn_rtr_lb_pfs:"},{"line_number":311,"context_line":"            ext_ids \u003d ovn_lb.get(\u0027external_ids\u0027, {})"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_0682ed52","line":308,"updated":"2020-07-21 17:16:58.000000000","message":"I think (just an opinion) those kind of generic OVN methods to convert from OVN config to Neutron objects, should be in neutron.common.ovn.utils","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":305,"context_line":""},{"line_number":306,"context_line":"        LOG.debug(\u0027ACL-SYNC: finished @ %s\u0027, str(datetime.now()))"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"    def _parse_ovn_lb_pfs(self, ovn_rtr_lb_pfs):"},{"line_number":309,"context_line":"        result \u003d {}"},{"line_number":310,"context_line":"        for ovn_lb in ovn_rtr_lb_pfs:"},{"line_number":311,"context_line":"            ext_ids \u003d ovn_lb.get(\u0027external_ids\u0027, {})"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_fba5b9a0","line":308,"in_reply_to":"bf51134e_0682ed52","updated":"2020-07-22 22:56:20.000000000","message":"valid point. will do.","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"cf6a2a084939f5e61dad92104138bfbda222330c","unresolved":false,"context_lines":[{"line_number":356,"context_line":"        db_mapped_pfs \u003d {}"},{"line_number":357,"context_line":"        for db_pf in db_pfs:"},{"line_number":358,"context_line":"            fip_id \u003d db_pf.get(\u0027floatingip_id\u0027)"},{"line_number":359,"context_line":"            protocol \u003d self._ovn_client._ovn_lb_protocol(db_pf.get(\u0027protocol\u0027))"},{"line_number":360,"context_line":"            db_vip \u003d \"{}:{} {}:{}\".format("},{"line_number":361,"context_line":"                db_pf.get(\u0027floating_ip_address\u0027), db_pf.get(\u0027external_port\u0027),"},{"line_number":362,"context_line":"                db_pf.get(\u0027internal_ip_address\u0027), db_pf.get(\u0027internal_port\u0027))"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_60cf5429","line":359,"range":{"start_line":359,"start_character":68,"end_line":359,"end_character":76},"updated":"2020-07-21 14:08:51.000000000","message":"In db_pf the protocol is defined in all cases? Could it be a db_fip with None as protocol, meaning all protocols? Or in that case meaning would be something else?\n\nI just wanted to note that if there is no protocol set in the OVN LB, but there is a port set in vips column, then OVN will use TCP, even tho it is not specified in protocol column.","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":356,"context_line":"        db_mapped_pfs \u003d {}"},{"line_number":357,"context_line":"        for db_pf in db_pfs:"},{"line_number":358,"context_line":"            fip_id \u003d db_pf.get(\u0027floatingip_id\u0027)"},{"line_number":359,"context_line":"            protocol \u003d self._ovn_client._ovn_lb_protocol(db_pf.get(\u0027protocol\u0027))"},{"line_number":360,"context_line":"            db_vip \u003d \"{}:{} {}:{}\".format("},{"line_number":361,"context_line":"                db_pf.get(\u0027floating_ip_address\u0027), db_pf.get(\u0027external_port\u0027),"},{"line_number":362,"context_line":"                db_pf.get(\u0027internal_ip_address\u0027), db_pf.get(\u0027internal_port\u0027))"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_66859007","line":359,"range":{"start_line":359,"start_character":68,"end_line":359,"end_character":76},"in_reply_to":"bf51134e_60cf5429","updated":"2020-07-22 22:56:20.000000000","message":"Right, the protocol is always provided. The data model does not allow it to be empty [1] and if none is provided, it will used tcp. The REST API makes it non-optional [2].\n\n[1] https://specs.openstack.org/openstack/neutron-specs/specs/rocky/port-forwarding.html#data-model-impact\n\n[2] https://docs.openstack.org/api-ref/network/v2/index.html?expanded\u003dcreate-port-forwarding-detail#floating-ips-port-forwarding","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"aa014fafb62486c0602a1f21fcdbebe2aa2288c5","unresolved":false,"context_lines":[{"line_number":404,"context_line":"        update_snats_list \u003d []"},{"line_number":405,"context_line":"        update_fips_list \u003d []"},{"line_number":406,"context_line":"        for lrouter in lrouters:"},{"line_number":407,"context_line":"            ovn_rtr_lb_pfs \u003d \\"},{"line_number":408,"context_line":"                self.ovn_api.get_router_floatingip_lbs("},{"line_number":409,"context_line":"                    utils.ovn_name(lrouter[\u0027name\u0027]))"},{"line_number":410,"context_line":"            if lrouter[\u0027name\u0027] in db_routers:"}],"source_content_type":"text/x-python","patch_set":28,"id":"9f560f44_46741c9d","line":407,"range":{"start_line":407,"start_character":29,"end_line":407,"end_character":30},"updated":"2020-08-11 15:27:07.000000000","message":"nit:\n\nUse parentheses for wrapping long lines: \n\n\"... Long lines can be broken over multiple lines by wrapping expressions in parentheses. These should be used in preference to using a backslash for line continuation.\"\n\nhttps://www.python.org/dev/peps/pep-0008/#maximum-line-length","commit_id":"3d3c0533062b07c5e9bdafa7768fd1a89bde82b7"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"257a9e5cb1fb3d9568692f098dc728375244b227","unresolved":false,"context_lines":[{"line_number":404,"context_line":"        update_snats_list \u003d []"},{"line_number":405,"context_line":"        update_fips_list \u003d []"},{"line_number":406,"context_line":"        for lrouter in lrouters:"},{"line_number":407,"context_line":"            ovn_rtr_lb_pfs \u003d \\"},{"line_number":408,"context_line":"                self.ovn_api.get_router_floatingip_lbs("},{"line_number":409,"context_line":"                    utils.ovn_name(lrouter[\u0027name\u0027]))"},{"line_number":410,"context_line":"            if lrouter[\u0027name\u0027] in db_routers:"}],"source_content_type":"text/x-python","patch_set":28,"id":"9f560f44_cc920b59","line":407,"range":{"start_line":407,"start_character":29,"end_line":407,"end_character":30},"in_reply_to":"9f560f44_46741c9d","updated":"2020-08-11 18:55:14.000000000","message":"Definitely! Not sure how I missed that. I blame pycharm. ;)\nDone.","commit_id":"3d3c0533062b07c5e9bdafa7768fd1a89bde82b7"}],"neutron/services/portforwarding/constants.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"eff5178ffd11d780dde628dbfc2b95a2bcb2c02a","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"PORT_FORWARDING \u003d \u0027port_forwarding\u0027"},{"line_number":21,"context_line":"PORT_FORWARDING_PLUGIN \u003d \u0027port_forwarding_plugin\u0027"},{"line_number":22,"context_line":"PORT_FORWARDING_PREFIX \u003d \u0027pf-floatingip\u0027"},{"line_number":23,"context_line":"LB_PROTOCOL_MAP \u003d {\u0027udp\u0027: const.PROTO_UDP, \u0027tcp\u0027: const.PROTO_TCP}"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_b1fc8bb5","line":22,"range":{"start_line":22,"start_character":28,"end_line":22,"end_character":29},"updated":"2020-08-06 16:45:56.000000000","message":"nit: I think we use \"_\" for all OVN constants","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"f64a83230242f6073fe3e62c7145c199d6e4c881","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"PORT_FORWARDING \u003d \u0027port_forwarding\u0027"},{"line_number":21,"context_line":"PORT_FORWARDING_PLUGIN \u003d \u0027port_forwarding_plugin\u0027"},{"line_number":22,"context_line":"PORT_FORWARDING_PREFIX \u003d \u0027pf-floatingip\u0027"},{"line_number":23,"context_line":"LB_PROTOCOL_MAP \u003d {\u0027udp\u0027: const.PROTO_UDP, \u0027tcp\u0027: const.PROTO_TCP}"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_42027f70","line":22,"range":{"start_line":22,"start_character":28,"end_line":22,"end_character":29},"in_reply_to":"9f560f44_b1fc8bb5","updated":"2020-08-06 19:19:06.000000000","message":"This constant is used as the prefix for the load balancer\u0027s name in OVN NB DB. Since their names are normally made of \u0027-\u0027 I think would be be best to keep it that. Valid point, tho.\nReference: https://review.opendev.org/#/c/741303/21/neutron/services/portforwarding/drivers/ovn/driver.py@41","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"}],"neutron/services/portforwarding/drivers/ovn/driver.py":[{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"96f5faf7ca9af0768d1753b4c64dfbb11e72ca1d","unresolved":false,"context_lines":[{"line_number":176,"context_line":"            elif event_type \u003d\u003d events.AFTER_DELETE:"},{"line_number":177,"context_line":"                for pf_payload in payload:"},{"line_number":178,"context_line":"                    self._handler.port_forwarding_deleted(ovn_txn, ovn_nb,"},{"line_number":179,"context_line":"                        pf_payload.original_pf)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"            # Collect the revision numbers of all floating ips visited and"},{"line_number":182,"context_line":"            # update the corresponding load balancer entries affected."}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_c1c1c8e6","line":179,"range":{"start_line":179,"start_character":35,"end_line":179,"end_character":46},"updated":"2020-07-31 14:39:47.000000000","message":"Why not current here? It will be empty?","commit_id":"3eac2293129fab8bdd284b95d38f3df797db1bb7"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"b79c830c58e281fb0d1a8d8d3451fb5ba296b14e","unresolved":false,"context_lines":[{"line_number":176,"context_line":"            elif event_type \u003d\u003d events.AFTER_DELETE:"},{"line_number":177,"context_line":"                for pf_payload in payload:"},{"line_number":178,"context_line":"                    self._handler.port_forwarding_deleted(ovn_txn, ovn_nb,"},{"line_number":179,"context_line":"                        pf_payload.original_pf)"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"            # Collect the revision numbers of all floating ips visited and"},{"line_number":182,"context_line":"            # update the corresponding load balancer entries affected."}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_273d2ac4","line":179,"range":{"start_line":179,"start_character":35,"end_line":179,"end_character":46},"in_reply_to":"9f560f44_c1c1c8e6","updated":"2020-08-01 12:57:16.000000000","message":"Right. This comes from here [1].\npf_plugin will only populate the original when we are deleting the entry.\n[1]: https://github.com/openstack/neutron/blob/fe79ef22b8edadc02c54435537a6cc947960906f/neutron/services/portforwarding/pf_plugin.py#L587-L589","commit_id":"3eac2293129fab8bdd284b95d38f3df797db1bb7"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"96f5faf7ca9af0768d1753b4c64dfbb11e72ca1d","unresolved":false,"context_lines":[{"line_number":190,"context_line":"        # Update revision of affected floating ips. Note that even in"},{"line_number":191,"context_line":"        # cases where port forwarding is deleted, floating ip remains."},{"line_number":192,"context_line":"        for fip_obj in fip_objs.values():"},{"line_number":193,"context_line":"            db_rev.bump_revision(context, fip_obj, ovn_const.TYPE_FLOATINGIPS)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def _maintenance_callback(self, context, fip_id, is_delete):"},{"line_number":196,"context_line":"        # NOTE: Since the maintenance callback is not granular to the level"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_5c13db5c","line":193,"updated":"2020-07-31 14:39:47.000000000","message":"I believe before bumping up the revision number You should verify the actual revision number in the db (that going to prevent races).\n\nPlease take a look on example about bumping up revision number for ports.\n1. There is a command about checking revision number:\nhttps://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py#L460\n\n2. If `checking revision number` went fine (ovn_const.TXN_COMMITTED) the rev number is bumped:\nhttps://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py#L562","commit_id":"3eac2293129fab8bdd284b95d38f3df797db1bb7"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"803c73d6175137e37777360ba087d3f174f3b737","unresolved":false,"context_lines":[{"line_number":190,"context_line":"        # Update revision of affected floating ips. Note that even in"},{"line_number":191,"context_line":"        # cases where port forwarding is deleted, floating ip remains."},{"line_number":192,"context_line":"        for fip_obj in fip_objs.values():"},{"line_number":193,"context_line":"            db_rev.bump_revision(context, fip_obj, ovn_const.TYPE_FLOATINGIPS)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def _maintenance_callback(self, context, fip_id, is_delete):"},{"line_number":196,"context_line":"        # NOTE: Since the maintenance callback is not granular to the level"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_eaa8178d","line":193,"in_reply_to":"9f560f44_073aa6ca","updated":"2020-08-01 19:19:21.000000000","message":"Done","commit_id":"3eac2293129fab8bdd284b95d38f3df797db1bb7"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"b79c830c58e281fb0d1a8d8d3451fb5ba296b14e","unresolved":false,"context_lines":[{"line_number":190,"context_line":"        # Update revision of affected floating ips. Note that even in"},{"line_number":191,"context_line":"        # cases where port forwarding is deleted, floating ip remains."},{"line_number":192,"context_line":"        for fip_obj in fip_objs.values():"},{"line_number":193,"context_line":"            db_rev.bump_revision(context, fip_obj, ovn_const.TYPE_FLOATINGIPS)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def _maintenance_callback(self, context, fip_id, is_delete):"},{"line_number":196,"context_line":"        # NOTE: Since the maintenance callback is not granular to the level"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_073aa6ca","line":193,"in_reply_to":"9f560f44_5c13db5c","updated":"2020-08-01 12:57:16.000000000","message":"ack. TY for pointing this out!","commit_id":"3eac2293129fab8bdd284b95d38f3df797db1bb7"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"96f5faf7ca9af0768d1753b4c64dfbb11e72ca1d","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                    ovn_nb, fip_id, fip_revision)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        if not is_delete:"},{"line_number":218,"context_line":"            db_rev.bump_revision(context, fip_obj, ovn_const.TYPE_FLOATINGIPS)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"    def maintenance_create(self, context, floatingip):"},{"line_number":221,"context_line":"        fip_id \u003d floatingip[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_3c0ee731","line":218,"range":{"start_line":218,"start_character":12,"end_line":218,"end_character":32},"updated":"2020-07-31 14:39:47.000000000","message":"Same here","commit_id":"3eac2293129fab8bdd284b95d38f3df797db1bb7"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"b79c830c58e281fb0d1a8d8d3451fb5ba296b14e","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                    ovn_nb, fip_id, fip_revision)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        if not is_delete:"},{"line_number":218,"context_line":"            db_rev.bump_revision(context, fip_obj, ovn_const.TYPE_FLOATINGIPS)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"    def maintenance_create(self, context, floatingip):"},{"line_number":221,"context_line":"        fip_id \u003d floatingip[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_6733a2f7","line":218,"range":{"start_line":218,"start_character":12,"end_line":218,"end_character":32},"in_reply_to":"9f560f44_3c0ee731","updated":"2020-08-01 12:57:16.000000000","message":"Ack.","commit_id":"3eac2293129fab8bdd284b95d38f3df797db1bb7"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"803c73d6175137e37777360ba087d3f174f3b737","unresolved":false,"context_lines":[{"line_number":215,"context_line":"                    ovn_nb, fip_id, fip_revision)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"        if not is_delete:"},{"line_number":218,"context_line":"            db_rev.bump_revision(context, fip_obj, ovn_const.TYPE_FLOATINGIPS)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"    def maintenance_create(self, context, floatingip):"},{"line_number":221,"context_line":"        fip_id \u003d floatingip[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":17,"id":"9f560f44_caad539c","line":218,"range":{"start_line":218,"start_character":12,"end_line":218,"end_character":32},"in_reply_to":"9f560f44_6733a2f7","updated":"2020-08-01 19:19:21.000000000","message":"Done","commit_id":"3eac2293129fab8bdd284b95d38f3df797db1bb7"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"334ec66b8be9a69f935f49ea1787de4d9f3a0f26","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from neutron.db import ovn_revision_numbers_db as db_rev"},{"line_number":24,"context_line":"from neutron import manager"},{"line_number":25,"context_line":"from neutron.objects import port_forwarding as port_forwarding_obj"},{"line_number":26,"context_line":"from neutron.services.portforwarding.constants import LB_PROTOCOL_MAP"},{"line_number":27,"context_line":"from neutron.services.portforwarding.constants import PORT_FORWARDING"},{"line_number":28,"context_line":"from neutron.services.portforwarding.constants import PORT_FORWARDING_PLUGIN"},{"line_number":29,"context_line":"from neutron.services.portforwarding.constants import PORT_FORWARDING_PREFIX"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_42ec3f8e","line":29,"range":{"start_line":26,"start_character":0,"end_line":29,"end_character":76},"updated":"2020-08-06 22:12:10.000000000","message":"I think normally OS projects prefer importing just the module, though honestly with line length limitations and the number of \u0027constant\u0027 imports...I\u0027m good with this (though maybe comma-seperated?)","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"7ceec569405fa973221129b27e71adac636e9e2e","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from neutron.db import ovn_revision_numbers_db as db_rev"},{"line_number":24,"context_line":"from neutron import manager"},{"line_number":25,"context_line":"from neutron.objects import port_forwarding as port_forwarding_obj"},{"line_number":26,"context_line":"from neutron.services.portforwarding.constants import LB_PROTOCOL_MAP"},{"line_number":27,"context_line":"from neutron.services.portforwarding.constants import PORT_FORWARDING"},{"line_number":28,"context_line":"from neutron.services.portforwarding.constants import PORT_FORWARDING_PLUGIN"},{"line_number":29,"context_line":"from neutron.services.portforwarding.constants import PORT_FORWARDING_PREFIX"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_b1ac749f","line":29,"range":{"start_line":26,"start_character":0,"end_line":29,"end_character":76},"in_reply_to":"9f560f44_42ec3f8e","updated":"2020-08-07 20:44:42.000000000","message":"Done","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"334ec66b8be9a69f935f49ea1787de4d9f3a0f26","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        return lb_name, vip, [internal_ip], rtr_name"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def port_forwarding_created(self, ovn_txn, nb_ovn, pf_obj,"},{"line_number":59,"context_line":"                                is_update\u003dFalse):"},{"line_number":60,"context_line":"        if not is_update:"},{"line_number":61,"context_line":"            LOG.info(\"CREATE for port-forwarding %s vip %s:%s to %s:%s\","},{"line_number":62,"context_line":"                     pf_obj.protocol,"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_e2063320","line":59,"range":{"start_line":59,"start_character":32,"end_line":59,"end_character":41},"updated":"2020-08-06 22:12:10.000000000","message":"ignorable nit:\n\ninstead of passing around an is_update flag solely for created/deleted log message, maybe just have:\n\n def _port_forwarding_created(...)  # and deleted\n     ...\n\n def port_forwarding_created(...):  # and deleted\n     LOG.info(\"CREATE...\")\n     return self._port_forwarding_created(...)\n\n\n def port_forwarding_updated(...):\n     LOG.info(\"UPDATE...\")\n     self._port_forwarding_deleted(...)\n     self._port_forwarding_created(...)\n\netc. but just my personal preference to avoid extra args/conditionals.","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"7ceec569405fa973221129b27e71adac636e9e2e","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        return lb_name, vip, [internal_ip], rtr_name"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def port_forwarding_created(self, ovn_txn, nb_ovn, pf_obj,"},{"line_number":59,"context_line":"                                is_update\u003dFalse):"},{"line_number":60,"context_line":"        if not is_update:"},{"line_number":61,"context_line":"            LOG.info(\"CREATE for port-forwarding %s vip %s:%s to %s:%s\","},{"line_number":62,"context_line":"                     pf_obj.protocol,"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_f1526c66","line":59,"range":{"start_line":59,"start_character":32,"end_line":59,"end_character":41},"in_reply_to":"9f560f44_e2063320","updated":"2020-08-07 20:44:42.000000000","message":"Yes! Will do.","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"334ec66b8be9a69f935f49ea1787de4d9f3a0f26","unresolved":false,"context_lines":[{"line_number":95,"context_line":"                     pf_obj.floating_ip_address, pf_obj.external_port,"},{"line_number":96,"context_line":"                     pf_obj.internal_ip_address, pf_obj.internal_port)"},{"line_number":97,"context_line":"        # Note: load balancer instance is expected to be removed by api once"},{"line_number":98,"context_line":"        #       last vip is removed. Since router has weak ref to the lb, that"},{"line_number":99,"context_line":"        #       gets taken care automatically as well."},{"line_number":100,"context_line":"        lb_name, vip, _internal_ips, _rtr \u003d self._get_lb_attributes(pf_obj)"},{"line_number":101,"context_line":"        ovn_txn.add(nb_ovn.lb_del(lb_name, vip, if_exists\u003dTrue))"},{"line_number":102,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_3d868a14","line":99,"range":{"start_line":98,"start_character":37,"end_line":99,"end_character":53},"updated":"2020-08-06 22:12:10.000000000","message":"It will, though one thing to note is that *during this transaction* if some bit of code looked at router.load_balancer, that entry would still show up if we don\u0027t explicitly delete it in the transaction. It is only at transaction commit time on the server itself that it will be automatically cleaned up.\n\nSince for an update, we do a delete() and a create() in the same transaction, for the purposes of that transaction there will be two LB entries in router.load_balancer until we get the update from the DB when the transaction is committed. It doesn\u0027t look like anything would fail, though (but I did have to look pretty carefully to determine that).\n\ntl;dr May not matter, but in general it is good practice to go ahead and delete the things that will be cleaned up automatically.","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"7ceec569405fa973221129b27e71adac636e9e2e","unresolved":false,"context_lines":[{"line_number":95,"context_line":"                     pf_obj.floating_ip_address, pf_obj.external_port,"},{"line_number":96,"context_line":"                     pf_obj.internal_ip_address, pf_obj.internal_port)"},{"line_number":97,"context_line":"        # Note: load balancer instance is expected to be removed by api once"},{"line_number":98,"context_line":"        #       last vip is removed. Since router has weak ref to the lb, that"},{"line_number":99,"context_line":"        #       gets taken care automatically as well."},{"line_number":100,"context_line":"        lb_name, vip, _internal_ips, _rtr \u003d self._get_lb_attributes(pf_obj)"},{"line_number":101,"context_line":"        ovn_txn.add(nb_ovn.lb_del(lb_name, vip, if_exists\u003dTrue))"},{"line_number":102,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_d16be8ed","line":99,"range":{"start_line":98,"start_character":37,"end_line":99,"end_character":53},"in_reply_to":"9f560f44_3d868a14","updated":"2020-08-07 20:44:42.000000000","message":"Ack. I trust ovsdb better than I trust myself. ;) In order to get this right, I would need to have a way to safely see if all the vips were removed. A way to do it would be to pass in the router name to lb_del and have it do the lb_lr_del. Will leave a note here on that regard.","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"334ec66b8be9a69f935f49ea1787de4d9f3a0f26","unresolved":false,"context_lines":[{"line_number":123,"context_line":"    def _get_pf_objs(self, context, fip_id):"},{"line_number":124,"context_line":"        pf_dicts \u003d self._pf_plugin.get_floatingip_port_forwardings("},{"line_number":125,"context_line":"            context, fip_id)"},{"line_number":126,"context_line":"        return[port_forwarding_obj.PortForwarding(context\u003dcontext, **pf_dict)"},{"line_number":127,"context_line":"               for pf_dict in pf_dicts]"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def _get_fip_objs(self, context, payload):"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_3d5e8a75","line":126,"range":{"start_line":126,"start_character":13,"end_line":126,"end_character":15},"updated":"2020-08-06 22:12:10.000000000","message":"missing space","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"7ceec569405fa973221129b27e71adac636e9e2e","unresolved":false,"context_lines":[{"line_number":123,"context_line":"    def _get_pf_objs(self, context, fip_id):"},{"line_number":124,"context_line":"        pf_dicts \u003d self._pf_plugin.get_floatingip_port_forwardings("},{"line_number":125,"context_line":"            context, fip_id)"},{"line_number":126,"context_line":"        return[port_forwarding_obj.PortForwarding(context\u003dcontext, **pf_dict)"},{"line_number":127,"context_line":"               for pf_dict in pf_dicts]"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def _get_fip_objs(self, context, payload):"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_914e9094","line":126,"range":{"start_line":126,"start_character":13,"end_line":126,"end_character":15},"in_reply_to":"9f560f44_3d5e8a75","updated":"2020-08-07 20:44:42.000000000","message":"Done","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"eff5178ffd11d780dde628dbfc2b95a2bcb2c02a","unresolved":false,"context_lines":[{"line_number":129,"context_line":"    def _get_fip_objs(self, context, payload):"},{"line_number":130,"context_line":"        floatingip_ids \u003d set()"},{"line_number":131,"context_line":"        for pf_payload in payload:"},{"line_number":132,"context_line":"            if pf_payload.current_pf:"},{"line_number":133,"context_line":"                floatingip_ids.add(pf_payload.current_pf.floatingip_id)"},{"line_number":134,"context_line":"            if pf_payload.original_pf:"},{"line_number":135,"context_line":"                floatingip_ids.add(pf_payload.original_pf.floatingip_id)"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_31107ba8","line":132,"updated":"2020-08-06 16:45:56.000000000","message":"I don\u0027t understand this: both floatingip_id should be the same, shouldn\u0027t they?","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"f64a83230242f6073fe3e62c7145c199d6e4c881","unresolved":false,"context_lines":[{"line_number":129,"context_line":"    def _get_fip_objs(self, context, payload):"},{"line_number":130,"context_line":"        floatingip_ids \u003d set()"},{"line_number":131,"context_line":"        for pf_payload in payload:"},{"line_number":132,"context_line":"            if pf_payload.current_pf:"},{"line_number":133,"context_line":"                floatingip_ids.add(pf_payload.current_pf.floatingip_id)"},{"line_number":134,"context_line":"            if pf_payload.original_pf:"},{"line_number":135,"context_line":"                floatingip_ids.add(pf_payload.original_pf.floatingip_id)"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_62b183e1","line":132,"in_reply_to":"9f560f44_31107ba8","updated":"2020-08-06 19:19:06.000000000","message":"Yes. However, depending on the operation of the callback\none of these will be None. While fip_id changes are not expected in UPDATE, they are supported in this codepath.\nPlease see here for where/how these are added to the event:\n\nhttps://github.com/openstack/neutron/blob/24590a334fff0ed1cb513b0f496be965bc9309d4/neutron/services/portforwarding/pf_plugin.py#L260\n\nhttps://github.com/openstack/neutron/blob/24590a334fff0ed1cb513b0f496be965bc9309d4/neutron/services/portforwarding/pf_plugin.py#L389\n\nhttps://github.com/openstack/neutron/blob/24590a334fff0ed1cb513b0f496be965bc9309d4/neutron/services/portforwarding/pf_plugin.py#L444\n\nhttps://github.com/openstack/neutron/blob/24590a334fff0ed1cb513b0f496be965bc9309d4/neutron/services/portforwarding/pf_plugin.py#L588","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"7ceec569405fa973221129b27e71adac636e9e2e","unresolved":false,"context_lines":[{"line_number":129,"context_line":"    def _get_fip_objs(self, context, payload):"},{"line_number":130,"context_line":"        floatingip_ids \u003d set()"},{"line_number":131,"context_line":"        for pf_payload in payload:"},{"line_number":132,"context_line":"            if pf_payload.current_pf:"},{"line_number":133,"context_line":"                floatingip_ids.add(pf_payload.current_pf.floatingip_id)"},{"line_number":134,"context_line":"            if pf_payload.original_pf:"},{"line_number":135,"context_line":"                floatingip_ids.add(pf_payload.original_pf.floatingip_id)"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_54efc6e7","line":132,"in_reply_to":"9f560f44_614980b5","updated":"2020-08-07 20:44:42.000000000","message":"Done","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"334ec66b8be9a69f935f49ea1787de4d9f3a0f26","unresolved":false,"context_lines":[{"line_number":129,"context_line":"    def _get_fip_objs(self, context, payload):"},{"line_number":130,"context_line":"        floatingip_ids \u003d set()"},{"line_number":131,"context_line":"        for pf_payload in payload:"},{"line_number":132,"context_line":"            if pf_payload.current_pf:"},{"line_number":133,"context_line":"                floatingip_ids.add(pf_payload.current_pf.floatingip_id)"},{"line_number":134,"context_line":"            if pf_payload.original_pf:"},{"line_number":135,"context_line":"                floatingip_ids.add(pf_payload.original_pf.floatingip_id)"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_614980b5","line":132,"in_reply_to":"9f560f44_62b183e1","updated":"2020-08-06 22:12:10.000000000","message":"I think I get it, anyway. :p\n\nI don\u0027t know if it\u0027s just having a single method that handles all of the different cases is what makes it a little confusing (i.e. you have to know that the case where you are deleting all pf resources is the one where you\u0027ll have multiple payloads, and those will have original_pf with current_pf\u003dNone, etc. or that update_floatingip_port_forwarding actually passes both current_pf and original_pf !\u003d None).\n\nMaybe some comments to help future visitors. :)","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"334ec66b8be9a69f935f49ea1787de4d9f3a0f26","unresolved":false,"context_lines":[{"line_number":133,"context_line":"                floatingip_ids.add(pf_payload.current_pf.floatingip_id)"},{"line_number":134,"context_line":"            if pf_payload.original_pf:"},{"line_number":135,"context_line":"                floatingip_ids.add(pf_payload.original_pf.floatingip_id)"},{"line_number":136,"context_line":"        fip_objs \u003d {}"},{"line_number":137,"context_line":"        for floatingip_id in floatingip_ids:"},{"line_number":138,"context_line":"            fip_objs[floatingip_id] \u003d self._l3_plugin.get_floatingip("},{"line_number":139,"context_line":"                context, floatingip_id)"},{"line_number":140,"context_line":"        return fip_objs"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def _add_check_rev(self, ovn_txn, ovn_nb, fip_id, fip_obj):"},{"line_number":143,"context_line":"        \"\"\"Updating revision number of OVN lb entries based on floatingip id"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_e1de303c","line":140,"range":{"start_line":136,"start_character":0,"end_line":140,"end_character":23},"updated":"2020-08-06 22:12:10.000000000","message":"nit:\n\n return {fip_id: self._l3_plugin.get_floatingip(context, floatingip_id) for fip_id in floatingip_ids}","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"7ceec569405fa973221129b27e71adac636e9e2e","unresolved":false,"context_lines":[{"line_number":133,"context_line":"                floatingip_ids.add(pf_payload.current_pf.floatingip_id)"},{"line_number":134,"context_line":"            if pf_payload.original_pf:"},{"line_number":135,"context_line":"                floatingip_ids.add(pf_payload.original_pf.floatingip_id)"},{"line_number":136,"context_line":"        fip_objs \u003d {}"},{"line_number":137,"context_line":"        for floatingip_id in floatingip_ids:"},{"line_number":138,"context_line":"            fip_objs[floatingip_id] \u003d self._l3_plugin.get_floatingip("},{"line_number":139,"context_line":"                context, floatingip_id)"},{"line_number":140,"context_line":"        return fip_objs"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def _add_check_rev(self, ovn_txn, ovn_nb, fip_id, fip_obj):"},{"line_number":143,"context_line":"        \"\"\"Updating revision number of OVN lb entries based on floatingip id"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_14fc2e6e","line":140,"range":{"start_line":136,"start_character":0,"end_line":140,"end_character":23},"in_reply_to":"9f560f44_e1de303c","updated":"2020-08-07 20:44:42.000000000","message":"Nice! Done.","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"334ec66b8be9a69f935f49ea1787de4d9f3a0f26","unresolved":false,"context_lines":[{"line_number":159,"context_line":"        return check_rev_tuples"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    def _do_db_rev_bump_revision(self, context, check_rev_tuples):"},{"line_number":162,"context_line":"        if not all([check_rev_cmd.result \u003d\u003d ovn_const.TXN_COMMITTED"},{"line_number":163,"context_line":"                    for check_rev_cmd, _fip_obj in check_rev_tuples]):"},{"line_number":164,"context_line":"            return"},{"line_number":165,"context_line":"        for _check_rev_cmd, fip_obj in check_rev_tuples:"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_218668ba","line":162,"range":{"start_line":162,"start_character":19,"end_line":162,"end_character":20},"updated":"2020-08-06 22:12:10.000000000","message":"can remove the list comprehension and just do generator expression in all by itself.","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"7ceec569405fa973221129b27e71adac636e9e2e","unresolved":false,"context_lines":[{"line_number":159,"context_line":"        return check_rev_tuples"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"    def _do_db_rev_bump_revision(self, context, check_rev_tuples):"},{"line_number":162,"context_line":"        if not all([check_rev_cmd.result \u003d\u003d ovn_const.TXN_COMMITTED"},{"line_number":163,"context_line":"                    for check_rev_cmd, _fip_obj in check_rev_tuples]):"},{"line_number":164,"context_line":"            return"},{"line_number":165,"context_line":"        for _check_rev_cmd, fip_obj in check_rev_tuples:"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_549186f0","line":162,"range":{"start_line":162,"start_character":19,"end_line":162,"end_character":20},"in_reply_to":"9f560f44_218668ba","updated":"2020-08-07 20:44:42.000000000","message":"Done","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"334ec66b8be9a69f935f49ea1787de4d9f3a0f26","unresolved":false,"context_lines":[{"line_number":170,"context_line":"            return"},{"line_number":171,"context_line":"        context \u003d payload[0].context"},{"line_number":172,"context_line":"        ovn_nb \u003d self._l3_plugin._ovn"},{"line_number":173,"context_line":"        txn \u003d ovn_nb.transaction"},{"line_number":174,"context_line":"        with txn(check_error\u003dTrue) as ovn_txn:"},{"line_number":175,"context_line":"            if event_type \u003d\u003d events.AFTER_CREATE:"},{"line_number":176,"context_line":"                for pf_payload in payload:"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_842022f9","line":173,"range":{"start_line":173,"start_character":8,"end_line":173,"end_character":32},"updated":"2020-08-06 22:12:10.000000000","message":"nit: \n\nthis shouldn\u0027t make the line too long just to use w/o the alias in the only place it is referenced. :)","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"7ceec569405fa973221129b27e71adac636e9e2e","unresolved":false,"context_lines":[{"line_number":170,"context_line":"            return"},{"line_number":171,"context_line":"        context \u003d payload[0].context"},{"line_number":172,"context_line":"        ovn_nb \u003d self._l3_plugin._ovn"},{"line_number":173,"context_line":"        txn \u003d ovn_nb.transaction"},{"line_number":174,"context_line":"        with txn(check_error\u003dTrue) as ovn_txn:"},{"line_number":175,"context_line":"            if event_type \u003d\u003d events.AFTER_CREATE:"},{"line_number":176,"context_line":"                for pf_payload in payload:"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_74680af4","line":173,"range":{"start_line":173,"start_character":8,"end_line":173,"end_character":32},"in_reply_to":"9f560f44_842022f9","updated":"2020-08-07 20:44:42.000000000","message":"Done","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"334ec66b8be9a69f935f49ea1787de4d9f3a0f26","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        # cases where port forwarding is deleted, floating ip remains."},{"line_number":200,"context_line":"        self._do_db_rev_bump_revision(context, check_rev_tuples)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def _maintenance_callback(self, context, fip_id, is_delete):"},{"line_number":203,"context_line":"        # NOTE: Since the maintenance callback is not granular to the level"},{"line_number":204,"context_line":"        #       of the affected pfs AND the fact that pfs are all vips"},{"line_number":205,"context_line":"        #       in a load balancer entry, it is cheap enough to simply rebuild."}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_64ea2ed8","line":202,"updated":"2020-08-06 22:12:10.000000000","message":"It seems like bundling all of this into a single method might not buy us much if we\u0027re having to branch on is_delete 3 times and most of the shared code is just aliased variables. Maybe _maintenance_create_update and just maintenance_delete?","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"7ceec569405fa973221129b27e71adac636e9e2e","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        # cases where port forwarding is deleted, floating ip remains."},{"line_number":200,"context_line":"        self._do_db_rev_bump_revision(context, check_rev_tuples)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def _maintenance_callback(self, context, fip_id, is_delete):"},{"line_number":203,"context_line":"        # NOTE: Since the maintenance callback is not granular to the level"},{"line_number":204,"context_line":"        #       of the affected pfs AND the fact that pfs are all vips"},{"line_number":205,"context_line":"        #       in a load balancer entry, it is cheap enough to simply rebuild."}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_1f9a87b7","line":202,"in_reply_to":"9f560f44_64ea2ed8","updated":"2020-08-07 20:44:42.000000000","message":"So true! Done.","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"334ec66b8be9a69f935f49ea1787de4d9f3a0f26","unresolved":false,"context_lines":[{"line_number":204,"context_line":"        #       of the affected pfs AND the fact that pfs are all vips"},{"line_number":205,"context_line":"        #       in a load balancer entry, it is cheap enough to simply rebuild."},{"line_number":206,"context_line":"        ovn_nb \u003d self._l3_plugin._ovn"},{"line_number":207,"context_line":"        txn \u003d ovn_nb.transaction"},{"line_number":208,"context_line":"        pf_objs \u003d [] if is_delete else self._get_pf_objs(context, fip_id)"},{"line_number":209,"context_line":"        LOG.debug(\"Maintenance port forwarding under fip %s (delete: %s) : %s\","},{"line_number":210,"context_line":"                  fip_id, is_delete, pf_objs)"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_84c4a241","line":207,"range":{"start_line":207,"start_character":8,"end_line":207,"end_character":32},"updated":"2020-08-06 22:12:10.000000000","message":"nit: \n\nthis shouldn\u0027t make the line too long just to use w/o the alias in the only place it is referenced. :)","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"7ceec569405fa973221129b27e71adac636e9e2e","unresolved":false,"context_lines":[{"line_number":204,"context_line":"        #       of the affected pfs AND the fact that pfs are all vips"},{"line_number":205,"context_line":"        #       in a load balancer entry, it is cheap enough to simply rebuild."},{"line_number":206,"context_line":"        ovn_nb \u003d self._l3_plugin._ovn"},{"line_number":207,"context_line":"        txn \u003d ovn_nb.transaction"},{"line_number":208,"context_line":"        pf_objs \u003d [] if is_delete else self._get_pf_objs(context, fip_id)"},{"line_number":209,"context_line":"        LOG.debug(\"Maintenance port forwarding under fip %s (delete: %s) : %s\","},{"line_number":210,"context_line":"                  fip_id, is_delete, pf_objs)"}],"source_content_type":"text/x-python","patch_set":20,"id":"9f560f44_f4737ae5","line":207,"range":{"start_line":207,"start_character":8,"end_line":207,"end_character":32},"in_reply_to":"9f560f44_84c4a241","updated":"2020-08-07 20:44:42.000000000","message":"Done","commit_id":"cb6729377f6fae7567712bf65da9edfd868ca09d"}],"neutron/services/portforwarding/ovn/port_forwarding.py":[{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"eda7ba99fac9ed6fa12fc2d1314ac2a13ccce801","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    def _get_lb_protocol(pf_obj):"},{"line_number":35,"context_line":"        return LB_PROTOCOL_MAP[pf_obj.protocol]"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @staticmethod"},{"line_number":38,"context_line":"    def lb_name(fip_id, proto):"},{"line_number":39,"context_line":"        return \"{}-{}-{}\".format(PORT_FORWARDING_PREFIX, fip_id, proto)"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    @classmethod"},{"line_number":42,"context_line":"    def lb_names(cls, fip_id):"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_d538cbd5","line":39,"range":{"start_line":37,"start_character":0,"end_line":39,"end_character":71},"updated":"2020-07-17 13:01:12.000000000","message":"Thanks for that. This creates unique space for OVN and PF LB\u0027s in the same OVN deployment.","commit_id":"a17cba399e8fbdb27f648e25e9a0e28d2d0c3ef9"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"a653e14ed64bb484602e88dd8cbc165bf156a87f","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    def _get_lb_protocol(pf_obj):"},{"line_number":35,"context_line":"        return LB_PROTOCOL_MAP[pf_obj.protocol]"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    @staticmethod"},{"line_number":38,"context_line":"    def lb_name(fip_id, proto):"},{"line_number":39,"context_line":"        return \"{}-{}-{}\".format(PORT_FORWARDING_PREFIX, fip_id, proto)"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    @classmethod"},{"line_number":42,"context_line":"    def lb_names(cls, fip_id):"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_7b1f14b7","line":39,"range":{"start_line":37,"start_character":0,"end_line":39,"end_character":71},"in_reply_to":"bf51134e_d538cbd5","updated":"2020-07-20 19:21:38.000000000","message":"heh, thanks to you, bud!","commit_id":"a17cba399e8fbdb27f648e25e9a0e28d2d0c3ef9"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"eda7ba99fac9ed6fa12fc2d1314ac2a13ccce801","unresolved":false,"context_lines":[{"line_number":76,"context_line":"        external_ids \u003d {"},{"line_number":77,"context_line":"            ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY: PORT_FORWARDING_PLUGIN,"},{"line_number":78,"context_line":"            ovn_const.OVN_FIP_EXT_ID_KEY: pf_obj.floatingip_id,"},{"line_number":79,"context_line":"            \u0027neutron:fip_address\u0027: str(pf_obj.floating_ip_address),"},{"line_number":80,"context_line":"            ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: rtr_name,"},{"line_number":81,"context_line":"        }"},{"line_number":82,"context_line":"        ovn_txn.add("}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_359dc7bb","line":79,"range":{"start_line":79,"start_character":13,"end_line":79,"end_character":32},"updated":"2020-07-17 13:01:12.000000000","message":"Why not create a constant?","commit_id":"a17cba399e8fbdb27f648e25e9a0e28d2d0c3ef9"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"a653e14ed64bb484602e88dd8cbc165bf156a87f","unresolved":false,"context_lines":[{"line_number":76,"context_line":"        external_ids \u003d {"},{"line_number":77,"context_line":"            ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY: PORT_FORWARDING_PLUGIN,"},{"line_number":78,"context_line":"            ovn_const.OVN_FIP_EXT_ID_KEY: pf_obj.floatingip_id,"},{"line_number":79,"context_line":"            \u0027neutron:fip_address\u0027: str(pf_obj.floating_ip_address),"},{"line_number":80,"context_line":"            ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: rtr_name,"},{"line_number":81,"context_line":"        }"},{"line_number":82,"context_line":"        ovn_txn.add("}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_9b7b08f3","line":79,"range":{"start_line":79,"start_character":13,"end_line":79,"end_character":32},"in_reply_to":"bf51134e_359dc7bb","updated":"2020-07-20 19:21:38.000000000","message":"That is a very good question. I added that here as debug and think that it servers no purpose. So, in the spirit of \"less is more\" I\u0027ll remove it. Note that the floating ip address is already part of the vip, so this is redundant.","commit_id":"a17cba399e8fbdb27f648e25e9a0e28d2d0c3ef9"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"eda7ba99fac9ed6fa12fc2d1314ac2a13ccce801","unresolved":false,"context_lines":[{"line_number":83,"context_line":"            nb_ovn.lb_add(lb_name, vip, internal_ips,"},{"line_number":84,"context_line":"                          self._get_lb_protocol(pf_obj), may_exist\u003dTrue,"},{"line_number":85,"context_line":"                          external_ids\u003dexternal_ids))"},{"line_number":86,"context_line":"        # Ensure logical router has load balancer configured."},{"line_number":87,"context_line":"        ovn_txn.add(nb_ovn.lr_lb_add(rtr_name, lb_name, may_exist\u003dTrue))"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    def port_forwarding_updated(self, ovn_txn, nb_ovn, pf_obj, orig_pf_obj):"},{"line_number":90,"context_line":"        LOG.debug(\"Handling UPDATE event orig_prt_fwd %s is now prt_fwd %s\","}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_d5d9eb84","line":87,"range":{"start_line":86,"start_character":8,"end_line":87,"end_character":72},"updated":"2020-07-17 13:01:12.000000000","message":"What about LS? I think we should also add a reference to LS, from which the port behind FIP exists. No?\n\nExample:\n\nLB (with pf-fip) -\u003e LS -\u003e (LSP behind FIP/PF)\n\nI think we should also update LS to point to LB.\n\nSomething like:\nself.ovn_nbdb_api.ls_lb_add(ovn_ls.uuid, ovn_lb.uuid, may_exist\u003dTrue))","commit_id":"a17cba399e8fbdb27f648e25e9a0e28d2d0c3ef9"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"a653e14ed64bb484602e88dd8cbc165bf156a87f","unresolved":false,"context_lines":[{"line_number":83,"context_line":"            nb_ovn.lb_add(lb_name, vip, internal_ips,"},{"line_number":84,"context_line":"                          self._get_lb_protocol(pf_obj), may_exist\u003dTrue,"},{"line_number":85,"context_line":"                          external_ids\u003dexternal_ids))"},{"line_number":86,"context_line":"        # Ensure logical router has load balancer configured."},{"line_number":87,"context_line":"        ovn_txn.add(nb_ovn.lr_lb_add(rtr_name, lb_name, may_exist\u003dTrue))"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    def port_forwarding_updated(self, ovn_txn, nb_ovn, pf_obj, orig_pf_obj):"},{"line_number":90,"context_line":"        LOG.debug(\"Handling UPDATE event orig_prt_fwd %s is now prt_fwd %s\","}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_be9eba66","line":87,"range":{"start_line":86,"start_character":8,"end_line":87,"end_character":72},"in_reply_to":"bf51134e_555ddbea","updated":"2020-07-20 19:21:38.000000000","message":"I did some poking around and consulted with my OVN gurus about this. Here is what I learned from Mark Michelson:\n\n\"\u003cmmichelson\u003e OVN allows load balancers to be set on logical switches or logical routers, actually. AFAIK, the load balancer works the same way on a logical switch as it does a logical router. That is, it\u0027s L3 load balancing. Typically in any sort of tests I run, I run load balancers on gateway routers. But I suppose that running a load balancer on a logical switch is used in cases where \"internal\" traffic needs to be load balanced. So E/W traffic that won\u0027t hit a GW router can get load balanced if necessary.\"\n\nSince port forwarding is used for floating ip -- north/south -- and there will always be a router associated with it, I beleive that adding it to a LogicalSwitch is no aplicable. Sounds reasonable?","commit_id":"a17cba399e8fbdb27f648e25e9a0e28d2d0c3ef9"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"3715aff30848b224fa23784a65d1e838fb4a671d","unresolved":false,"context_lines":[{"line_number":83,"context_line":"            nb_ovn.lb_add(lb_name, vip, internal_ips,"},{"line_number":84,"context_line":"                          self._get_lb_protocol(pf_obj), may_exist\u003dTrue,"},{"line_number":85,"context_line":"                          external_ids\u003dexternal_ids))"},{"line_number":86,"context_line":"        # Ensure logical router has load balancer configured."},{"line_number":87,"context_line":"        ovn_txn.add(nb_ovn.lr_lb_add(rtr_name, lb_name, may_exist\u003dTrue))"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    def port_forwarding_updated(self, ovn_txn, nb_ovn, pf_obj, orig_pf_obj):"},{"line_number":90,"context_line":"        LOG.debug(\"Handling UPDATE event orig_prt_fwd %s is now prt_fwd %s\","}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_a0920c57","line":87,"range":{"start_line":86,"start_character":8,"end_line":87,"end_character":72},"in_reply_to":"bf51134e_be9eba66","updated":"2020-07-21 13:40:49.000000000","message":"+1 good explanation, so all is fine and that makes process to be much easier, thanks!","commit_id":"a17cba399e8fbdb27f648e25e9a0e28d2d0c3ef9"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"87e44457c45753d38086ba4746c07bb646ec81de","unresolved":false,"context_lines":[{"line_number":83,"context_line":"            nb_ovn.lb_add(lb_name, vip, internal_ips,"},{"line_number":84,"context_line":"                          self._get_lb_protocol(pf_obj), may_exist\u003dTrue,"},{"line_number":85,"context_line":"                          external_ids\u003dexternal_ids))"},{"line_number":86,"context_line":"        # Ensure logical router has load balancer configured."},{"line_number":87,"context_line":"        ovn_txn.add(nb_ovn.lr_lb_add(rtr_name, lb_name, may_exist\u003dTrue))"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    def port_forwarding_updated(self, ovn_txn, nb_ovn, pf_obj, orig_pf_obj):"},{"line_number":90,"context_line":"        LOG.debug(\"Handling UPDATE event orig_prt_fwd %s is now prt_fwd %s\","}],"source_content_type":"text/x-python","patch_set":3,"id":"bf51134e_555ddbea","line":87,"range":{"start_line":86,"start_character":8,"end_line":87,"end_character":72},"in_reply_to":"bf51134e_d5d9eb84","updated":"2020-07-17 13:02:19.000000000","message":"Hmm, or maybe even also in LS of FIP network (provider network)? I think Numan will know the answer.","commit_id":"a17cba399e8fbdb27f648e25e9a0e28d2d0c3ef9"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8070946b7d95528ce9493a51ed2006312fff7fea","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":2,"context_line":"#    not use this file except in compliance with the License. You may obtain"},{"line_number":3,"context_line":"#    a copy of the License at"},{"line_number":4,"context_line":"#"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_867b9d27","line":1,"updated":"2020-07-21 17:16:58.000000000","message":"This file, following the convention defined in other plugins, should be in services.portforwarding.drivers.ovn and call it \"driver\"","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":2,"context_line":"#    not use this file except in compliance with the License. You may obtain"},{"line_number":3,"context_line":"#    a copy of the License at"},{"line_number":4,"context_line":"#"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_c60bbc28","line":1,"in_reply_to":"bf51134e_867b9d27","updated":"2020-07-22 22:56:20.000000000","message":"Done","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8070946b7d95528ce9493a51ed2006312fff7fea","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def port_forwarding_update_revision_number(self, ovn_txn, nb_ovn,"},{"line_number":57,"context_line":"                                               floatingip_id, fip_revision):"},{"line_number":58,"context_line":"        for iter_lb_name in self.lb_names(floatingip_id):"},{"line_number":59,"context_line":"            LOG.debug(\"Setting lb for port-forwarding %s to revision %s\","},{"line_number":60,"context_line":"                      iter_lb_name, fip_revision)"},{"line_number":61,"context_line":"            ovn_txn.add(nb_ovn.update_lb_external_ids("}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_c6191523","line":58,"updated":"2020-07-21 17:16:58.000000000","message":"I don\u0027t understand this method.\n\n1) The object updated is the LB, right? Why?\n2) Instead of this, you should group the LB and the revisions and commit one single txn per LB register.","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def port_forwarding_update_revision_number(self, ovn_txn, nb_ovn,"},{"line_number":57,"context_line":"                                               floatingip_id, fip_revision):"},{"line_number":58,"context_line":"        for iter_lb_name in self.lb_names(floatingip_id):"},{"line_number":59,"context_line":"            LOG.debug(\"Setting lb for port-forwarding %s to revision %s\","},{"line_number":60,"context_line":"                      iter_lb_name, fip_revision)"},{"line_number":61,"context_line":"            ovn_txn.add(nb_ovn.update_lb_external_ids("}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_0db94c14","line":58,"in_reply_to":"bf51134e_c6191523","updated":"2020-07-22 22:56:20.000000000","message":"Right, this is for the ovn load balancer entries.\nHere is the rub: The reviosion number for the load balancer entries follow the revision number of the floating ip, not the neutron port forwarding.\n\nAlso, there can only be 1 protocol per ovn lb entry. So 1 floating ip can map to 1 or 2 ovn load balancer entries: tcp and udp, which are unique baed on their name.\n\nPlease look at the doc in this gerrit for an example [1].\n\nSo, this function updates the 2 ovn lb entries that may exist for a given fip_id. I will add some comments here.\n\n[1]: https://review.opendev.org/#/c/740955/5/doc/source/ovn/port_forwarding.rst@27","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8070946b7d95528ce9493a51ed2006312fff7fea","unresolved":false,"context_lines":[{"line_number":65,"context_line":"    def port_forwarding_created(self, ovn_txn, nb_ovn, pf_obj,"},{"line_number":66,"context_line":"                                is_update\u003dFalse):"},{"line_number":67,"context_line":"        if not is_update:"},{"line_number":68,"context_line":"            LOG.debug(\"Handling CREATE event prt_fwd %s\", pf_obj)"},{"line_number":69,"context_line":"            LOG.info(\"CREATE for port-forwarding %s vip %s:%s to %s:%s\","},{"line_number":70,"context_line":"                     pf_obj.protocol,"},{"line_number":71,"context_line":"                     pf_obj.floating_ip_address, pf_obj.external_port,"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_06d14d30","line":68,"updated":"2020-07-21 17:16:58.000000000","message":"If we have an info message, I don\u0027t think we need the debug one","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":65,"context_line":"    def port_forwarding_created(self, ovn_txn, nb_ovn, pf_obj,"},{"line_number":66,"context_line":"                                is_update\u003dFalse):"},{"line_number":67,"context_line":"        if not is_update:"},{"line_number":68,"context_line":"            LOG.debug(\"Handling CREATE event prt_fwd %s\", pf_obj)"},{"line_number":69,"context_line":"            LOG.info(\"CREATE for port-forwarding %s vip %s:%s to %s:%s\","},{"line_number":70,"context_line":"                     pf_obj.protocol,"},{"line_number":71,"context_line":"                     pf_obj.floating_ip_address, pf_obj.external_port,"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_2d76d03f","line":68,"in_reply_to":"bf51134e_06d14d30","updated":"2020-07-22 22:56:20.000000000","message":"The debug message carries more information about the pf_obj.\nBut I think we should be okay w/out that.\nDone.","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"cf6a2a084939f5e61dad92104138bfbda222330c","unresolved":false,"context_lines":[{"line_number":170,"context_line":"                    self._handler.port_forwarding_deleted(ovn_txn, ovn_nb,"},{"line_number":171,"context_line":"                        pf_payload.original_pf)"},{"line_number":172,"context_line":"            else:"},{"line_number":173,"context_line":"                LOG.debug(\"Ignoring unexpected event type %s\", event_type)"},{"line_number":174,"context_line":"                return"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"            # Collect the revision numbers of all floating ips visited and"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_c08d80d4","line":173,"range":{"start_line":173,"start_character":16,"end_line":173,"end_character":74},"updated":"2020-07-21 14:08:51.000000000","message":"I believe it is not possible to execute this code.","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8070946b7d95528ce9493a51ed2006312fff7fea","unresolved":false,"context_lines":[{"line_number":170,"context_line":"                    self._handler.port_forwarding_deleted(ovn_txn, ovn_nb,"},{"line_number":171,"context_line":"                        pf_payload.original_pf)"},{"line_number":172,"context_line":"            else:"},{"line_number":173,"context_line":"                LOG.debug(\"Ignoring unexpected event type %s\", event_type)"},{"line_number":174,"context_line":"                return"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"            # Collect the revision numbers of all floating ips visited and"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_e6eb790f","line":173,"range":{"start_line":173,"start_character":16,"end_line":173,"end_character":74},"in_reply_to":"bf51134e_c08d80d4","updated":"2020-07-21 17:16:58.000000000","message":"Correct, _handle_notification is subscribed only to after_create, update and delete","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":170,"context_line":"                    self._handler.port_forwarding_deleted(ovn_txn, ovn_nb,"},{"line_number":171,"context_line":"                        pf_payload.original_pf)"},{"line_number":172,"context_line":"            else:"},{"line_number":173,"context_line":"                LOG.debug(\"Ignoring unexpected event type %s\", event_type)"},{"line_number":174,"context_line":"                return"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"            # Collect the revision numbers of all floating ips visited and"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_6db608df","line":173,"range":{"start_line":173,"start_character":16,"end_line":173,"end_character":74},"in_reply_to":"bf51134e_e6eb790f","updated":"2020-07-22 22:56:20.000000000","message":"ha, funny how I had this added because bhaley kinda asked me to [1]. I will remove it but if bhaley complains, you will have to arm wrestle with him. ;)\n\n[1]: https://review.opendev.org/#/c/723863/26/neutron/services/portforwarding/ovn/port_forwarding.py@173","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"cf6a2a084939f5e61dad92104138bfbda222330c","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            # one for each protocol."},{"line_number":180,"context_line":"            fip_objs \u003d self._get_fip_objs(context, payload)"},{"line_number":181,"context_line":"            for floatingip_id, fip_obj in fip_objs.items():"},{"line_number":182,"context_line":"                fip_revision \u003d str(fip_obj.get(\u0027revision_number\u0027, -1))"},{"line_number":183,"context_line":"                self._handler.port_forwarding_update_revision_number("},{"line_number":184,"context_line":"                    ovn_txn, ovn_nb, floatingip_id, fip_revision)"},{"line_number":185,"context_line":"        # Update revision of affected floating ips. Note that even in"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_e07ec4ad","line":182,"range":{"start_line":182,"start_character":31,"end_line":182,"end_character":34},"updated":"2020-07-21 14:08:51.000000000","message":"Why this conversion?","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            # one for each protocol."},{"line_number":180,"context_line":"            fip_objs \u003d self._get_fip_objs(context, payload)"},{"line_number":181,"context_line":"            for floatingip_id, fip_obj in fip_objs.items():"},{"line_number":182,"context_line":"                fip_revision \u003d str(fip_obj.get(\u0027revision_number\u0027, -1))"},{"line_number":183,"context_line":"                self._handler.port_forwarding_update_revision_number("},{"line_number":184,"context_line":"                    ovn_txn, ovn_nb, floatingip_id, fip_revision)"},{"line_number":185,"context_line":"        # Update revision of affected floating ips. Note that even in"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_edf7b88b","line":182,"range":{"start_line":182,"start_character":31,"end_line":182,"end_character":34},"in_reply_to":"bf51134e_e07ec4ad","updated":"2020-07-22 22:56:20.000000000","message":"That is how it gets stored in the db. Ref [1]. I will move this to a central location, but am afraid I cannot avoid doing it.\n\n[1]: https://review.opendev.org/#/c/741303/7/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"cf6a2a084939f5e61dad92104138bfbda222330c","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                    self._handler.port_forwarding_created("},{"line_number":206,"context_line":"                        ovn_txn, ovn_nb, pf_obj)"},{"line_number":207,"context_line":"                fip_obj \u003d self._l3_plugin.get_floatingip(context, fip_id)"},{"line_number":208,"context_line":"                fip_revision \u003d str(fip_obj.get(\u0027revision_number\u0027, -1))"},{"line_number":209,"context_line":"                self._handler.port_forwarding_update_revision_number(ovn_txn,"},{"line_number":210,"context_line":"                    ovn_nb, fip_id, fip_revision)"},{"line_number":211,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_202f1ca8","line":208,"range":{"start_line":208,"start_character":31,"end_line":208,"end_character":34},"updated":"2020-07-21 14:08:51.000000000","message":"ditto","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":205,"context_line":"                    self._handler.port_forwarding_created("},{"line_number":206,"context_line":"                        ovn_txn, ovn_nb, pf_obj)"},{"line_number":207,"context_line":"                fip_obj \u003d self._l3_plugin.get_floatingip(context, fip_id)"},{"line_number":208,"context_line":"                fip_revision \u003d str(fip_obj.get(\u0027revision_number\u0027, -1))"},{"line_number":209,"context_line":"                self._handler.port_forwarding_update_revision_number(ovn_txn,"},{"line_number":210,"context_line":"                    ovn_nb, fip_id, fip_revision)"},{"line_number":211,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_6d29e8f0","line":208,"range":{"start_line":208,"start_character":31,"end_line":208,"end_character":34},"in_reply_to":"bf51134e_202f1ca8","updated":"2020-07-22 22:56:20.000000000","message":"see above. ;^)","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"cf6a2a084939f5e61dad92104138bfbda222330c","unresolved":false,"context_lines":[{"line_number":242,"context_line":"        for pf_obj in pf_objs:"},{"line_number":243,"context_line":"            self._handler.port_forwarding_created(ovn_txn, ovn_nb, pf_obj)"},{"line_number":244,"context_line":"        fip_obj \u003d self._l3_plugin.get_floatingip(context, fip_id)"},{"line_number":245,"context_line":"        fip_revision \u003d str(fip_obj.get(\u0027revision_number\u0027, -1))"},{"line_number":246,"context_line":"        self._handler.port_forwarding_update_revision_number(ovn_txn,"},{"line_number":247,"context_line":"            ovn_nb, fip_id, fip_revision)"},{"line_number":248,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_c029009c","line":245,"range":{"start_line":245,"start_character":23,"end_line":245,"end_character":26},"updated":"2020-07-21 14:08:51.000000000","message":"ditto","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":242,"context_line":"        for pf_obj in pf_objs:"},{"line_number":243,"context_line":"            self._handler.port_forwarding_created(ovn_txn, ovn_nb, pf_obj)"},{"line_number":244,"context_line":"        fip_obj \u003d self._l3_plugin.get_floatingip(context, fip_id)"},{"line_number":245,"context_line":"        fip_revision \u003d str(fip_obj.get(\u0027revision_number\u0027, -1))"},{"line_number":246,"context_line":"        self._handler.port_forwarding_update_revision_number(ovn_txn,"},{"line_number":247,"context_line":"            ovn_nb, fip_id, fip_revision)"},{"line_number":248,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_a87c8e72","line":245,"range":{"start_line":245,"start_character":23,"end_line":245,"end_character":26},"in_reply_to":"bf51134e_c029009c","updated":"2020-07-22 22:56:20.000000000","message":"See above :^)","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"cf6a2a084939f5e61dad92104138bfbda222330c","unresolved":false,"context_lines":[{"line_number":254,"context_line":""},{"line_number":255,"context_line":"    @staticmethod"},{"line_number":256,"context_line":"    def ovn_lb_protocol(pf_protocol):"},{"line_number":257,"context_line":"        return LB_PROTOCOL_MAP.get(pf_protocol, \u0027\u0027)"},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"    @registry.receives(PORT_FORWARDING_PLUGIN, [events.AFTER_INIT])"},{"line_number":260,"context_line":"    def register(self, resource, event, trigger, payload\u003dNone):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_9b101952","line":257,"range":{"start_line":257,"start_character":48,"end_line":257,"end_character":50},"updated":"2020-07-21 14:08:51.000000000","message":"Hmm, maybe raise kind-of NotImplemented exception while asking for a PF protocol that is not supported? I don\u0027t know if there is this kind of exception handling for this exception, just asking.\n\nIf OVN LB will not have the protocol chosen, it will use TCP instead (if port will be used, but usually we use the port for pf :P).","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":254,"context_line":""},{"line_number":255,"context_line":"    @staticmethod"},{"line_number":256,"context_line":"    def ovn_lb_protocol(pf_protocol):"},{"line_number":257,"context_line":"        return LB_PROTOCOL_MAP.get(pf_protocol, \u0027\u0027)"},{"line_number":258,"context_line":""},{"line_number":259,"context_line":"    @registry.receives(PORT_FORWARDING_PLUGIN, [events.AFTER_INIT])"},{"line_number":260,"context_line":"    def register(self, resource, event, trigger, payload\u003dNone):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_e848462f","line":257,"range":{"start_line":257,"start_character":48,"end_line":257,"end_character":50},"in_reply_to":"bf51134e_9b101952","updated":"2020-07-22 22:56:20.000000000","message":"As asked by bhaley on [1], we have a more restrict version of this on line 34. But it makes more sense for this public function to return the default lb that OVN uses, as you described. Done.\n\n[1]: https://review.opendev.org/#/c/723863/26/neutron/services/portforwarding/ovn/port_forwarding.py@37","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"}],"neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py":[{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"cf6a2a084939f5e61dad92104138bfbda222330c","unresolved":false,"context_lines":[{"line_number":1383,"context_line":"                    monitor_nats)"},{"line_number":1384,"context_line":""},{"line_number":1385,"context_line":"    def _validate_fip_port_forwarding(self, should_match\u003dTrue):"},{"line_number":1386,"context_line":"        Fip_pf_cmp \u003d namedtuple("},{"line_number":1387,"context_line":"            \u0027Fip_pf_cmp\u0027,"},{"line_number":1388,"context_line":"            \u0027fip_id proto rtr_name ext_ip ext_port int_ip int_port\u0027)"},{"line_number":1389,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_fb4f9567","line":1386,"range":{"start_line":1386,"start_character":8,"end_line":1386,"end_character":18},"updated":"2020-07-21 14:08:51.000000000","message":"fip_pf_cmp","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":1383,"context_line":"                    monitor_nats)"},{"line_number":1384,"context_line":""},{"line_number":1385,"context_line":"    def _validate_fip_port_forwarding(self, should_match\u003dTrue):"},{"line_number":1386,"context_line":"        Fip_pf_cmp \u003d namedtuple("},{"line_number":1387,"context_line":"            \u0027Fip_pf_cmp\u0027,"},{"line_number":1388,"context_line":"            \u0027fip_id proto rtr_name ext_ip ext_port int_ip int_port\u0027)"},{"line_number":1389,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_1bb7cd3c","line":1386,"range":{"start_line":1386,"start_character":8,"end_line":1386,"end_character":18},"in_reply_to":"bf51134e_fb4f9567","updated":"2020-07-22 22:56:20.000000000","message":"Done","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":24791,"name":"Maciej Jozefczyk","email":"jeicam.pl@gmail.com","username":"maciej.jozefczyk"},"change_message_id":"cf6a2a084939f5e61dad92104138bfbda222330c","unresolved":false,"context_lines":[{"line_number":1384,"context_line":""},{"line_number":1385,"context_line":"    def _validate_fip_port_forwarding(self, should_match\u003dTrue):"},{"line_number":1386,"context_line":"        Fip_pf_cmp \u003d namedtuple("},{"line_number":1387,"context_line":"            \u0027Fip_pf_cmp\u0027,"},{"line_number":1388,"context_line":"            \u0027fip_id proto rtr_name ext_ip ext_port int_ip int_port\u0027)"},{"line_number":1389,"context_line":""},{"line_number":1390,"context_line":"        # Helper function to break a single ovn lb entry into multiple"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_db48d15d","line":1387,"range":{"start_line":1387,"start_character":13,"end_line":1387,"end_character":23},"updated":"2020-07-21 14:08:51.000000000","message":"fip_pf_cmp","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"},{"author":{"_account_id":11952,"name":"Flavio Fernandes","email":"flavio@flaviof.com","username":"ffernand"},"change_message_id":"65d960abfca2f835c4ba484585c1f2b516186ab7","unresolved":false,"context_lines":[{"line_number":1384,"context_line":""},{"line_number":1385,"context_line":"    def _validate_fip_port_forwarding(self, should_match\u003dTrue):"},{"line_number":1386,"context_line":"        Fip_pf_cmp \u003d namedtuple("},{"line_number":1387,"context_line":"            \u0027Fip_pf_cmp\u0027,"},{"line_number":1388,"context_line":"            \u0027fip_id proto rtr_name ext_ip ext_port int_ip int_port\u0027)"},{"line_number":1389,"context_line":""},{"line_number":1390,"context_line":"        # Helper function to break a single ovn lb entry into multiple"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_fbb13946","line":1387,"range":{"start_line":1387,"start_character":13,"end_line":1387,"end_character":23},"in_reply_to":"bf51134e_db48d15d","updated":"2020-07-22 22:56:20.000000000","message":"Done","commit_id":"1a03828d35d3aec6dbf67e9fd66799638a413288"}]}