)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"f4e98a10a8ba6e34b5f117144769a0b1cef42c8e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"5743f4e7_7a053950","updated":"2022-03-15 08:38:12.000000000","message":"It seems that functional tests failure may be related to this patch. Please check it before rechecking.","commit_id":"6dc11b84671bff6706fdab62aafeb845487307fb"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"a981481f7fad32317d7217b128e7df29a999b55b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"02903980_5cd28088","updated":"2022-03-15 08:54:51.000000000","message":"besides slaweq point, do you also need to add the loadbalancer to newly created subnet/LS, and therefore not only having to react to port_forwarding events, but also, once there is a port_forwarding loadbalancer make sure newly added logical switches gets also the loadbalancer added so that its VMs can access it?","commit_id":"6dc11b84671bff6706fdab62aafeb845487307fb"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"a235bf4ef9fc5015fe05bc1a6ce5700809e8467f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"038fe609_53141ec3","in_reply_to":"02903980_5cd28088","updated":"2022-03-15 10:06:12.000000000","message":"no not on creation of subnet/ls but want to add/delete on addition/removal of those subnets to router. Not sure if you mean the same, if yes i tried to do that with [1] on router_interface after create/delete event.\nIf you mean something else can you please share some more details so i can re look.\n\n[1] https://review.opendev.org/c/openstack/neutron/+/833620/2/neutron/services/portforwarding/drivers/ovn/driver.py#330","commit_id":"6dc11b84671bff6706fdab62aafeb845487307fb"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"e1d0320c184f6c70de00fab7486bd416f66d5daf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e8581d82_9b6bbc99","in_reply_to":"038fe609_53141ec3","updated":"2022-03-15 10:16:28.000000000","message":"Ohh, right, that is what I meant, I missed the ROUTER_INTERFACE there! nice!","commit_id":"6dc11b84671bff6706fdab62aafeb845487307fb"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"a235bf4ef9fc5015fe05bc1a6ce5700809e8467f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a976dcd9_a75ec1eb","in_reply_to":"5743f4e7_7a053950","updated":"2022-03-15 10:06:12.000000000","message":"yeap i noticed that it\u0027s happening as ls name is getting None somehow, will check and get it fixed in next PS.","commit_id":"6dc11b84671bff6706fdab62aafeb845487307fb"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"c3b6f5d7f7e4e4ba487cedaaafff03ca6ab53e20","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"76aaf95f_8da5f125","in_reply_to":"a976dcd9_a75ec1eb","updated":"2022-03-15 11:11:30.000000000","message":"Done","commit_id":"6dc11b84671bff6706fdab62aafeb845487307fb"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"c3b6f5d7f7e4e4ba487cedaaafff03ca6ab53e20","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fcb1a4f2_38fc7573","in_reply_to":"e8581d82_9b6bbc99","updated":"2022-03-15 11:11:30.000000000","message":"Thanks for confirmation.","commit_id":"6dc11b84671bff6706fdab62aafeb845487307fb"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"fb308650f971c638dd99bad0552e9519614d868f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"9d2ac38b_82b4fb18","updated":"2022-03-15 13:15:25.000000000","message":"pushed https://review.opendev.org/c/openstack/neutron/+/833779 to fix timeouts in tox-py38","commit_id":"ae3e9477324d4edbdfdcc0fa873a925419cf6f13"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"03fa82e6ac2a04909dc29f4557d26536d42ad9fa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"9cb7eabf_5cd65e2e","updated":"2022-03-16 07:34:33.000000000","message":"Code looks good, just one thing that we hit on the ovn-octavia side, and that could be hit here too","commit_id":"162a4548f167ec328e2eda10ea4e114d997409e3"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"1462284dc0241aa63fd2e178a50deb0e2149ba1d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"790edd8f_4b1991a5","updated":"2022-03-15 15:18:59.000000000","message":"recheck test_log_ovn_unsupported functional failure","commit_id":"162a4548f167ec328e2eda10ea4e114d997409e3"},{"author":{"_account_id":30380,"name":"ZhouHeng","email":"zhouhenglc@inspur.com","username":"zhouhenglc"},"change_message_id":"4649db9b1482d694230ca21b67fe1a136b41d65e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"54a43fa5_b1e1178d","updated":"2022-03-22 09:09:06.000000000","message":"I test this patch , can partial slove bug:#1935959 [1]\n[1] https://bugs.launchpad.net/neutron/+bug/1935959","commit_id":"2492cf2e0703dbe6fcfe26e199505b7b43a65f50"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"0d4702335ab3fd769ed0665df5dfd623a63ca5f4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"f4e0980c_7e2805e5","in_reply_to":"54a43fa5_b1e1178d","updated":"2022-03-24 12:11:19.000000000","message":"Replied for not working scenario in https://bugs.launchpad.net/neutron/+bug/1935959/comments/7, me not sure if it can be handled on neutron/ovn side.","commit_id":"2492cf2e0703dbe6fcfe26e199505b7b43a65f50"}],"neutron/services/portforwarding/drivers/ovn/driver.py":[{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"e0814619f235299684da0eb0e299e8d0918dee8b","unresolved":true,"context_lines":[{"line_number":60,"context_line":"        rtr_name \u003d ovn_utils.ovn_name(payload.resource_id)"},{"line_number":61,"context_line":"        ovn_lr \u003d nb_ovn.get_lrouter(rtr_name)"},{"line_number":62,"context_line":"        if ovn_lr:"},{"line_number":63,"context_line":"            lr_lbs \u003d ovn_lr.load_balancer"},{"line_number":64,"context_line":"            r_port \u003d payload.metadata.get(\u0027port\u0027)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"            if r_port:"}],"source_content_type":"text/x-python","patch_set":2,"id":"bdfa2b19_d8a42ee7","line":63,"range":{"start_line":63,"start_character":12,"end_line":63,"end_character":41},"updated":"2022-03-15 10:28:48.000000000","message":"just realized one more thing, need to filter only lbs managed by port forwarding(i.e \"neutron:device_owner\"\u003dport_forwarding_plugin) ones, else actions may conflict with lbs managed by ovn-octavia-provider.","commit_id":"6dc11b84671bff6706fdab62aafeb845487307fb"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"c3b6f5d7f7e4e4ba487cedaaafff03ca6ab53e20","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        rtr_name \u003d ovn_utils.ovn_name(payload.resource_id)"},{"line_number":61,"context_line":"        ovn_lr \u003d nb_ovn.get_lrouter(rtr_name)"},{"line_number":62,"context_line":"        if ovn_lr:"},{"line_number":63,"context_line":"            lr_lbs \u003d ovn_lr.load_balancer"},{"line_number":64,"context_line":"            r_port \u003d payload.metadata.get(\u0027port\u0027)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"            if r_port:"}],"source_content_type":"text/x-python","patch_set":2,"id":"8e45dcd3_12595db0","line":63,"range":{"start_line":63,"start_character":12,"end_line":63,"end_character":41},"in_reply_to":"bdfa2b19_d8a42ee7","updated":"2022-03-15 11:11:30.000000000","message":"Done","commit_id":"6dc11b84671bff6706fdab62aafeb845487307fb"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"03fa82e6ac2a04909dc29f4557d26536d42ad9fa","unresolved":true,"context_lines":[{"line_number":112,"context_line":"                       for port in ovn_lr.ports"},{"line_number":113,"context_line":"                       if port.external_ids.get(ext_id_key) and"},{"line_number":114,"context_line":"                       not port.gateway_chassis]"},{"line_number":115,"context_line":"            for ls_name in ovn_lss:"},{"line_number":116,"context_line":"                ovn_txn.add(nb_ovn.ls_lb_add(ls_name, lb_name, may_exist\u003dTrue))"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def port_forwarding_created(self, ovn_txn, nb_ovn, pf_obj):"},{"line_number":119,"context_line":"        LOG.info(\"CREATE for port-forwarding %s vip %s:%s to %s:%s\","}],"source_content_type":"text/x-python","patch_set":4,"id":"b40bedf6_db7a1aa5","line":116,"range":{"start_line":115,"start_character":0,"end_line":116,"end_character":79},"updated":"2022-03-16 07:34:33.000000000","message":"this is a chance that, when there are many subnets associated to the same router, that one of the ovn_ls is deleted in between, check https://review.opendev.org/c/openstack/ovn-octavia-provider/+/829126. So, you should catch idlutils.RowNotFound and continue the loop if that is the case","commit_id":"162a4548f167ec328e2eda10ea4e114d997409e3"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"8e933ee4a0f632f0e5d27209d9d47a860f794584","unresolved":false,"context_lines":[{"line_number":112,"context_line":"                       for port in ovn_lr.ports"},{"line_number":113,"context_line":"                       if port.external_ids.get(ext_id_key) and"},{"line_number":114,"context_line":"                       not port.gateway_chassis]"},{"line_number":115,"context_line":"            for ls_name in ovn_lss:"},{"line_number":116,"context_line":"                ovn_txn.add(nb_ovn.ls_lb_add(ls_name, lb_name, may_exist\u003dTrue))"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def port_forwarding_created(self, ovn_txn, nb_ovn, pf_obj):"},{"line_number":119,"context_line":"        LOG.info(\"CREATE for port-forwarding %s vip %s:%s to %s:%s\","}],"source_content_type":"text/x-python","patch_set":4,"id":"c1aa5906_6113b5ed","line":116,"range":{"start_line":115,"start_character":0,"end_line":116,"end_character":79},"in_reply_to":"b40bedf6_db7a1aa5","updated":"2022-03-17 07:00:23.000000000","message":"Done","commit_id":"162a4548f167ec328e2eda10ea4e114d997409e3"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e08d99e8704ae90de1976b298029c27382ce13ff","unresolved":true,"context_lines":[{"line_number":77,"context_line":"                ovn_ls \u003d nb_ovn.get_lswitch(ls_name)"},{"line_number":78,"context_line":"                ls_lbs \u003d ovn_ls.load_balancer"},{"line_number":79,"context_line":"                return lr_lbs, ls_lbs, ls_name"},{"line_number":80,"context_line":"        return [], [], None"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def _add_lb_on_ls(self, ovn_txn, nb_ovn, payload):"},{"line_number":83,"context_line":"        lr_lbs, ls_lbs, ls_name \u003d self._get_lbs_and_ls(nb_ovn, payload)"}],"source_content_type":"text/x-python","patch_set":5,"id":"80a31f3f_0f45e915","line":80,"updated":"2022-04-12 10:30:31.000000000","message":"nitty nit: to avoid return in 2 places in method, You could do something like:\n\n    lr_lbs \u003d []\n    ls_lbs \u003d []\n    ls_name \u003d None\n\nsomewhere in e.g. L66-67 and then here have only:\n\n    return lr_lbs, ls_lbs, ls_name\n\nand remove return statement from L79","commit_id":"2492cf2e0703dbe6fcfe26e199505b7b43a65f50"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"617cadc0f0bf10105f762e9b35d242c7687c5df5","unresolved":true,"context_lines":[{"line_number":77,"context_line":"                ovn_ls \u003d nb_ovn.get_lswitch(ls_name)"},{"line_number":78,"context_line":"                ls_lbs \u003d ovn_ls.load_balancer"},{"line_number":79,"context_line":"                return lr_lbs, ls_lbs, ls_name"},{"line_number":80,"context_line":"        return [], [], None"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def _add_lb_on_ls(self, ovn_txn, nb_ovn, payload):"},{"line_number":83,"context_line":"        lr_lbs, ls_lbs, ls_name \u003d self._get_lbs_and_ls(nb_ovn, payload)"}],"source_content_type":"text/x-python","patch_set":5,"id":"88005561_74b660a5","line":80,"in_reply_to":"80a31f3f_0f45e915","updated":"2022-04-12 11:04:38.000000000","message":"Thanks slawek, Actually i had thought about it initially but avoided the initialization as those vars only be used when ovn_lr exists, But now seems it may be more cleaner like you say. Considering it just a nit will update it if patch need some more revisions.","commit_id":"2492cf2e0703dbe6fcfe26e199505b7b43a65f50"}]}
