)]}'
{"neutron/db/l3_agentschedulers_db.py":[{"author":{"_account_id":17120,"name":"Manjeet Singh Bhatia","email":"manjeet.s.bhatia@intel.com","username":"manjeets"},"change_message_id":"258740e7ff58123c4ba6aa05ac77f7a1461a52dd","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        if not agentschedulers_db.services_available(agent.admin_state_up):"},{"line_number":314,"context_line":"            return []"},{"line_number":315,"context_line":"        if not with_dvr:"},{"line_number":316,"context_line":"            return self._get_router_ids_for_agent_no_dvr("},{"line_number":317,"context_line":"                context, agent, router_ids)"},{"line_number":318,"context_line":"        return self._get_router_ids_for_agent(context, agent, router_ids)"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"    def _get_router_ids_for_agent_no_dvr(self, context, agent, router_ids):"},{"line_number":321,"context_line":"        \"\"\"Get IDs of routers that the agent should host"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffb9cba7_621ec50d","line":318,"range":{"start_line":316,"start_character":12,"end_line":318,"end_character":73},"updated":"2019-04-22 16:46:43.000000000","message":"they both are essentially the same thing, unless _get_router_ids_for_agent_no_dvr is updated later to exclude dvr routers ?","commit_id":"6068094fd51ddfe6cceda63fb98b7fde1c2f7c0a"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"4870b5cf8cb378ecd6f5a085b6d432d9423353bf","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        if not agentschedulers_db.services_available(agent.admin_state_up):"},{"line_number":314,"context_line":"            return []"},{"line_number":315,"context_line":"        if not with_dvr:"},{"line_number":316,"context_line":"            return self._get_router_ids_for_agent_no_dvr("},{"line_number":317,"context_line":"                context, agent, router_ids)"},{"line_number":318,"context_line":"        return self._get_router_ids_for_agent(context, agent, router_ids)"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"    def _get_router_ids_for_agent_no_dvr(self, context, agent, router_ids):"},{"line_number":321,"context_line":"        \"\"\"Get IDs of routers that the agent should host"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffb9cba7_2c3f3726","line":318,"range":{"start_line":316,"start_character":12,"end_line":318,"end_character":73},"in_reply_to":"ffb9cba7_012a3c8f","updated":"2019-04-24 10:21:43.000000000","message":"@Slawek, add parameter to _get_router_ids_for_agent does not save too much work than current approach. This parameter will be needed to add to the method _get_router_ids_for_agent in both this class and l3_dvrscheduler_db class. So we need to change another file, that\u0027s an extra cost.\nAnd for readibility, when developer try to figure out the code path, the current approach will directly point to the right file/class right method. But for that solution you suggested, it will be implicitly pointed to l3_dvrscheduler_db and then back to this file. This is precisely the root cause of this bug (commit message has this information). Problems caused by such implicit inheritance relations.","commit_id":"6068094fd51ddfe6cceda63fb98b7fde1c2f7c0a"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e30a580618b6b5ab00a4a3ed8cdc90c0bb20ab13","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        if not agentschedulers_db.services_available(agent.admin_state_up):"},{"line_number":314,"context_line":"            return []"},{"line_number":315,"context_line":"        if not with_dvr:"},{"line_number":316,"context_line":"            return self._get_router_ids_for_agent_no_dvr("},{"line_number":317,"context_line":"                context, agent, router_ids)"},{"line_number":318,"context_line":"        return self._get_router_ids_for_agent(context, agent, router_ids)"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"    def _get_router_ids_for_agent_no_dvr(self, context, agent, router_ids):"},{"line_number":321,"context_line":"        \"\"\"Get IDs of routers that the agent should host"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffb9cba7_cf6c3f38","line":318,"range":{"start_line":316,"start_character":12,"end_line":318,"end_character":73},"in_reply_to":"ffb9cba7_2c3f3726","updated":"2019-04-24 20:15:44.000000000","message":"I\u0027m not sure about this solution. I know that it works fine but it looks a bit odd for me. You have two different methods which accepts exactly same arguments and are doing exactly the same thing.\nIt took me a while to understand it now, when I was reviewing this patch and have context of it. So I assume that when someone will read it in e.g. 3 months it may be very confusing for him.\n\nLets see what others think about it.","commit_id":"6068094fd51ddfe6cceda63fb98b7fde1c2f7c0a"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"f2b74d7976b7177a823094b0778ed703e083065c","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        if not agentschedulers_db.services_available(agent.admin_state_up):"},{"line_number":314,"context_line":"            return []"},{"line_number":315,"context_line":"        if not with_dvr:"},{"line_number":316,"context_line":"            return self._get_router_ids_for_agent_no_dvr("},{"line_number":317,"context_line":"                context, agent, router_ids)"},{"line_number":318,"context_line":"        return self._get_router_ids_for_agent(context, agent, router_ids)"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"    def _get_router_ids_for_agent_no_dvr(self, context, agent, router_ids):"},{"line_number":321,"context_line":"        \"\"\"Get IDs of routers that the agent should host"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffb9cba7_ef9a95c3","line":318,"range":{"start_line":316,"start_character":12,"end_line":318,"end_character":73},"in_reply_to":"ffb9cba7_621ec50d","updated":"2019-04-23 01:16:16.000000000","message":"Here are something I wrote in the bug description:\n\"\"\"\nThe issue is because function _get_router_ids_for_agent [2] is overrided in [3] for dvr. Such dvr local router will be counted to the HA router ids list.\n\n[2] https://github.com/openstack/neutron/blob/master/neutron/db/l3_agentschedulers_db.py#L323\n[3] https://github.com/openstack/neutron/blob/master/neutron/db/l3_dvrscheduler_db.py#L401\n\"\"\"","commit_id":"6068094fd51ddfe6cceda63fb98b7fde1c2f7c0a"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9f1aa0dfb4ccd4a3684e88e3d8c833ffaa092625","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        if not agentschedulers_db.services_available(agent.admin_state_up):"},{"line_number":314,"context_line":"            return []"},{"line_number":315,"context_line":"        if not with_dvr:"},{"line_number":316,"context_line":"            return self._get_router_ids_for_agent_no_dvr("},{"line_number":317,"context_line":"                context, agent, router_ids)"},{"line_number":318,"context_line":"        return self._get_router_ids_for_agent(context, agent, router_ids)"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"    def _get_router_ids_for_agent_no_dvr(self, context, agent, router_ids):"},{"line_number":321,"context_line":"        \"\"\"Get IDs of routers that the agent should host"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffb9cba7_9ddad748","line":318,"range":{"start_line":316,"start_character":12,"end_line":318,"end_character":73},"in_reply_to":"ffb9cba7_cf6c3f38","updated":"2019-04-26 22:44:46.000000000","message":"I also find this change confusing and think Slawek\u0027s solution might be better.","commit_id":"6068094fd51ddfe6cceda63fb98b7fde1c2f7c0a"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"2dd3364b20b7ecfda9044ab7c1f147bda9ad3d17","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        if not agentschedulers_db.services_available(agent.admin_state_up):"},{"line_number":314,"context_line":"            return []"},{"line_number":315,"context_line":"        if not with_dvr:"},{"line_number":316,"context_line":"            return self._get_router_ids_for_agent_no_dvr("},{"line_number":317,"context_line":"                context, agent, router_ids)"},{"line_number":318,"context_line":"        return self._get_router_ids_for_agent(context, agent, router_ids)"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"    def _get_router_ids_for_agent_no_dvr(self, context, agent, router_ids):"},{"line_number":321,"context_line":"        \"\"\"Get IDs of routers that the agent should host"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffb9cba7_377e5faf","line":318,"range":{"start_line":316,"start_character":12,"end_line":318,"end_character":73},"in_reply_to":"ffb9cba7_cf6c3f38","updated":"2019-04-28 03:45:58.000000000","message":"Like I said, the code path jump out this file and then jump back, this can be the reason for developers to feel confusing... And pram may not be used in function.\nIMO, the current change had the minimum influence for neutron and sub projects.\nBut OK, let\u0027s try the new style...","commit_id":"6068094fd51ddfe6cceda63fb98b7fde1c2f7c0a"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"5ed5d5a2f43e5de2d8f7a7c537a824d74158864e","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        if not agentschedulers_db.services_available(agent.admin_state_up):"},{"line_number":314,"context_line":"            return []"},{"line_number":315,"context_line":"        if not with_dvr:"},{"line_number":316,"context_line":"            return self._get_router_ids_for_agent_no_dvr("},{"line_number":317,"context_line":"                context, agent, router_ids)"},{"line_number":318,"context_line":"        return self._get_router_ids_for_agent(context, agent, router_ids)"},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"    def _get_router_ids_for_agent_no_dvr(self, context, agent, router_ids):"},{"line_number":321,"context_line":"        \"\"\"Get IDs of routers that the agent should host"}],"source_content_type":"text/x-python","patch_set":2,"id":"ffb9cba7_012a3c8f","line":318,"range":{"start_line":316,"start_character":12,"end_line":318,"end_character":73},"in_reply_to":"ffb9cba7_ef9a95c3","updated":"2019-04-24 09:51:50.000000000","message":"can\u0027t You maybe add \"with_dvr\" parameter to _get_router_ids_for_agent() method and in https://github.com/openstack/neutron/blob/master/neutron/db/l3_dvrscheduler_db.py#L401 do something like:\n\n    if not with_dvr:\n        return super(L3_DVRsch_db_mixin, self)._get_router_ids_for_agent_no_dvr(...)\n\nIMO it would be more elegant solution than what is proposed here.","commit_id":"6068094fd51ddfe6cceda63fb98b7fde1c2f7c0a"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"c635ecada9a9560ec32821191028bbe142333625","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        if not agentschedulers_db.services_available(agent.admin_state_up):"},{"line_number":314,"context_line":"            return []"},{"line_number":315,"context_line":"        return self._get_router_ids_for_agent(context, agent,"},{"line_number":316,"context_line":"                                              router_ids, with_dvr)"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    def get_host_ha_router_count(self, context, host):"},{"line_number":319,"context_line":"        router_ids \u003d self.list_router_ids_on_host(context, host,"}],"source_content_type":"text/x-python","patch_set":3,"id":"ffb9cba7_b119db7a","line":316,"range":{"start_line":316,"start_character":58,"end_line":316,"end_character":66},"updated":"2019-04-30 16:42:24.000000000","message":"nit: with_dvr\u003dwith_dvr","commit_id":"8f2fc1ee5ee0d5cf8f412fa510adb87f3ed5f4b2"}],"neutron/db/l3_dvrscheduler_db.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"c635ecada9a9560ec32821191028bbe142333625","unresolved":false,"context_lines":[{"line_number":402,"context_line":"                                  with_dvr\u003dTrue):"},{"line_number":403,"context_line":"        result_set \u003d set(super(L3_DVRsch_db_mixin,"},{"line_number":404,"context_line":"                               self)._get_router_ids_for_agent("},{"line_number":405,"context_line":"            context, agent_db, router_ids, with_dvr))"},{"line_number":406,"context_line":"        if not with_dvr:"},{"line_number":407,"context_line":"            return result_set"},{"line_number":408,"context_line":"        router_ids \u003d set(router_ids or [])"}],"source_content_type":"text/x-python","patch_set":3,"id":"ffb9cba7_d116cf49","line":405,"range":{"start_line":405,"start_character":43,"end_line":405,"end_character":51},"updated":"2019-04-30 16:42:24.000000000","message":"same here","commit_id":"8f2fc1ee5ee0d5cf8f412fa510adb87f3ed5f4b2"}]}