)]}'
{"neutron/agent/l3/dvr_fip_ns.py":[{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"aafffb131e94d81e72ae720eeec7efb2c401179f","unresolved":false,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def subscribe(self, external_net_id):"},{"line_number":83,"context_line":"        is_first \u003d not self.has_subscribers()"},{"line_number":84,"context_line":"        self._subscribers.add(external_net_id)"},{"line_number":85,"context_line":"        return is_first"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def unsubscribe(self, external_net_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_5bbafae2","line":84,"updated":"2016-01-21 02:22:28.000000000","message":"Since external_net_id is subscribed here. Is a set object needed here? I guess a single boolean will be enough.","commit_id":"cfb37f0e7792b233cab474e8d661fd54b1a070af"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"e84438e6f2c3c38a0bec97afe319890ed4cea5f8","unresolved":false,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def subscribe(self, external_net_id):"},{"line_number":83,"context_line":"        is_first \u003d not self.has_subscribers()"},{"line_number":84,"context_line":"        self._subscribers.add(external_net_id)"},{"line_number":85,"context_line":"        return is_first"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def unsubscribe(self, external_net_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_b6cd24b9","line":84,"in_reply_to":"9a68dd71_5bbafae2","updated":"2016-01-21 17:40:20.000000000","message":"This should handle it since subscribe is part of fip namespace which is derived for every external_network.","commit_id":"cfb37f0e7792b233cab474e8d661fd54b1a070af"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"a6c84b69d3795c9bfaf62df78fa7dcc585e972dc","unresolved":false,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def subscribe(self, external_net_id):"},{"line_number":83,"context_line":"        is_first \u003d not self.has_subscribers()"},{"line_number":84,"context_line":"        self._subscribers.add(external_net_id)"},{"line_number":85,"context_line":"        return is_first"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def unsubscribe(self, external_net_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_885b0a2e","line":84,"in_reply_to":"9a68dd71_b6cd24b9","updated":"2016-01-22 02:07:49.000000000","message":"A fip namespace should only associate to one external net, not to multiple external nets. For that, a set here seems unnecessary.","commit_id":"cfb37f0e7792b233cab474e8d661fd54b1a070af"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"04558d1927be8d86e01b0e0202c0c81704d26abd","unresolved":false,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def subscribe(self, external_net_id):"},{"line_number":83,"context_line":"        is_first \u003d not self.has_subscribers()"},{"line_number":84,"context_line":"        self._subscribers.add(external_net_id)"},{"line_number":85,"context_line":"        return is_first"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def unsubscribe(self, external_net_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_ad994021","line":84,"in_reply_to":"9a68dd71_b6cd24b9","updated":"2016-01-22 02:07:14.000000000","message":"The set object(self._subscribers) can do the job here. But since \u0027subscribe\u0027 here takes \u0027external_net_id\u0027, which can only be \u0027self._ext_net_id\u0027. So the maximum value of \u0027len(self._subscribers)\u0027 will be 1. That makes me think the set object here is a bit inappropriate.","commit_id":"cfb37f0e7792b233cab474e8d661fd54b1a070af"}],"neutron/agent/l3/dvr_local_router.py":[{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"aafffb131e94d81e72ae720eeec7efb2c401179f","unresolved":false,"context_lines":[{"line_number":428,"context_line":"                \"plugin: %s\", fip_agent_port)"},{"line_number":429,"context_line":"        is_first \u003d False"},{"line_number":430,"context_line":"        if floating_ips:"},{"line_number":431,"context_line":"            is_first \u003d self.fip_ns.subscribe(self.router_id)"},{"line_number":432,"context_line":"            if is_first and not fip_agent_port:"},{"line_number":433,"context_line":"                LOG.debug(\"No FloatingIP agent gateway port possibly due to \""},{"line_number":434,"context_line":"                          \"late binding of the private port to the host, \""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_bbe4563b","line":431,"updated":"2016-01-21 02:22:28.000000000","message":"Do you miss to modify here?","commit_id":"cfb37f0e7792b233cab474e8d661fd54b1a070af"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"e84438e6f2c3c38a0bec97afe319890ed4cea5f8","unresolved":false,"context_lines":[{"line_number":428,"context_line":"                \"plugin: %s\", fip_agent_port)"},{"line_number":429,"context_line":"        is_first \u003d False"},{"line_number":430,"context_line":"        if floating_ips:"},{"line_number":431,"context_line":"            is_first \u003d self.fip_ns.subscribe(self.router_id)"},{"line_number":432,"context_line":"            if is_first and not fip_agent_port:"},{"line_number":433,"context_line":"                LOG.debug(\"No FloatingIP agent gateway port possibly due to \""},{"line_number":434,"context_line":"                          \"late binding of the private port to the host, \""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_76ab2cd7","line":431,"in_reply_to":"9a68dd71_bbe4563b","updated":"2016-01-21 17:40:20.000000000","message":"Yes I might have missed here.","commit_id":"cfb37f0e7792b233cab474e8d661fd54b1a070af"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"e005afc3259e2cb380f427c8b11b84cca94c932b","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            ns_ip.del_veth(fip_2_rtr_name)"},{"line_number":136,"context_line":"            # NOTE (Swami): The fg interface and the namespace will be deleted"},{"line_number":137,"context_line":"            # when the external gateway port is removed. This will be"},{"line_number":138,"context_line":"            # initiated from the server through an RPC call."},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def add_floating_ip(self, fip, interface_name, device):"},{"line_number":141,"context_line":"        if not self._add_fip_addr_to_device(fip, device):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a68dd71_ad670024","line":138,"updated":"2016-01-22 02:26:54.000000000","message":"nit: since fip_ns.delete() is called by fipnamespace_delete_on_ext_net, https://review.openstack.org/#/c/270532/3/neutron/agent/l3/dvr.py, do we still need the note comments here, or do we need put it at a more fit place?","commit_id":"21f715efcad0e353c7e02ad76c49a4e374f57d7e"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"e005afc3259e2cb380f427c8b11b84cca94c932b","unresolved":false,"context_lines":[{"line_number":428,"context_line":"                \"plugin: %s\", fip_agent_port)"},{"line_number":429,"context_line":"        is_first \u003d False"},{"line_number":430,"context_line":"        if floating_ips:"},{"line_number":431,"context_line":"            is_first \u003d self.fip_ns.subscribe(ex_gw_port[\u0027network_id\u0027])"},{"line_number":432,"context_line":"            if is_first and not fip_agent_port:"},{"line_number":433,"context_line":"                LOG.debug(\"No FloatingIP agent gateway port possibly due to \""},{"line_number":434,"context_line":"                          \"late binding of the private port to the host, \""}],"source_content_type":"text/x-python","patch_set":3,"id":"9a68dd71_689a5665","line":431,"updated":"2016-01-22 02:26:54.000000000","message":"Since you changed the definition of fip_ns.subscribe, \"is_first\" should work for external net, no longer for router. However \"is_first\" in following lines work for router.\nPlease update the logic here.\n\nAt least as I can see, L443 should be fixed. It may not work for a second router associated to fip_ns to create gateway port fg.","commit_id":"21f715efcad0e353c7e02ad76c49a4e374f57d7e"}],"neutron/tests/unit/agent/l3/test_agent.py":[{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"b0cf7670c03cff53dcc005035ebf9fa66558f858","unresolved":false,"context_lines":[{"line_number":934,"context_line":"            ri.create_dvr_fip_interfaces(ext_gw_port)"},{"line_number":935,"context_line":"            self.assertTrue(fip_gw_port.called)"},{"line_number":936,"context_line":"            self.assertTrue(fips.called)"},{"line_number":937,"context_line":"            self.assertTrue(c_gwp.called)"},{"line_number":938,"context_line":"            ri.fip_ns.subscribe \u003d mock.Mock(return_value\u003dFalse)"},{"line_number":939,"context_line":"            ri.create_dvr_fip_interfaces(ext_gw_port)"},{"line_number":940,"context_line":"            c_gwp.assert_called_once_with(mock.ANY)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a68dd71_c8782292","line":937,"updated":"2016-01-22 02:07:51.000000000","message":"I think using \"c_gwp.assert_called_once_with(agent_gateway_port[0])\" here will be better.","commit_id":"21f715efcad0e353c7e02ad76c49a4e374f57d7e"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"b0cf7670c03cff53dcc005035ebf9fa66558f858","unresolved":false,"context_lines":[{"line_number":937,"context_line":"            self.assertTrue(c_gwp.called)"},{"line_number":938,"context_line":"            ri.fip_ns.subscribe \u003d mock.Mock(return_value\u003dFalse)"},{"line_number":939,"context_line":"            ri.create_dvr_fip_interfaces(ext_gw_port)"},{"line_number":940,"context_line":"            c_gwp.assert_called_once_with(mock.ANY)"},{"line_number":941,"context_line":""},{"line_number":942,"context_line":"    @mock.patch.object(lla.LinkLocalAllocator, \u0027_write\u0027)"},{"line_number":943,"context_line":"    def test_create_dvr_fip_interfaces_for_late_binding(self, lla_write):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a68dd71_2843fe43","line":940,"updated":"2016-01-22 02:07:51.000000000","message":"I think using \"c_gwp.assert_called_once_with(agent_gateway_port[0])\" here will be better.","commit_id":"21f715efcad0e353c7e02ad76c49a4e374f57d7e"}],"neutron/tests/unit/agent/l3/test_dvr_fip_ns.py":[{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"aafffb131e94d81e72ae720eeec7efb2c401179f","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def test_subscribe_not_first(self):"},{"line_number":46,"context_line":"        self.fip_ns.subscribe(mock.sentinel.external_net_id)"},{"line_number":47,"context_line":"        is_first \u003d self.fip_ns.subscribe(mock.sentinel.external_net_id2)"},{"line_number":48,"context_line":"        self.assertFalse(is_first)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def test_unsubscribe(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_7b31fea1","line":47,"updated":"2016-01-21 02:22:28.000000000","message":"Is it possible to subscribe 2 external net to one fip_ns?","commit_id":"cfb37f0e7792b233cab474e8d661fd54b1a070af"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"e84438e6f2c3c38a0bec97afe319890ed4cea5f8","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def test_subscribe_not_first(self):"},{"line_number":46,"context_line":"        self.fip_ns.subscribe(mock.sentinel.external_net_id)"},{"line_number":47,"context_line":"        is_first \u003d self.fip_ns.subscribe(mock.sentinel.external_net_id2)"},{"line_number":48,"context_line":"        self.assertFalse(is_first)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def test_unsubscribe(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_d69bf8c7","line":47,"in_reply_to":"9a68dd71_7b31fea1","updated":"2016-01-21 17:40:20.000000000","message":"No each of the subscribe is associated with a fip namespace object and each fip namespace is associated with external_network_id.","commit_id":"cfb37f0e7792b233cab474e8d661fd54b1a070af"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"04558d1927be8d86e01b0e0202c0c81704d26abd","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def test_subscribe_not_first(self):"},{"line_number":46,"context_line":"        self.fip_ns.subscribe(mock.sentinel.external_net_id)"},{"line_number":47,"context_line":"        is_first \u003d self.fip_ns.subscribe(mock.sentinel.external_net_id2)"},{"line_number":48,"context_line":"        self.assertFalse(is_first)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def test_unsubscribe(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_ad8ce035","line":47,"in_reply_to":"9a68dd71_d69bf8c7","updated":"2016-01-22 02:07:14.000000000","message":"fip_ns is identified by external_network_id. And from the logic, every fip_ns can only subscribe to its own external_network_id. Is it necessary to test subscribe 2 different external_network_id?\nI looks to me that the UT is testing something that will never happen. Correct me if I am wrong.","commit_id":"cfb37f0e7792b233cab474e8d661fd54b1a070af"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"aafffb131e94d81e72ae720eeec7efb2c401179f","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        self.assertTrue(is_last)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def test_unsubscribe_not_last(self):"},{"line_number":56,"context_line":"        self.fip_ns.subscribe(mock.sentinel.external_net_id)"},{"line_number":57,"context_line":"        self.fip_ns.subscribe(mock.sentinel.external_net_id2)"},{"line_number":58,"context_line":"        is_last \u003d self.fip_ns.unsubscribe(mock.sentinel.external_net_id2)"},{"line_number":59,"context_line":"        self.assertFalse(is_last)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_3b2b8632","line":56,"updated":"2016-01-21 02:22:28.000000000","message":"Ditto","commit_id":"cfb37f0e7792b233cab474e8d661fd54b1a070af"}]}