)]}'
{"neutron/agent/l3/agent.py":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"9332c19f6bea8d69c7fda003ccb25570caaf69a2","unresolved":false,"context_lines":[{"line_number":395,"context_line":""},{"line_number":396,"context_line":"        try:"},{"line_number":397,"context_line":"            self._router_removed(router_id)"},{"line_number":398,"context_line":"            self.l3_ext_manager.delete_router(self.context, router_id)"},{"line_number":399,"context_line":"        except Exception:"},{"line_number":400,"context_line":"            LOG.exception(\u0027Error while deleting router %s\u0027, router_id)"},{"line_number":401,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_45ca117b","side":"PARENT","line":398,"range":{"start_line":398,"start_character":60,"end_line":398,"end_character":69},"updated":"2018-12-20 01:53:02.000000000","message":"Seems qos gateway ip is first one influenced by this, my bad...","commit_id":"8a9439975ca50b30a89b4e1f31db8a11384da289"},{"author":{"_account_id":25618,"name":"Paweł Suder","email":"pawel@suder.info","username":"SuderPawel"},"change_message_id":"2efdbda2c783789a33f37af92ec9567c95e8ed79","unresolved":false,"context_lines":[{"line_number":394,"context_line":"        \"\"\"Try to delete a router and return True if successful.\"\"\""},{"line_number":395,"context_line":"        # The l3_ext_manager API expects a router, make a dict with the ID"},{"line_number":396,"context_line":"        # if one was not passed."},{"line_number":397,"context_line":"        if not router:"},{"line_number":398,"context_line":"            router \u003d {\u0027id\u0027: router_id}"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        try:"},{"line_number":401,"context_line":"            self._router_removed(router_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_011ab068","line":398,"range":{"start_line":397,"start_character":0,"end_line":398,"end_character":38},"updated":"2018-12-21 08:46:43.000000000","message":"It does not smell very nice. IMO change of types of method params should be hardly applied and trying to be backward compatible is not good :)","commit_id":"251560718f94788936be00c15a49a12d60758fa6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7463273aae673e39771dfce6fdcfa9c6e89fbee8","unresolved":false,"context_lines":[{"line_number":394,"context_line":"        \"\"\"Try to delete a router and return True if successful.\"\"\""},{"line_number":395,"context_line":"        # The l3_ext_manager API expects a router, make a dict with the ID"},{"line_number":396,"context_line":"        # if one was not passed."},{"line_number":397,"context_line":"        if not router:"},{"line_number":398,"context_line":"            router \u003d {\u0027id\u0027: router_id}"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        try:"},{"line_number":401,"context_line":"            self._router_removed(router_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_41f73e2a","line":398,"range":{"start_line":397,"start_character":0,"end_line":398,"end_character":38},"in_reply_to":"3f79a3b5_011ab068","updated":"2018-12-21 23:25:25.000000000","message":"I think the qos code is the only consumer of the extension api so far, and it\u0027s expecting a dict as the class defined, but passing the whole dict as Liu suggested would be best.","commit_id":"251560718f94788936be00c15a49a12d60758fa6"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"9332c19f6bea8d69c7fda003ccb25570caaf69a2","unresolved":false,"context_lines":[{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        try:"},{"line_number":401,"context_line":"            self._router_removed(router_id)"},{"line_number":402,"context_line":"            self.l3_ext_manager.delete_router(self.context, router)"},{"line_number":403,"context_line":"        except Exception:"},{"line_number":404,"context_line":"            LOG.exception(\u0027Error while deleting router %s\u0027, router_id)"},{"line_number":405,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_45fc51bb","line":402,"range":{"start_line":402,"start_character":60,"end_line":402,"end_character":66},"updated":"2018-12-20 01:53:02.000000000","message":"why not pass real router info dict to this?\nsuch as:\nri \u003d self.router_info.get(router_id)\nself.l3_ext_manager.delete_router(self.context, ri.router)\n\nIt\u0027s basicly safe since such l3_ext_manager call is sequential, line 422 and 423 should not influence here.\n\nSuch simple dict may not well cover some l3 agent extension, since they may needs more router info such as type, gateway_info, status etc.","commit_id":"251560718f94788936be00c15a49a12d60758fa6"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a2321feb0511c30e2474d189c81b9906b07b49b5","unresolved":false,"context_lines":[{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        try:"},{"line_number":401,"context_line":"            self._router_removed(router_id)"},{"line_number":402,"context_line":"            self.l3_ext_manager.delete_router(self.context, router)"},{"line_number":403,"context_line":"        except Exception:"},{"line_number":404,"context_line":"            LOG.exception(\u0027Error while deleting router %s\u0027, router_id)"},{"line_number":405,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_7a156339","line":402,"range":{"start_line":402,"start_character":60,"end_line":402,"end_character":66},"in_reply_to":"3f79a3b5_45fc51bb","updated":"2018-12-20 08:47:28.000000000","message":"I\u0027m not sure about this and I can give the example of ML2. When an object is deleted, in the agent only the ID is passed. The agent will consider this object doesn\u0027t exist and can\u0027t access to the data contained, because the agent can\u0027t rely anymore in this information.\n\nIMO, we should do just the opposite: change the \"delete\" functions to use only the ID. In L3, I\u0027ve seen the meta class L3AgentExtension, and the child classes FipQosAgentExtension and RouterGatewayIPQosAgentExtension.","commit_id":"251560718f94788936be00c15a49a12d60758fa6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7463273aae673e39771dfce6fdcfa9c6e89fbee8","unresolved":false,"context_lines":[{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        try:"},{"line_number":401,"context_line":"            self._router_removed(router_id)"},{"line_number":402,"context_line":"            self.l3_ext_manager.delete_router(self.context, router)"},{"line_number":403,"context_line":"        except Exception:"},{"line_number":404,"context_line":"            LOG.exception(\u0027Error while deleting router %s\u0027, router_id)"},{"line_number":405,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_c1d4ae1f","line":402,"range":{"start_line":402,"start_character":60,"end_line":402,"end_character":66},"in_reply_to":"3f79a3b5_45fc51bb","updated":"2018-12-21 23:25:25.000000000","message":"Yes, passing the whole dict would be a better answer, even though the only one that cares right now only wants the id.","commit_id":"251560718f94788936be00c15a49a12d60758fa6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7463273aae673e39771dfce6fdcfa9c6e89fbee8","unresolved":false,"context_lines":[{"line_number":399,"context_line":""},{"line_number":400,"context_line":"        try:"},{"line_number":401,"context_line":"            self._router_removed(router_id)"},{"line_number":402,"context_line":"            self.l3_ext_manager.delete_router(self.context, router)"},{"line_number":403,"context_line":"        except Exception:"},{"line_number":404,"context_line":"            LOG.exception(\u0027Error while deleting router %s\u0027, router_id)"},{"line_number":405,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_611d9a76","line":402,"range":{"start_line":402,"start_character":60,"end_line":402,"end_character":66},"in_reply_to":"3f79a3b5_7a156339","updated":"2018-12-21 23:25:25.000000000","message":"The problem is the API was defined as taking a dict, so I don\u0027t think it should change now.","commit_id":"251560718f94788936be00c15a49a12d60758fa6"},{"author":{"_account_id":25618,"name":"Paweł Suder","email":"pawel@suder.info","username":"SuderPawel"},"change_message_id":"4f46ac3af5bebe939cd1c138fd6fafc7dda015e2","unresolved":false,"context_lines":[{"line_number":402,"context_line":"        try:"},{"line_number":403,"context_line":"            self._router_removed(router_id)"},{"line_number":404,"context_line":"            if router:"},{"line_number":405,"context_line":"                self.l3_ext_manager.delete_router(self.context, router)"},{"line_number":406,"context_line":"        except Exception:"},{"line_number":407,"context_line":"            LOG.exception(\u0027Error while deleting router %s\u0027, router_id)"},{"line_number":408,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f769fc5_bb792f94","line":405,"range":{"start_line":405,"start_character":21,"end_line":405,"end_character":49},"updated":"2019-01-02 09:06:54.000000000","message":"Why this method does require object router, not router_id?","commit_id":"12711463a8b7238dd865a4325de4a4206a4e7af9"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"44f3f688263033e176fba22ca9058bb0bf28b6db","unresolved":false,"context_lines":[{"line_number":402,"context_line":"        try:"},{"line_number":403,"context_line":"            self._router_removed(router_id)"},{"line_number":404,"context_line":"            if router:"},{"line_number":405,"context_line":"                self.l3_ext_manager.delete_router(self.context, router)"},{"line_number":406,"context_line":"        except Exception:"},{"line_number":407,"context_line":"            LOG.exception(\u0027Error while deleting router %s\u0027, router_id)"},{"line_number":408,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffd0ebdf_4fb5f7ed","line":405,"range":{"start_line":405,"start_character":21,"end_line":405,"end_character":49},"in_reply_to":"1f769fc5_bb792f94","updated":"2019-01-03 22:29:42.000000000","message":"That\u0027s what the l3_ext_manager API defines as the argument, and is the bug I\u0027m fixing since there is a consumer throwing an exception when we don\u0027t pass the dict as we do in the other *_router calls.","commit_id":"12711463a8b7238dd865a4325de4a4206a4e7af9"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3163ffaadf75c4bbe9328ab08b1a0b21f4805f70","unresolved":false,"context_lines":[{"line_number":402,"context_line":"        try:"},{"line_number":403,"context_line":"            self._router_removed(router_id)"},{"line_number":404,"context_line":"            if router:"},{"line_number":405,"context_line":"                self.l3_ext_manager.delete_router(self.context, router)"},{"line_number":406,"context_line":"        except Exception:"},{"line_number":407,"context_line":"            LOG.exception(\u0027Error while deleting router %s\u0027, router_id)"},{"line_number":408,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffd0ebdf_ce0474c9","line":405,"range":{"start_line":405,"start_character":21,"end_line":405,"end_character":49},"in_reply_to":"ffd0ebdf_4fb5f7ed","updated":"2019-01-04 22:02:16.000000000","message":"So I\u0027m beginning to like my original version (hack) better after looking a this again.  The reason is that on a restart the l3-agent could be deleting stale routers, and in that case there is no router_info for those routers, just IDs.  So we won\u0027t call the l3_ext_manager.delete_router() code in that case.\n\nOr we fix the caller that\u0027s throwing the exception and make it clear in the API that this method passes just the ID, since that\u0027s actually is what it\u0027s done since it was added in 2016.\n\nBut after thinking some more seems like the correct thing to do is to just have this method look up the router_info, and if found make the *.delete_router() call, since if there is none the namespace is going to be cleaned, and the extension probably has no knowledge the router existed.","commit_id":"12711463a8b7238dd865a4325de4a4206a4e7af9"},{"author":{"_account_id":25618,"name":"Paweł Suder","email":"pawel@suder.info","username":"SuderPawel"},"change_message_id":"4f46ac3af5bebe939cd1c138fd6fafc7dda015e2","unresolved":false,"context_lines":[{"line_number":539,"context_line":"                        router_update.id, router_update.action)"},{"line_number":540,"context_line":"            if router_update.action !\u003d DELETE_ROUTER:"},{"line_number":541,"context_line":"                LOG.debug(\"Deleting router %s\", router_update.id)"},{"line_number":542,"context_line":"                self._safe_router_removed(router_update.id,"},{"line_number":543,"context_line":"                                          router_update.resource)"},{"line_number":544,"context_line":"            return"},{"line_number":545,"context_line":"        router_update.timestamp \u003d timeutils.utcnow()"},{"line_number":546,"context_line":"        router_update.priority \u003d priority"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f769fc5_fb456744","line":543,"range":{"start_line":542,"start_character":16,"end_line":543,"end_character":65},"updated":"2019-01-02 09:06:54.000000000","message":"Is the only one place where change of order of params is needed?\n\nIf this is the only place, why not to change declaration of _safe_router_removed to accept only router_update.resource (router object)?","commit_id":"12711463a8b7238dd865a4325de4a4206a4e7af9"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"44f3f688263033e176fba22ca9058bb0bf28b6db","unresolved":false,"context_lines":[{"line_number":539,"context_line":"                        router_update.id, router_update.action)"},{"line_number":540,"context_line":"            if router_update.action !\u003d DELETE_ROUTER:"},{"line_number":541,"context_line":"                LOG.debug(\"Deleting router %s\", router_update.id)"},{"line_number":542,"context_line":"                self._safe_router_removed(router_update.id,"},{"line_number":543,"context_line":"                                          router_update.resource)"},{"line_number":544,"context_line":"            return"},{"line_number":545,"context_line":"        router_update.timestamp \u003d timeutils.utcnow()"},{"line_number":546,"context_line":"        router_update.priority \u003d priority"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffd0ebdf_6feb1bb0","line":543,"range":{"start_line":542,"start_character":16,"end_line":543,"end_character":65},"in_reply_to":"1f769fc5_fb456744","updated":"2019-01-03 22:29:42.000000000","message":"From inspection, there is a caller that can create an update without a resource assigned, network_update -\u003e _resync_router, but it actually has the router_info, so it could pass it even if it gets tossed below on L547.  So perhaps that\u0027s enough to change this to just pass the resource and not the ID.","commit_id":"12711463a8b7238dd865a4325de4a4206a4e7af9"}]}