)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4d5db11427f4ce24e5ed139393c2375eb3c401ca","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"605ba4e5_957c14f1","updated":"2022-03-16 08:45:07.000000000","message":"recheck","commit_id":"cc3fb8651e1d6ecc6ee3c077b0a7fb0a85098bda"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"63318b3834d06d5d88cbe429f40735f7fcefbe5e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"7457c7f4_4e426b0d","updated":"2022-03-15 13:47:42.000000000","message":"recheck","commit_id":"cc3fb8651e1d6ecc6ee3c077b0a7fb0a85098bda"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a8e76f1e9c8864fe6fdaa0faf83e2d032c3661db","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c13048b5_e9807a2d","updated":"2022-03-15 09:29:47.000000000","message":"recheck","commit_id":"cc3fb8651e1d6ecc6ee3c077b0a7fb0a85098bda"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"aa90691d2d5f1ad70669e95621204bc65c467ddc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"d8332cc8_6d6408aa","updated":"2022-03-15 18:22:54.000000000","message":"recheck","commit_id":"cc3fb8651e1d6ecc6ee3c077b0a7fb0a85098bda"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"a42368013eeeab26ea55f784320ac65d6f5b082b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"68421fdd_63e1c5ae","updated":"2022-03-23 14:08:55.000000000","message":"Looks good to me. One nit that I\u0027m leaving the W+1 off just in case the method should be removed because it\u0027s unused. Otherwise, I say \"ship it!\"","commit_id":"7b19eb92a26ddb5acf89cacee1e436e7ba1c6fa9"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"aeaeb6590ca672b698d0d352c61637e1e8b59a39","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"9b356c4a_aab7d232","updated":"2022-03-23 15:12:28.000000000","message":"Re-adding +2, latest patch-set only removes unused code","commit_id":"2d1b4fd80f1317d741a6e29219e5a230513a4762"}],"neutron/common/ovn/utils.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"a42368013eeeab26ea55f784320ac65d6f5b082b","unresolved":true,"context_lines":[{"line_number":438,"context_line":""},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"def get_lrouter_gw_port_id(ovn_router):"},{"line_number":441,"context_line":"    external_ids \u003d getattr(ovn_router, \u0027external_ids\u0027)"},{"line_number":442,"context_line":"    return external_ids.get(constants.OVN_GW_PORT_EXT_ID_KEY)"},{"line_number":443,"context_line":""},{"line_number":444,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"6170022f_2fb770e5","line":441,"range":{"start_line":441,"start_character":4,"end_line":441,"end_character":54},"updated":"2022-03-23 14:08:55.000000000","message":"*super nit*: we have room for ovn_router.external_ids below, and getattr will throw the same AttributeError as that would if external_ids wasn\u0027t there.\n\nAlso, I don\u0027t see this method used anywhere here or the other patch in the chain. Is it used in a follow up patch?","commit_id":"7b19eb92a26ddb5acf89cacee1e436e7ba1c6fa9"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"42cf25c90e94a4ad4690fc8fb249be36fe6fae51","unresolved":false,"context_lines":[{"line_number":438,"context_line":""},{"line_number":439,"context_line":""},{"line_number":440,"context_line":"def get_lrouter_gw_port_id(ovn_router):"},{"line_number":441,"context_line":"    external_ids \u003d getattr(ovn_router, \u0027external_ids\u0027)"},{"line_number":442,"context_line":"    return external_ids.get(constants.OVN_GW_PORT_EXT_ID_KEY)"},{"line_number":443,"context_line":""},{"line_number":444,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"070492e3_136fad9d","line":441,"range":{"start_line":441,"start_character":4,"end_line":441,"end_character":54},"in_reply_to":"6170022f_2fb770e5","updated":"2022-03-23 15:09:15.000000000","message":"Hmmm I think I created this method in a previous implementation. I\u0027ll remove it","commit_id":"7b19eb92a26ddb5acf89cacee1e436e7ba1c6fa9"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"33e46f340032c07c3c969146e2379d15211935fb","unresolved":true,"context_lines":[{"line_number":393,"context_line":"            # NOTE(ralonsoh): when the gateway network is detached, the gateway"},{"line_number":394,"context_line":"            # port is deleted. Any QoS policy related to this port_id is"},{"line_number":395,"context_line":"            # deleted in \"self.update_port()\"."},{"line_number":396,"context_line":"            LOG.debug(\u0027Router %s does not have ID or gateway assigned\u0027, router)"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"        admin_context \u003d n_context.get_admin_context()"},{"line_number":399,"context_line":"        qos_rules \u003d self._qos_rules(admin_context, qos_policy_id)"}],"source_content_type":"text/x-python","patch_set":5,"id":"18eed048_f9f9cb78","line":396,"updated":"2022-03-16 14:26:23.000000000","message":"shouldn\u0027t You return here?","commit_id":"36eb4f2efea59852419fd55bbdef4ea406aafa31"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1a8dd8782276a46b86535d17551c646a09abd6c4","unresolved":true,"context_lines":[{"line_number":393,"context_line":"            # NOTE(ralonsoh): when the gateway network is detached, the gateway"},{"line_number":394,"context_line":"            # port is deleted. Any QoS policy related to this port_id is"},{"line_number":395,"context_line":"            # deleted in \"self.update_port()\"."},{"line_number":396,"context_line":"            LOG.debug(\u0027Router %s does not have ID or gateway assigned\u0027, router)"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"        admin_context \u003d n_context.get_admin_context()"},{"line_number":399,"context_line":"        qos_rules \u003d self._qos_rules(admin_context, qos_policy_id)"}],"source_content_type":"text/x-python","patch_set":5,"id":"d58c5b0e_b0909b81","line":396,"in_reply_to":"18eed048_f9f9cb78","updated":"2022-03-16 17:48:47.000000000","message":"Exactly. And this is working because L399 is not returning any rule.\n\nIn any case, you are right: we need to exit here.","commit_id":"36eb4f2efea59852419fd55bbdef4ea406aafa31"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e6fdfa21904225f4fdae1afd60b3ce3fb4d1422d","unresolved":false,"context_lines":[{"line_number":393,"context_line":"            # NOTE(ralonsoh): when the gateway network is detached, the gateway"},{"line_number":394,"context_line":"            # port is deleted. Any QoS policy related to this port_id is"},{"line_number":395,"context_line":"            # deleted in \"self.update_port()\"."},{"line_number":396,"context_line":"            LOG.debug(\u0027Router %s does not have ID or gateway assigned\u0027, router)"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"        admin_context \u003d n_context.get_admin_context()"},{"line_number":399,"context_line":"        qos_rules \u003d self._qos_rules(admin_context, qos_policy_id)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1c0cfeb9_cd664ac1","line":396,"in_reply_to":"d58c5b0e_b0909b81","updated":"2022-03-16 17:51:12.000000000","message":"Done","commit_id":"36eb4f2efea59852419fd55bbdef4ea406aafa31"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"a42368013eeeab26ea55f784320ac65d6f5b082b","unresolved":false,"context_lines":[{"line_number":761,"context_line":"            cmds.append(self._nb_idl.db_set("},{"line_number":762,"context_line":"                \u0027Logical_Router\u0027, lr.uuid, (\u0027external_ids\u0027, external_ids)))"},{"line_number":763,"context_line":""},{"line_number":764,"context_line":"        if cmds:"},{"line_number":765,"context_line":"            with self._nb_idl.transaction(check_error\u003dTrue) as txn:"},{"line_number":766,"context_line":"                for cmd in cmds:"},{"line_number":767,"context_line":"                    txn.add(cmd)"}],"source_content_type":"text/x-python","patch_set":7,"id":"e40f99ec_b9a2f86d","line":764,"updated":"2022-03-23 14:08:55.000000000","message":"Seems like a utility function we could add to ovsdbapp. Normally I\u0027d just put the with block outside the for, but it does lead to some excessive indentation.","commit_id":"7b19eb92a26ddb5acf89cacee1e436e7ba1c6fa9"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4d581d0ba9db89a9e91704308303e714bd548b7a","unresolved":true,"context_lines":[{"line_number":1278,"context_line":"        return networks"},{"line_number":1279,"context_line":""},{"line_number":1280,"context_line":"    def _gen_router_ext_ids(self, router):"},{"line_number":1281,"context_line":"        gw_net_id \u003d (router.get(\u0027external_gateway_info\u0027) or"},{"line_number":1282,"context_line":"                     {}).get(\u0027network_id\u0027) or \u0027\u0027"},{"line_number":1283,"context_line":"        return {"},{"line_number":1284,"context_line":"            ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:"},{"line_number":1285,"context_line":"                router.get(\u0027name\u0027, \u0027no_router_name\u0027),"}],"source_content_type":"text/x-python","patch_set":6,"id":"e2590052_9a74111e","line":1282,"range":{"start_line":1281,"start_character":0,"end_line":1282,"end_character":48},"updated":"2022-03-23 08:53:40.000000000","message":"We can use get()\u0027s default values instead of the \"or\" to improve readability here:\n\nrouter.get(\u0027external_gateway_info\u0027, {}).get(\u0027network_id\u0027, \u0027\u0027)","commit_id":"0304b10d6ecafb0af226fcf12fddabf7f6160d0c"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"a42368013eeeab26ea55f784320ac65d6f5b082b","unresolved":false,"context_lines":[{"line_number":1278,"context_line":"        return networks"},{"line_number":1279,"context_line":""},{"line_number":1280,"context_line":"    def _gen_router_ext_ids(self, router):"},{"line_number":1281,"context_line":"        gw_net_id \u003d (router.get(\u0027external_gateway_info\u0027) or"},{"line_number":1282,"context_line":"                     {}).get(\u0027network_id\u0027) or \u0027\u0027"},{"line_number":1283,"context_line":"        return {"},{"line_number":1284,"context_line":"            ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:"},{"line_number":1285,"context_line":"                router.get(\u0027name\u0027, \u0027no_router_name\u0027),"}],"source_content_type":"text/x-python","patch_set":6,"id":"22cf05d2_c3a42b21","line":1282,"range":{"start_line":1281,"start_character":0,"end_line":1282,"end_character":48},"in_reply_to":"416f069c_a2c8389f","updated":"2022-03-23 14:08:55.000000000","message":"Ahhh. *goes back to delete one of his previous comments in this review*","commit_id":"0304b10d6ecafb0af226fcf12fddabf7f6160d0c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"27ccebbe152456849107f0b276ee8243bcf39a51","unresolved":true,"context_lines":[{"line_number":1278,"context_line":"        return networks"},{"line_number":1279,"context_line":""},{"line_number":1280,"context_line":"    def _gen_router_ext_ids(self, router):"},{"line_number":1281,"context_line":"        gw_net_id \u003d (router.get(\u0027external_gateway_info\u0027) or"},{"line_number":1282,"context_line":"                     {}).get(\u0027network_id\u0027) or \u0027\u0027"},{"line_number":1283,"context_line":"        return {"},{"line_number":1284,"context_line":"            ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY:"},{"line_number":1285,"context_line":"                router.get(\u0027name\u0027, \u0027no_router_name\u0027),"}],"source_content_type":"text/x-python","patch_set":6,"id":"416f069c_a2c8389f","line":1282,"range":{"start_line":1281,"start_character":0,"end_line":1282,"end_character":48},"in_reply_to":"e2590052_9a74111e","updated":"2022-03-23 09:18:56.000000000","message":"I tried router.get(\u0027external_gateway_info\u0027, {}) but sometimes \"external_gateway_info\" has an empty string. That means \"get\" will return \"\", not {}.","commit_id":"0304b10d6ecafb0af226fcf12fddabf7f6160d0c"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"a42368013eeeab26ea55f784320ac65d6f5b082b","unresolved":false,"context_lines":[{"line_number":1288,"context_line":"            ovn_const.OVN_REV_NUM_EXT_ID_KEY: str(utils.get_revision_number("},{"line_number":1289,"context_line":"                router, ovn_const.TYPE_ROUTERS)),"},{"line_number":1290,"context_line":"            ovn_const.OVN_AZ_HINTS_EXT_ID_KEY:"},{"line_number":1291,"context_line":"                \u0027,\u0027.join(common_utils.get_az_hints(router)),"},{"line_number":1292,"context_line":"            ovn_const.OVN_GW_NETWORK_EXT_ID_KEY: gw_net_id,"},{"line_number":1293,"context_line":"        }"},{"line_number":1294,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5564cd41_9004393d","line":1291,"range":{"start_line":1291,"start_character":16,"end_line":1291,"end_character":59},"updated":"2022-03-23 14:08:55.000000000","message":"I realize it\u0027s not part of this change, but this reminds me that I want to go through all of the external_ids where we store \"variously delimited lists\" and convert them all to json. It\u0027s probably possible to even make the access in the Row do a json.loads that if it fails to parse, returns the original value for backward compatibility...","commit_id":"7b19eb92a26ddb5acf89cacee1e436e7ba1c6fa9"}]}
