)]}'
{"neutron/agent/l3/agent.py":[{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"740d833536c101ae3fc7131364a5b6676d47c7e3","unresolved":false,"context_lines":[{"line_number":651,"context_line":"        ri.snat_ports \u003d ri.router.get(l3_constants.SNAT_ROUTER_INTF_KEY, [])"},{"line_number":652,"context_line":"        ri.enable_snat \u003d ri.router.get(\u0027enable_snat\u0027)"},{"line_number":653,"context_line":""},{"line_number":654,"context_line":"    def _handle_router_snat_rules(self, ri, ex_gw_port,"},{"line_number":655,"context_line":"                                  interface_name, action):"},{"line_number":656,"context_line":"        # Remove all the rules"},{"line_number":657,"context_line":"        # This is safe because if use_namespaces is set as False"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_d465941e","line":654,"updated":"2014-12-22 14:47:22.000000000","message":"Is the plan to move more of methods like these to RouterInfo in another step? Seems like some of these operate exclusively on the router?","commit_id":"3f771ee3f4fce53c019ba4a1fa9fb3c6559e62ee"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8bfd5d112687604c6d047daabc01a4dd9821820d","unresolved":false,"context_lines":[{"line_number":651,"context_line":"        ri.snat_ports \u003d ri.router.get(l3_constants.SNAT_ROUTER_INTF_KEY, [])"},{"line_number":652,"context_line":"        ri.enable_snat \u003d ri.router.get(\u0027enable_snat\u0027)"},{"line_number":653,"context_line":""},{"line_number":654,"context_line":"    def _handle_router_snat_rules(self, ri, ex_gw_port,"},{"line_number":655,"context_line":"                                  interface_name, action):"},{"line_number":656,"context_line":"        # Remove all the rules"},{"line_number":657,"context_line":"        # This is safe because if use_namespaces is set as False"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_cdb40584","line":654,"in_reply_to":"3a961159_d465941e","updated":"2014-12-23 20:39:44.000000000","message":"Yes, eventually all specific knowledge of a router will move out of the agent and be encapsulated in the router classes.  This will take some time.  I need time to tease them apart, fix up test failures in the unit tests, and then run the full test suite.\n\nThis patch will focus on floating ip stuff.  Then, I\u0027ll pick some other area.","commit_id":"3f771ee3f4fce53c019ba4a1fa9fb3c6559e62ee"},{"author":{"_account_id":7183,"name":"Xu Han Peng","email":"xuhanp@linux.vnet.ibm.com","username":"xuhanp"},"change_message_id":"a180fd6c81afae3be0af65a1e2e9b9ae072b3239","unresolved":false,"context_lines":[{"line_number":747,"context_line":"            if ri.router[\u0027distributed\u0027]:"},{"line_number":748,"context_line":"                self.floating_ip_removed_dist(ri, ip_cidr)"},{"line_number":749,"context_line":""},{"line_number":750,"context_line":"    def process_router_floating_ip_addresses(self, ri, ex_gw_port):"},{"line_number":751,"context_line":"        \"\"\"Configure IP addresses on router\u0027s external gateway interface."},{"line_number":752,"context_line":""},{"line_number":753,"context_line":"        Ensures addresses for existing floating IPs and cleans up"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_a5f52984","line":750,"updated":"2014-12-23 00:16:08.000000000","message":"Similar question with Paul, will methods like process_router_floating_ip_addresses be moved to router_info as well? ex_gw_port can be get by ri._get_ex_gw_port().","commit_id":"3f771ee3f4fce53c019ba4a1fa9fb3c6559e62ee"},{"author":{"_account_id":7183,"name":"Xu Han Peng","email":"xuhanp@linux.vnet.ibm.com","username":"xuhanp"},"change_message_id":"5914e75cd9fda5859e54ed3d2d79a3937a542ca4","unresolved":false,"context_lines":[{"line_number":747,"context_line":"            if ri.router[\u0027distributed\u0027]:"},{"line_number":748,"context_line":"                self.floating_ip_removed_dist(ri, ip_cidr)"},{"line_number":749,"context_line":""},{"line_number":750,"context_line":"    def process_router_floating_ip_addresses(self, ri, ex_gw_port):"},{"line_number":751,"context_line":"        \"\"\"Configure IP addresses on router\u0027s external gateway interface."},{"line_number":752,"context_line":""},{"line_number":753,"context_line":"        Ensures addresses for existing floating IPs and cleans up"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_aa831501","line":750,"in_reply_to":"3a961159_0db28d93","updated":"2014-12-24 05:10:34.000000000","message":"Since this patch will focus on floating ip stuff, will this method be considered?","commit_id":"3f771ee3f4fce53c019ba4a1fa9fb3c6559e62ee"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8bfd5d112687604c6d047daabc01a4dd9821820d","unresolved":false,"context_lines":[{"line_number":747,"context_line":"            if ri.router[\u0027distributed\u0027]:"},{"line_number":748,"context_line":"                self.floating_ip_removed_dist(ri, ip_cidr)"},{"line_number":749,"context_line":""},{"line_number":750,"context_line":"    def process_router_floating_ip_addresses(self, ri, ex_gw_port):"},{"line_number":751,"context_line":"        \"\"\"Configure IP addresses on router\u0027s external gateway interface."},{"line_number":752,"context_line":""},{"line_number":753,"context_line":"        Ensures addresses for existing floating IPs and cleans up"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_0db28d93","line":750,"in_reply_to":"3a961159_a5f52984","updated":"2014-12-23 20:39:44.000000000","message":"ditto","commit_id":"3f771ee3f4fce53c019ba4a1fa9fb3c6559e62ee"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"91c94463c1bd7406c96af290dd4c63d6b117d3c7","unresolved":false,"context_lines":[{"line_number":391,"context_line":"                        \"one external network.\")"},{"line_number":392,"context_line":"                    raise Exception(msg)"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    def _create_router(self, router_id, router):"},{"line_number":395,"context_line":"        ns_name \u003d (self.get_ns_name(router_id)"},{"line_number":396,"context_line":"                   if self.conf.use_namespaces else None)"},{"line_number":397,"context_line":"        if router.get(\u0027distributed\u0027):"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_a96a9817","line":394,"updated":"2015-01-06 13:06:33.000000000","message":"Do you think this method should live in the router class as a factory method (e.g. standalone method that creates a DVR router or regular router, based on distributed flag)?","commit_id":"4b285cab954db270a9ab45af8abc190a8519c843"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"70153ed083dedb0c5b7daf26a5f01ee11d2774b7","unresolved":false,"context_lines":[{"line_number":391,"context_line":"                        \"one external network.\")"},{"line_number":392,"context_line":"                    raise Exception(msg)"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    def _create_router(self, router_id, router):"},{"line_number":395,"context_line":"        ns_name \u003d (self.get_ns_name(router_id)"},{"line_number":396,"context_line":"                   if self.conf.use_namespaces else None)"},{"line_number":397,"context_line":"        if router.get(\u0027distributed\u0027):"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_6631b6a1","line":394,"in_reply_to":"3a961159_a96a9817","updated":"2015-01-06 16:11:17.000000000","message":"I thought of doing that.  However, I didn\u0027t think that the base router class should or even can be aware of the DVR and HA routers (circular dependency).  I think at some point we could abstract this out to a separate manager or factory class in its own module.  For now, it is fine here.  This gets us almost all of the way there by confining the knowledge of the various classes to a single place.","commit_id":"4b285cab954db270a9ab45af8abc190a8519c843"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"9b828a59d894c1f2eb108c928718b68950e4de0f","unresolved":false,"context_lines":[{"line_number":407,"context_line":"                                      router\u003drouter,"},{"line_number":408,"context_line":"                                      use_ipv6\u003dself.use_ipv6,"},{"line_number":409,"context_line":"                                      ns_name\u003dns_name,"},{"line_number":410,"context_line":"                                      host\u003dself.host)"},{"line_number":411,"context_line":""},{"line_number":412,"context_line":"    def _router_added(self, router_id, router):"},{"line_number":413,"context_line":"        ri \u003d self._create_router(router_id, router)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3a961159_1764acb7","line":410,"updated":"2015-01-06 05:35:05.000000000","message":"Do we need to pass the host to a non-DVR router?","commit_id":"4b285cab954db270a9ab45af8abc190a8519c843"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"94e61d09d8e3f9dd74af66a4ac8547f7755f2408","unresolved":false,"context_lines":[{"line_number":889,"context_line":"    def get_floating_ips(self, ri):"},{"line_number":890,"context_line":"        \"\"\"Filter Floating IPs to be hosted on this agent.\"\"\""},{"line_number":891,"context_line":"        floating_ips \u003d ri.router.get(l3_constants.FLOATINGIP_KEY, [])"},{"line_number":892,"context_line":"        if ri.router[\u0027distributed\u0027]:"},{"line_number":893,"context_line":"            floating_ips \u003d [i for i in floating_ips if i[\u0027host\u0027] \u003d\u003d self.host]"},{"line_number":894,"context_line":"        return floating_ips"},{"line_number":895,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_d6fe8d37","side":"PARENT","line":892,"updated":"2015-01-06 22:45:59.000000000","message":"This can now be teased out in to a base class implementation and a dvr implementation.","commit_id":"61bad9c7d6c09a648096d55c4c8664a1137ccb01"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"c8c02b3df560f2b2e25851429dadf209d583d9f9","unresolved":false,"context_lines":[{"line_number":834,"context_line":"    def get_floating_ips(self, ri):"},{"line_number":835,"context_line":"        \"\"\"Filter Floating IPs to be hosted on this agent.\"\"\""},{"line_number":836,"context_line":"        floating_ips \u003d ri.router.get(l3_constants.FLOATINGIP_KEY, [])"},{"line_number":837,"context_line":"        if ri.router[\u0027distributed\u0027]:"},{"line_number":838,"context_line":"            floating_ips \u003d [i for i in floating_ips if i[\u0027host\u0027] \u003d\u003d self.host]"},{"line_number":839,"context_line":"        return floating_ips"},{"line_number":840,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"3a961159_29c83da7","side":"PARENT","line":837,"updated":"2015-01-07 17:53:08.000000000","message":"Note that creating get_floating_ips in both the base class and the dvr class removes the need for this check and the tests got simpler too.  Good ol\u0027 polymorphism.  More to come.","commit_id":"781081937ff6b142f5c30f252259e35c3db0ecb8"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"bd5a0b2f540b1d6e8abfce37113cdbe0dde316e6","unresolved":false,"context_lines":[{"line_number":470,"context_line":"    def _process_external_gateway(self, ri):"},{"line_number":471,"context_line":"        ex_gw_port \u003d ri.get_ex_gw_port()"},{"line_number":472,"context_line":"        ex_gw_port_id \u003d (ex_gw_port and ex_gw_port[\u0027id\u0027] or"},{"line_number":473,"context_line":"                         ri.ex_gw_port and ri.ex_gw_port[\u0027id\u0027])"},{"line_number":474,"context_line":""},{"line_number":475,"context_line":"        interface_name \u003d None"},{"line_number":476,"context_line":"        if ex_gw_port_id:"}],"source_content_type":"text/x-python","patch_set":25,"id":"1a930d6b_5602610a","line":473,"updated":"2015-01-27 17:38:34.000000000","message":"@Paul  note the use of both ex_gw_port and ri.ex_gw_port here.  There is a big difference and I agree it is not clear at all.  This is on my list to fix in the near future.","commit_id":"139685b6f01abc2ce90375e3c03b07af0a6a33bc"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"3899b3b6a471fe69b2c659e7e3beb034b4935ad5","unresolved":false,"context_lines":[{"line_number":470,"context_line":"    def _process_external_gateway(self, ri):"},{"line_number":471,"context_line":"        ex_gw_port \u003d ri.get_ex_gw_port()"},{"line_number":472,"context_line":"        ex_gw_port_id \u003d (ex_gw_port and ex_gw_port[\u0027id\u0027] or"},{"line_number":473,"context_line":"                         ri.ex_gw_port and ri.ex_gw_port[\u0027id\u0027])"},{"line_number":474,"context_line":""},{"line_number":475,"context_line":"        interface_name \u003d None"},{"line_number":476,"context_line":"        if ex_gw_port_id:"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa81d914_5f8ae8bf","line":473,"in_reply_to":"1a930d6b_5602610a","updated":"2015-01-29 16:35:06.000000000","message":"That would be great for people new to the code trying to understand.","commit_id":"139685b6f01abc2ce90375e3c03b07af0a6a33bc"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"bd5a0b2f540b1d6e8abfce37113cdbe0dde316e6","unresolved":false,"context_lines":[{"line_number":486,"context_line":"                port2_filtered \u003d _get_filtered_dict(port2, keys_to_ignore)"},{"line_number":487,"context_line":"                return port1_filtered \u003d\u003d port2_filtered"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"            self._set_subnet_info(ex_gw_port)"},{"line_number":490,"context_line":"            if not ri.ex_gw_port:"},{"line_number":491,"context_line":"                self.external_gateway_added(ri, ex_gw_port, interface_name)"},{"line_number":492,"context_line":"            elif not _gateway_ports_equal(ex_gw_port, ri.ex_gw_port):"}],"source_content_type":"text/x-python","patch_set":25,"id":"1a930d6b_5650a10a","line":489,"updated":"2015-01-27 17:38:34.000000000","message":"@Paul:  This section is quite confusing, isn\u0027t it?","commit_id":"139685b6f01abc2ce90375e3c03b07af0a6a33bc"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"3899b3b6a471fe69b2c659e7e3beb034b4935ad5","unresolved":false,"context_lines":[{"line_number":486,"context_line":"                port2_filtered \u003d _get_filtered_dict(port2, keys_to_ignore)"},{"line_number":487,"context_line":"                return port1_filtered \u003d\u003d port2_filtered"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"            self._set_subnet_info(ex_gw_port)"},{"line_number":490,"context_line":"            if not ri.ex_gw_port:"},{"line_number":491,"context_line":"                self.external_gateway_added(ri, ex_gw_port, interface_name)"},{"line_number":492,"context_line":"            elif not _gateway_ports_equal(ex_gw_port, ri.ex_gw_port):"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa81d914_da834654","line":489,"in_reply_to":"1a930d6b_5650a10a","updated":"2015-01-29 16:35:06.000000000","message":"Yeah. Something for future consideration maybe.","commit_id":"139685b6f01abc2ce90375e3c03b07af0a6a33bc"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"bd5a0b2f540b1d6e8abfce37113cdbe0dde316e6","unresolved":false,"context_lines":[{"line_number":490,"context_line":"            if not ri.ex_gw_port:"},{"line_number":491,"context_line":"                self.external_gateway_added(ri, ex_gw_port, interface_name)"},{"line_number":492,"context_line":"            elif not _gateway_ports_equal(ex_gw_port, ri.ex_gw_port):"},{"line_number":493,"context_line":"                self.external_gateway_updated(ri, ex_gw_port, interface_name)"},{"line_number":494,"context_line":"        elif not ex_gw_port and ri.ex_gw_port:"},{"line_number":495,"context_line":"            self.external_gateway_removed(ri, ri.ex_gw_port, interface_name)"},{"line_number":496,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"1a930d6b_161c5969","line":493,"updated":"2015-01-27 17:38:34.000000000","message":"@Paul ^","commit_id":"139685b6f01abc2ce90375e3c03b07af0a6a33bc"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"113cfce2d0c952026eff6bd63f4ab7ba5a40c83d","unresolved":false,"context_lines":[{"line_number":582,"context_line":"        # TODO(mrsmith) - we shouldn\u0027t need to check here"},{"line_number":583,"context_line":"        if \u0027distributed\u0027 not in ri.router:"},{"line_number":584,"context_line":"            ri.router[\u0027distributed\u0027] \u003d False"},{"line_number":585,"context_line":"        ex_gw_port \u003d ri.get_ex_gw_port()"},{"line_number":586,"context_line":"        if ri.router.get(\u0027distributed\u0027) and ex_gw_port:"},{"line_number":587,"context_line":"            fip_ns \u003d self.get_fip_ns(ex_gw_port[\u0027network_id\u0027])"},{"line_number":588,"context_line":"            fip_ns.scan_fip_ports(ri)"}],"source_content_type":"text/x-python","patch_set":25,"id":"1a930d6b_20f10f95","line":585,"updated":"2015-01-27 17:11:35.000000000","message":"Just noticing the awkwardness of getting this port in several places (L558, L471, here) all in the flow of handling process_router().","commit_id":"139685b6f01abc2ce90375e3c03b07af0a6a33bc"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"3899b3b6a471fe69b2c659e7e3beb034b4935ad5","unresolved":false,"context_lines":[{"line_number":582,"context_line":"        # TODO(mrsmith) - we shouldn\u0027t need to check here"},{"line_number":583,"context_line":"        if \u0027distributed\u0027 not in ri.router:"},{"line_number":584,"context_line":"            ri.router[\u0027distributed\u0027] \u003d False"},{"line_number":585,"context_line":"        ex_gw_port \u003d ri.get_ex_gw_port()"},{"line_number":586,"context_line":"        if ri.router.get(\u0027distributed\u0027) and ex_gw_port:"},{"line_number":587,"context_line":"            fip_ns \u003d self.get_fip_ns(ex_gw_port[\u0027network_id\u0027])"},{"line_number":588,"context_line":"            fip_ns.scan_fip_ports(ri)"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa81d914_1a707ea4","line":585,"in_reply_to":"1a930d6b_037a5d23","updated":"2015-01-29 16:35:06.000000000","message":"Just seems odd that the get_ex_gw_port() was being obtained multiple times in the same call flow.  I didn\u0027t look at what the logic was below it, and it sounds like it is trivial, so no worries.","commit_id":"139685b6f01abc2ce90375e3c03b07af0a6a33bc"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"bd5a0b2f540b1d6e8abfce37113cdbe0dde316e6","unresolved":false,"context_lines":[{"line_number":582,"context_line":"        # TODO(mrsmith) - we shouldn\u0027t need to check here"},{"line_number":583,"context_line":"        if \u0027distributed\u0027 not in ri.router:"},{"line_number":584,"context_line":"            ri.router[\u0027distributed\u0027] \u003d False"},{"line_number":585,"context_line":"        ex_gw_port \u003d ri.get_ex_gw_port()"},{"line_number":586,"context_line":"        if ri.router.get(\u0027distributed\u0027) and ex_gw_port:"},{"line_number":587,"context_line":"            fip_ns \u003d self.get_fip_ns(ex_gw_port[\u0027network_id\u0027])"},{"line_number":588,"context_line":"            fip_ns.scan_fip_ports(ri)"}],"source_content_type":"text/x-python","patch_set":25,"id":"1a930d6b_037a5d23","line":585,"in_reply_to":"1a930d6b_20f10f95","updated":"2015-01-27 17:38:34.000000000","message":"How is it awkward?  What alternative would you suggest?  Pass it around as an argument?  I would call that much more awkward.\n\nAre you concerned about performance?  Don\u0027t be.  It is basically \"self.router.get(\u0027gw_port\u0027)\".\n\nAt any rate, this is really out of the scope of this patch.","commit_id":"139685b6f01abc2ce90375e3c03b07af0a6a33bc"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"bd5a0b2f540b1d6e8abfce37113cdbe0dde316e6","unresolved":false,"context_lines":[{"line_number":594,"context_line":"        # Enable or disable keepalived for ha routers"},{"line_number":595,"context_line":"        self._process_ha_router(ri)"},{"line_number":596,"context_line":""},{"line_number":597,"context_line":"        # Update ex_gw_port and enable_snat on the router info cache"},{"line_number":598,"context_line":"        ri.ex_gw_port \u003d ex_gw_port"},{"line_number":599,"context_line":"        ri.snat_ports \u003d ri.router.get(l3_constants.SNAT_ROUTER_INTF_KEY, [])"},{"line_number":600,"context_line":"        ri.enable_snat \u003d ri.router.get(\u0027enable_snat\u0027)"}],"source_content_type":"text/x-python","patch_set":25,"id":"1a930d6b_83f48d5b","line":597,"updated":"2015-01-27 17:38:34.000000000","message":"@Paul:  Notice that ri.ex_gw_port is only set here after the router has been processed.","commit_id":"139685b6f01abc2ce90375e3c03b07af0a6a33bc"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"b84ab8b73469d1480515336477ed47b6f220153d","unresolved":false,"context_lines":[{"line_number":687,"context_line":"        else:"},{"line_number":688,"context_line":"            return self.get_external_device_name(ex_gw_port[\u0027id\u0027])"},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"    def _add_floating_ip(self, ri, fip, interface_name, device):"},{"line_number":691,"context_line":"        fip_ip \u003d fip[\u0027floating_ip_address\u0027]"},{"line_number":692,"context_line":"        ip_cidr \u003d common_utils.ip_to_cidr(fip_ip)"},{"line_number":693,"context_line":""}],"source_content_type":"text/x-python","patch_set":33,"id":"fa81d914_d623f0ba","side":"PARENT","line":690,"updated":"2015-01-30 18:52:12.000000000","message":"This method was an example of a more complex method with \"if ri.is_ha\" this and \"if ri.router[\u0027distributed\u0027]\" that.  Notice how the refactoring removed all of that and each of the resulting methods is very straight-forward.","commit_id":"f4165dfa4fdd1f0b9795e105d2d2cda062ee0cf3"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f7d5b7b4fc13c0cf3917ee6de6c87505a84073c2","unresolved":false,"context_lines":[{"line_number":714,"context_line":"            if common_utils.is_cidr_host(ip_cidr))"},{"line_number":715,"context_line":"        for ip_cidr in fips_to_remove:"},{"line_number":716,"context_line":"            ri.remove_floating_ip(device, ip_cidr)"},{"line_number":717,"context_line":"            # TODO(Carl) remove when dvr_router\u0027s remove_floating_ip does it."},{"line_number":718,"context_line":"            if ri.router[\u0027distributed\u0027]:"},{"line_number":719,"context_line":"                self.floating_ip_removed_dist(ri, ip_cidr)"},{"line_number":720,"context_line":""}],"source_content_type":"text/x-python","patch_set":37,"id":"da86d52c_fda3ca9b","line":717,"updated":"2015-02-02 21:06:15.000000000","message":"FYI:  I\u0027m hoping I can get rid of this before this patch is up for merging.","commit_id":"b3e1618f482feaabdb3c28d19b605dfabec2bc0e"}],"neutron/agent/l3/ha_router.py":[{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"30405d732af36978daa61438d249647133f981a9","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        fip_ip \u003d fip[\u0027floating_ip_address\u0027]"},{"line_number":246,"context_line":"        ip_cidr \u003d common_utils.ip_to_cidr(fip_ip)"},{"line_number":247,"context_line":"        self._add_vip(ip_cidr, interface_name)"},{"line_number":248,"context_line":"        # TODO(Carl) Should this return status?"},{"line_number":249,"context_line":"        # return l3_constants.FLOATINGIP_STATUS_ACTIVE"}],"source_content_type":"text/x-python","patch_set":31,"id":"fa81d914_057713a1","line":248,"updated":"2015-01-30 16:19:59.000000000","message":"Seems to make sense for consistency and possible testing.","commit_id":"31121962a979a7778fd5b8fda91088798f8d9c27"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a7ca1ad6aebb9bc223ab7c2302da119d042df3ca","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        fip_ip \u003d fip[\u0027floating_ip_address\u0027]"},{"line_number":246,"context_line":"        ip_cidr \u003d common_utils.ip_to_cidr(fip_ip)"},{"line_number":247,"context_line":"        self._add_vip(ip_cidr, interface_name)"},{"line_number":248,"context_line":"        # TODO(Carl) Should this return status?"},{"line_number":249,"context_line":"        # return l3_constants.FLOATINGIP_STATUS_ACTIVE"}],"source_content_type":"text/x-python","patch_set":31,"id":"fa81d914_d93883d5","line":248,"in_reply_to":"fa81d914_057713a1","updated":"2015-01-30 18:54:11.000000000","message":"I will not include this in this patch to preserve the existing behavior.  I did want to call it out for the HA guys to look at.","commit_id":"31121962a979a7778fd5b8fda91088798f8d9c27"}],"neutron/agent/l3/router_info.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"94e61d09d8e3f9dd74af66a4ac8547f7755f2408","unresolved":false,"context_lines":[{"line_number":35,"context_line":"            use_ipv6\u003duse_ipv6,"},{"line_number":36,"context_line":"            namespace\u003dself.ns_name)"},{"line_number":37,"context_line":"        self.routes \u003d []"},{"line_number":38,"context_line":"        # TODO(Carl) I think only DVR is interested in host.  So, non-dvr"},{"line_number":39,"context_line":"        # routers should not be initialized with a host."},{"line_number":40,"context_line":"        self.host \u003d host"},{"line_number":41,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"3a961159_d6accd37","line":38,"updated":"2015-01-06 22:45:59.000000000","message":"I think we\u0027re ready to encapsulate this as a dvr only detail.  This should show how functionality can be teased out in to dvr and legacy specific stuff and better encapsulated.","commit_id":"7aaca83c1701c9374c15ee554a57ceeaded926ef"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"24b10d36fd38aa7615e9fc2a901eb77882551f35","unresolved":false,"context_lines":[{"line_number":19,"context_line":"class RouterInfo(object):"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    def __init__(self, router_id, root_helper, router,"},{"line_number":22,"context_line":"                 use_ipv6\u003dFalse, ns_name\u003dNone, host\u003dNone):"},{"line_number":23,"context_line":"        self.router_id \u003d router_id"},{"line_number":24,"context_line":"        self.ex_gw_port \u003d None"},{"line_number":25,"context_line":"        self._snat_enabled \u003d None"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a961159_f62aa98f","line":22,"updated":"2015-01-06 22:49:50.000000000","message":"Oops, this should go away.  I thought I got rid of it.","commit_id":"df6419d210a9901771f00b992efb0cfc17331a1a"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"113cfce2d0c952026eff6bd63f4ab7ba5a40c83d","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                          *args, action\u003dself._snat_action)"},{"line_number":68,"context_line":"        self._snat_action \u003d None"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def get_ex_gw_port(self):"},{"line_number":71,"context_line":"        return self.router.get(\u0027gw_port\u0027)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def get_floating_ips(self):"}],"source_content_type":"text/x-python","patch_set":25,"id":"1a930d6b_e07e2718","line":70,"updated":"2015-01-27 17:11:35.000000000","message":"Forgive my ignorance on this..\n\nHow is self.router.get(\u0027gw_port\u0027), which is used via get_ex_gw_port(), different than self.ex_gw_port?","commit_id":"139685b6f01abc2ce90375e3c03b07af0a6a33bc"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"3899b3b6a471fe69b2c659e7e3beb034b4935ad5","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                          *args, action\u003dself._snat_action)"},{"line_number":68,"context_line":"        self._snat_action \u003d None"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def get_ex_gw_port(self):"},{"line_number":71,"context_line":"        return self.router.get(\u0027gw_port\u0027)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def get_floating_ips(self):"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa81d914_7a47ba1d","line":70,"in_reply_to":"1a930d6b_83664d4f","updated":"2015-01-29 16:35:06.000000000","message":"Thanks for the info. It\u0027s making more sense now.","commit_id":"139685b6f01abc2ce90375e3c03b07af0a6a33bc"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"bd5a0b2f540b1d6e8abfce37113cdbe0dde316e6","unresolved":false,"context_lines":[{"line_number":67,"context_line":"                          *args, action\u003dself._snat_action)"},{"line_number":68,"context_line":"        self._snat_action \u003d None"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def get_ex_gw_port(self):"},{"line_number":71,"context_line":"        return self.router.get(\u0027gw_port\u0027)"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    def get_floating_ips(self):"}],"source_content_type":"text/x-python","patch_set":25,"id":"1a930d6b_83664d4f","line":70,"in_reply_to":"1a930d6b_e07e2718","updated":"2015-01-27 17:38:34.000000000","message":"It isn\u0027t obvious, is it?  :)\n\nThis has been kind of a pain for a very long time.  There is a very important difference.  Look at my comment on neutron/agent/l3/agent.py L597.  ri.ex_gw_port is a record of the old gw.  So, when I router is processed with new information from the server, it is found in self.router.get(\u0027gw_port\u0027).  The old one is found in ri.ex_gw_port.\n\nI agree this is strange and must be fixed.  However, it is not this patch\u0027s job to do that.  It would be out of scope.\n\nMy thinking is that it will be easier to see and fix issues like this after the refactoring than before because the code will be much clearer and detangled.","commit_id":"139685b6f01abc2ce90375e3c03b07af0a6a33bc"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e42e172c28edb3552f1da68837c2506caf3df6ea","unresolved":false,"context_lines":[{"line_number":99,"context_line":"                (\u0027float-snat\u0027, \u0027-s %s -j SNAT --to %s\u0027 %"},{"line_number":100,"context_line":"                 (fixed_ip, floating_ip))]"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def process_floating_ip_nat_rules(self):"},{"line_number":103,"context_line":"        \"\"\"Configure NAT rules for the router\u0027s floating IPs."},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        Configures iptables rules for the floating ips of the given router"}],"source_content_type":"text/x-python","patch_set":42,"id":"da86d52c_5a72c6ad","line":102,"updated":"2015-02-04 14:53:00.000000000","message":"nit: consider a more descriptive name in future refactorings, as process_* is not very intention revealing (L102, L122, L157).","commit_id":"29b0701b82cabb9c17e409259dd0cadb954033c7"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"ba0a4f5f95c28ddf07c7719ddce5eae0b2f7cb69","unresolved":false,"context_lines":[{"line_number":99,"context_line":"                (\u0027float-snat\u0027, \u0027-s %s -j SNAT --to %s\u0027 %"},{"line_number":100,"context_line":"                 (fixed_ip, floating_ip))]"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    def process_floating_ip_nat_rules(self):"},{"line_number":103,"context_line":"        \"\"\"Configure NAT rules for the router\u0027s floating IPs."},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        Configures iptables rules for the floating ips of the given router"}],"source_content_type":"text/x-python","patch_set":42,"id":"da86d52c_b885ae5a","line":102,"in_reply_to":"da86d52c_5a72c6ad","updated":"2015-02-10 19:36:23.000000000","message":"I will.","commit_id":"29b0701b82cabb9c17e409259dd0cadb954033c7"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e42e172c28edb3552f1da68837c2506caf3df6ea","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                \u0027L3 agent failure to setup NAT for floating IPs\u0027)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def _add_fip_addr_to_device(self, fip, device):"},{"line_number":131,"context_line":"        \"\"\"Configures the floating ip address on the device."},{"line_number":132,"context_line":"        \"\"\""},{"line_number":133,"context_line":"        try:"},{"line_number":134,"context_line":"            ip_cidr \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"}],"source_content_type":"text/x-python","patch_set":42,"id":"da86d52c_bab74239","line":131,"updated":"2015-02-04 14:53:00.000000000","message":"supernit: Can put trailing quotes on same line.","commit_id":"29b0701b82cabb9c17e409259dd0cadb954033c7"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"ba0a4f5f95c28ddf07c7719ddce5eae0b2f7cb69","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                \u0027L3 agent failure to setup NAT for floating IPs\u0027)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def _add_fip_addr_to_device(self, fip, device):"},{"line_number":131,"context_line":"        \"\"\"Configures the floating ip address on the device."},{"line_number":132,"context_line":"        \"\"\""},{"line_number":133,"context_line":"        try:"},{"line_number":134,"context_line":"            ip_cidr \u003d common_utils.ip_to_cidr(fip[\u0027floating_ip_address\u0027])"}],"source_content_type":"text/x-python","patch_set":42,"id":"da86d52c_f8f646f8","line":131,"in_reply_to":"da86d52c_bab74239","updated":"2015-02-10 19:36:23.000000000","message":"Yes, I missed that when I moved the code.","commit_id":"29b0701b82cabb9c17e409259dd0cadb954033c7"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e42e172c28edb3552f1da68837c2506caf3df6ea","unresolved":false,"context_lines":[{"line_number":198,"context_line":"                \u0027floating IPs\u0027)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    def put_fips_in_error_state(self):"},{"line_number":201,"context_line":"        fip_statuses \u003d {}"},{"line_number":202,"context_line":"        for fip in self.router.get(l3_constants.FLOATINGIP_KEY, []):"},{"line_number":203,"context_line":"            fip_statuses[fip[\u0027id\u0027]] \u003d l3_constants.FLOATINGIP_STATUS_ERROR"},{"line_number":204,"context_line":"        return fip_statuses"}],"source_content_type":"text/x-python","patch_set":42,"id":"da86d52c_5aec868f","line":201,"updated":"2015-02-04 14:53:00.000000000","message":"Could just do dict comprehension\n    return dict([(f[\u0027id\u0027], l3_constants.FLOATINGIP_STATUS_ERROR)\n                for f in self.router.get(l3_constants.FLOATINGIP_KEY, [])])","commit_id":"29b0701b82cabb9c17e409259dd0cadb954033c7"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"ba0a4f5f95c28ddf07c7719ddce5eae0b2f7cb69","unresolved":false,"context_lines":[{"line_number":198,"context_line":"                \u0027floating IPs\u0027)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"    def put_fips_in_error_state(self):"},{"line_number":201,"context_line":"        fip_statuses \u003d {}"},{"line_number":202,"context_line":"        for fip in self.router.get(l3_constants.FLOATINGIP_KEY, []):"},{"line_number":203,"context_line":"            fip_statuses[fip[\u0027id\u0027]] \u003d l3_constants.FLOATINGIP_STATUS_ERROR"},{"line_number":204,"context_line":"        return fip_statuses"}],"source_content_type":"text/x-python","patch_set":42,"id":"da86d52c_b8ecce2a","line":201,"in_reply_to":"da86d52c_5aec868f","updated":"2015-02-10 19:36:23.000000000","message":"True","commit_id":"29b0701b82cabb9c17e409259dd0cadb954033c7"}],"neutron/common/utils.py":[{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"7a79a39137b9b9e86aa7c320f397f2bcfb2dd6a8","unresolved":false,"context_lines":[{"line_number":376,"context_line":"    \"\"\"Convert an ip with no prefix to cidr notation"},{"line_number":377,"context_line":""},{"line_number":378,"context_line":"    By default, ipv4 addresses will get /32 appended and ipv6 /128.  However,"},{"line_number":379,"context_line":"    if a different prefix is desired it can be specified."},{"line_number":380,"context_line":"    \"\"\""},{"line_number":381,"context_line":"    net \u003d netaddr.IPNetwork(ip)"},{"line_number":382,"context_line":"    if prefix is not None:"}],"source_content_type":"text/x-python","patch_set":22,"id":"1a930d6b_0e050bc2","line":379,"updated":"2015-01-20 20:18:56.000000000","message":"Since it\u0027s a common utility, don\u0027t know if you want to note that the ip can be a string, IPAddress, or IPNetwork.","commit_id":"a47297830a4ce61836b5b4b829efbd9e50237155"}],"neutron/tests/unit/test_dvr_router.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f4cc4a2e4029ee9bd6b9dc4a85ae0b60b2df032e","unresolved":false,"context_lines":[{"line_number":28,"context_line":"        return dvr_router.DvrRouter(host,"},{"line_number":29,"context_line":"                                    mock.sentinel.router_id,"},{"line_number":30,"context_line":"                                    mock.sentinel.root_helper,"},{"line_number":31,"context_line":"                                    router,"},{"line_number":32,"context_line":"                                    **kwargs)"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    def test_get_floating_ips_dvr(self):"}],"source_content_type":"text/x-python","patch_set":27,"id":"fa81d914_264ab6c5","line":31,"updated":"2015-01-28 22:26:17.000000000","message":"The order of these arguments here has changed.","commit_id":"8fab54f712d5283a41d70db3579789e922e5bcc2"}],"neutron/tests/unit/test_l3_agent.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"108f347e76d05a2903c011c0e9242b272f1be4b8","unresolved":false,"context_lines":[{"line_number":880,"context_line":""},{"line_number":881,"context_line":"        router \u003d prepare_router_data(enable_snat\u003dTrue)"},{"line_number":882,"context_line":"        router[l3_constants.FLOATINGIP_AGENT_INTF_KEY] \u003d agent_gateway_port"},{"line_number":883,"context_line":"        router[\u0027distributed\u0027] \u003d True"},{"line_number":884,"context_line":"        ri \u003d dvr_router.DvrRouter(router[\u0027id\u0027], router, **self.ri_kwargs)"},{"line_number":885,"context_line":"        agent \u003d l3_agent.L3NATAgent(HOSTNAME, self.conf)"},{"line_number":886,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":55,"id":"da86d52c_0a31b7a1","line":883,"updated":"2015-02-19 02:21:49.000000000","message":"missing HOSTNAME?","commit_id":"50ff2fd45bd0f83c14928126cee7d4ba2a2226b0"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"108f347e76d05a2903c011c0e9242b272f1be4b8","unresolved":false,"context_lines":[{"line_number":909,"context_line":"        router \u003d prepare_router_data(enable_snat\u003dTrue)"},{"line_number":910,"context_line":"        router[l3_constants.FLOATINGIP_KEY] \u003d fake_floatingips[\u0027floatingips\u0027]"},{"line_number":911,"context_line":"        router[l3_constants.FLOATINGIP_AGENT_INTF_KEY] \u003d agent_gateway_port"},{"line_number":912,"context_line":"        router[\u0027distributed\u0027] \u003d True"},{"line_number":913,"context_line":"        ri \u003d dvr_router.DvrRouter(router[\u0027id\u0027], router, **self.ri_kwargs)"},{"line_number":914,"context_line":""},{"line_number":915,"context_line":"        agent \u003d l3_agent.L3NATAgent(HOSTNAME, self.conf)"}],"source_content_type":"text/x-python","patch_set":55,"id":"da86d52c_aa00abab","line":912,"updated":"2015-02-19 02:21:49.000000000","message":"missing HOSTNAME?","commit_id":"50ff2fd45bd0f83c14928126cee7d4ba2a2226b0"}],"neutron/tests/unit/test_l3_router.py":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"ed5201e4f5b1c897677d1bcf982ec7deeeaa6baa","unresolved":false,"context_lines":[{"line_number":88,"context_line":"                         ipv4_nat.mock_calls[0])"},{"line_number":89,"context_line":"        self.assertEqual(mock.call.apply(), ri.iptables_manager.mock_calls[-1])"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        # Be sure that add_rule is called somewhere in the middle"},{"line_number":92,"context_line":"        self.assertEqual(0, ipv4_nat.add_rule.call_count)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_dd4353ad","line":91,"updated":"2015-01-04 00:34:25.000000000","message":"Nit: comment should be:\n\n# Be sure that add_rule is not called somewhere in the middle","commit_id":"3f771ee3f4fce53c019ba4a1fa9fb3c6559e62ee"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"171c60c5bb46b4e4e96105c3867f20902000d27e","unresolved":false,"context_lines":[{"line_number":88,"context_line":"                         ipv4_nat.mock_calls[0])"},{"line_number":89,"context_line":"        self.assertEqual(mock.call.apply(), ri.iptables_manager.mock_calls[-1])"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        # Be sure that add_rule is called somewhere in the middle"},{"line_number":92,"context_line":"        self.assertEqual(0, ipv4_nat.add_rule.call_count)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_f1c65a9b","line":91,"updated":"2015-01-05 22:08:01.000000000","message":"In this test case, we want to make sure that ipv4_nat.add_rule is not called, unlike the test case immediately above. The comment should be something like:\n\n#  Be sure that add_rule is not called","commit_id":"ff4746d2d7887038df9d25aeb451a496d4f00a5f"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"03e0444cee4a87f41403f466283120a1abd9e98b","unresolved":false,"context_lines":[{"line_number":88,"context_line":"                         ipv4_nat.mock_calls[0])"},{"line_number":89,"context_line":"        self.assertEqual(mock.call.apply(), ri.iptables_manager.mock_calls[-1])"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        # Be sure that add_rule is called somewhere in the middle"},{"line_number":92,"context_line":"        self.assertEqual(0, ipv4_nat.add_rule.call_count)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_f479defc","line":91,"in_reply_to":"3a961159_f1c65a9b","updated":"2015-01-06 05:09:40.000000000","message":"Good catch.  I copied the comment and failed to change it.","commit_id":"ff4746d2d7887038df9d25aeb451a496d4f00a5f"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"3d6bed8865ab7629024b77e90e43a6b43ce53c83","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    def test_get_floating_ips(self):"},{"line_number":26,"context_line":"        router \u003d mock.MagicMock()"},{"line_number":27,"context_line":"        router.get.return_value \u003d [mock.sentinel.floating_ip]"},{"line_number":28,"context_line":"        # TODO(Carl) Remove when DVR sub-class is created."},{"line_number":29,"context_line":"        router[\u0027distributed\u0027].__nonzero__ \u003d lambda self: False"},{"line_number":30,"context_line":"        ri \u003d self._create_router(router\u003drouter)"},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_be8468d3","line":28,"updated":"2015-01-07 13:49:50.000000000","message":"Still revelent, as have DVR class?","commit_id":"27920a89143bc14d579962d8a3f993464e53b752"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6e2dddb2593251a7a4275e157ed720cc4e0a0fd0","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    def test_get_floating_ips(self):"},{"line_number":26,"context_line":"        router \u003d mock.MagicMock()"},{"line_number":27,"context_line":"        router.get.return_value \u003d [mock.sentinel.floating_ip]"},{"line_number":28,"context_line":"        # TODO(Carl) Remove when DVR sub-class is created."},{"line_number":29,"context_line":"        router[\u0027distributed\u0027].__nonzero__ \u003d lambda self: False"},{"line_number":30,"context_line":"        ri \u003d self._create_router(router\u003drouter)"},{"line_number":31,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_08c1bfe3","line":28,"in_reply_to":"3a961159_be8468d3","updated":"2015-01-07 16:32:14.000000000","message":"Paul, thanks.  Now that this patch has progressed a bit, I do need to work on these tests a bit.  I think I need to create a test_dvr_router.py and split these get_floating_ip tests.  Somehow a test failure sneaked through, too.  I thought they all passed before I submitted.  :|","commit_id":"27920a89143bc14d579962d8a3f993464e53b752"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"32091cea7685eb5189710c728f2e729362c04a16","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        self.assertEqual(mock.call.apply(), ri.iptables_manager.mock_calls[-1])"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        # Be sure that add_rule is called somewhere in the middle"},{"line_number":91,"context_line":"        self.assertEqual(0, ipv4_nat.add_rule.call_count)"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_b8cc8b36","line":91,"updated":"2015-01-07 02:36:33.000000000","message":"self.assertFalse(ipv4_nat.add_rule.called)","commit_id":"27920a89143bc14d579962d8a3f993464e53b752"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6e2dddb2593251a7a4275e157ed720cc4e0a0fd0","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        self.assertEqual(mock.call.apply(), ri.iptables_manager.mock_calls[-1])"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        # Be sure that add_rule is called somewhere in the middle"},{"line_number":91,"context_line":"        self.assertEqual(0, ipv4_nat.add_rule.call_count)"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a961159_687883c4","line":91,"in_reply_to":"3a961159_b8cc8b36","updated":"2015-01-07 16:32:14.000000000","message":"That\u0027s all you could find?  ;)","commit_id":"27920a89143bc14d579962d8a3f993464e53b752"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"30405d732af36978daa61438d249647133f981a9","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from neutron.tests import base"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class TestBasicRouterOperations(base.BaseTestCase):"},{"line_number":22,"context_line":"    def setUp(self):"},{"line_number":23,"context_line":"        super(TestBasicRouterOperations, self).setUp()"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"fa81d914_e5ed9fb8","line":21,"updated":"2015-01-30 16:19:59.000000000","message":"nit? The router test modules have the same class name. Would it be useful to use different names, rather than relying on module name? E.g. TestBasicLegacyRouterOperations...","commit_id":"31121962a979a7778fd5b8fda91088798f8d9c27"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a7ca1ad6aebb9bc223ab7c2302da119d042df3ca","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from neutron.tests import base"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class TestBasicRouterOperations(base.BaseTestCase):"},{"line_number":22,"context_line":"    def setUp(self):"},{"line_number":23,"context_line":"        super(TestBasicRouterOperations, self).setUp()"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"fa81d914_796cf7d0","line":21,"in_reply_to":"fa81d914_e5ed9fb8","updated":"2015-01-30 18:54:11.000000000","message":"I personally think that the module name gives all of the context needed.  I don\u0027t mind calling them all the same thing.","commit_id":"31121962a979a7778fd5b8fda91088798f8d9c27"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cf5228069b4face24824cea46f69d092f636b8b6","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright (c) 2015 Openstack Foundation"},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":4,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":44,"id":"da86d52c_3ea8788c","line":1,"updated":"2015-02-05 14:23:02.000000000","message":"So are we removing copyrights for new files then? (Makes sense to me).","commit_id":"61f77beb4135ca78715bb434b441f8ddebc7aeea"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"daae116f04c4b250e841df419caaa221bb0ed463","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright (c) 2015 Openstack Foundation"},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":4,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":44,"id":"da86d52c_f85ac628","line":1,"in_reply_to":"da86d52c_3ea8788c","updated":"2015-02-10 18:33:29.000000000","message":"I\u0027m a little behind.  I shouldn\u0027t have said anything on your other patch.  It was just that I was getting comments from others on it.  I have decided not to nit-pick copyright headers anymore.  But, I\u0027ll take it out if I have to roll the patch again.","commit_id":"61f77beb4135ca78715bb434b441f8ddebc7aeea"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"cf5228069b4face24824cea46f69d092f636b8b6","unresolved":false,"context_lines":[{"line_number":24,"context_line":"_uuid \u003d uuidutils.generate_uuid"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"class TestBasicRouterOperations(base.BaseTestCase):"},{"line_number":28,"context_line":"    def setUp(self):"},{"line_number":29,"context_line":"        super(TestBasicRouterOperations, self).setUp()"},{"line_number":30,"context_line":""}],"source_content_type":"text/x-python","patch_set":44,"id":"da86d52c_be1e280a","line":27,"updated":"2015-02-05 14:23:02.000000000","message":"You can remove this setUp implementation.","commit_id":"61f77beb4135ca78715bb434b441f8ddebc7aeea"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"daae116f04c4b250e841df419caaa221bb0ed463","unresolved":false,"context_lines":[{"line_number":24,"context_line":"_uuid \u003d uuidutils.generate_uuid"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"class TestBasicRouterOperations(base.BaseTestCase):"},{"line_number":28,"context_line":"    def setUp(self):"},{"line_number":29,"context_line":"        super(TestBasicRouterOperations, self).setUp()"},{"line_number":30,"context_line":""}],"source_content_type":"text/x-python","patch_set":44,"id":"da86d52c_383c5ebf","line":27,"in_reply_to":"da86d52c_be1e280a","updated":"2015-02-10 18:33:29.000000000","message":"I will if I crack open the patch.","commit_id":"61f77beb4135ca78715bb434b441f8ddebc7aeea"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"27a049e2f00efa1455c0a85a885e23a549850f6d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":47,"id":"da86d52c_6dd8cd1e","updated":"2015-02-10 22:08:29.000000000","message":"I think these should be relocated following @Assaf\u0027s lead and go in the neutron/tests/unit/agent/l3/ directory.","commit_id":"6ce14bbd37ce32a6c6f83bdc6446e4614ad12fcf"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b2b24defa02a30fbd29cc6ef54509f24f14e2eca","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        ri.process_floating_ip_addresses.assert_called_once_with("},{"line_number":139,"context_line":"            mock.sentinel.interface_name)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    @mock.patch.object(ip_lib, \u0027IPDevice\u0027)"},{"line_number":142,"context_line":"    def test_process_floating_ip_addresses_remap(self, IPDevice):"},{"line_number":143,"context_line":"        fip_id \u003d _uuid()"},{"line_number":144,"context_line":"        fip \u003d {"}],"source_content_type":"text/x-python","patch_set":47,"id":"da86d52c_339e4155","line":141,"updated":"2015-02-10 19:10:52.000000000","message":"this could\u0027ve been done once","commit_id":"6ce14bbd37ce32a6c6f83bdc6446e4614ad12fcf"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"70efbb5dacc55b199e261277f00a5180c972a2c1","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        ri.process_floating_ip_addresses.assert_called_once_with("},{"line_number":139,"context_line":"            mock.sentinel.interface_name)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    @mock.patch.object(ip_lib, \u0027IPDevice\u0027)"},{"line_number":142,"context_line":"    def test_process_floating_ip_addresses_remap(self, IPDevice):"},{"line_number":143,"context_line":"        fip_id \u003d _uuid()"},{"line_number":144,"context_line":"        fip \u003d {"}],"source_content_type":"text/x-python","patch_set":47,"id":"da86d52c_69460651","line":141,"in_reply_to":"da86d52c_339e4155","updated":"2015-02-10 19:46:02.000000000","message":"Do you suggest doing it in setUp?  I\u0027m not sure I like that either.  Then we get something like what we have now in test_l3_agent.py where we have more than a dozen things that are mocked out in setUp and apply to every test case regardless of which tests need it.\n\nI prefer lighter weight tests which mock what needs to be mocked as close as possible to the test and do not require complex frameworks full of cruft that may or may not apply to a given test.\n\nHere it is plain to see what needs to be mocked out for this particular test.\n\nDo you have any other suggestions that I have not considered?","commit_id":"6ce14bbd37ce32a6c6f83bdc6446e4614ad12fcf"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"878aebf4571a87407c0634207890f8306cc71932","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        ri.process_floating_ip_addresses.assert_called_once_with("},{"line_number":139,"context_line":"            mock.sentinel.interface_name)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    @mock.patch.object(ip_lib, \u0027IPDevice\u0027)"},{"line_number":142,"context_line":"    def test_process_floating_ip_addresses_remap(self, IPDevice):"},{"line_number":143,"context_line":"        fip_id \u003d _uuid()"},{"line_number":144,"context_line":"        fip \u003d {"}],"source_content_type":"text/x-python","patch_set":47,"id":"da86d52c_f9ad679c","line":141,"in_reply_to":"da86d52c_69460651","updated":"2015-02-10 20:15:59.000000000","message":"understood...but perhaps that means that the testcase may need to be split?","commit_id":"6ce14bbd37ce32a6c6f83bdc6446e4614ad12fcf"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e7967d70c5cf25efc3390246af11b1d76cf50427","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        ri.process_floating_ip_addresses.assert_called_once_with("},{"line_number":139,"context_line":"            mock.sentinel.interface_name)"},{"line_number":140,"context_line":""},{"line_number":141,"context_line":"    @mock.patch.object(ip_lib, \u0027IPDevice\u0027)"},{"line_number":142,"context_line":"    def test_process_floating_ip_addresses_remap(self, IPDevice):"},{"line_number":143,"context_line":"        fip_id \u003d _uuid()"},{"line_number":144,"context_line":"        fip \u003d {"}],"source_content_type":"text/x-python","patch_set":47,"id":"da86d52c_cdd9b958","line":141,"in_reply_to":"da86d52c_f9ad679c","updated":"2015-02-10 22:02:20.000000000","message":"I\u0027ll look in to that.  A good idea.","commit_id":"6ce14bbd37ce32a6c6f83bdc6446e4614ad12fcf"}],"neutron/tests/unit/test_legacy_router.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b2b24defa02a30fbd29cc6ef54509f24f14e2eca","unresolved":false,"context_lines":[{"line_number":37,"context_line":"                                          ns_name\u003dmock.sentinel.namespace,"},{"line_number":38,"context_line":"                                          **kwargs)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    @mock.patch.object(ip_lib, \u0027send_gratuitous_arp\u0027)"},{"line_number":41,"context_line":"    def test_add_floating_ip(self, send_gratuitous_arp):"},{"line_number":42,"context_line":"        ri \u003d self._create_router()"},{"line_number":43,"context_line":"        ri._add_fip_addr_to_device \u003d mock.Mock(return_value\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":47,"id":"da86d52c_3377213d","line":40,"updated":"2015-02-10 19:10:52.000000000","message":"this could\u0027ve been done once...I am starting to see a bad pattern here..","commit_id":"6ce14bbd37ce32a6c6f83bdc6446e4614ad12fcf"}]}