)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"2de4f53b9bf27a8c2fae28a69c82d337d7875b98","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Brandon Logan \u003cbrandon.logan@rackspace.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-07-13 17:55:44 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Filter out subnets on different segments for dhcp"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The dhcp agent will request network information from neutron-server."},{"line_number":10,"context_line":"neutron-server will give the dhcp agent the entire network tree back"},{"line_number":11,"context_line":"to the agent including subnets.  If that agent is on a host that"},{"line_number":12,"context_line":"is mapped to a segment, subnets that are do not belong to that"},{"line_number":13,"context_line":"segment are also returned and the dhcp agent will attempt to create"},{"line_number":14,"context_line":"or update the dhcp port based on those subnets.  These requests"},{"line_number":15,"context_line":"will fail because the ipam logic forbids it for multiple segments."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"This patch strips out those subnets that do not belong to the host\u0027s"},{"line_number":18,"context_line":"segment.  If the segments plugin is not enabled or the network is not"},{"line_number":19,"context_line":"segmented then the neutron-server will return back to the agent what"},{"line_number":20,"context_line":"it normally would without segments."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I379b56f510853c60bb52c2d710117fecfb9aca2e"},{"line_number":23,"context_line":"Partially-Implements: blueprint routed-networks"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"1aa78d24_75802e8c","line":20,"range":{"start_line":7,"start_character":0,"end_line":20,"end_character":35},"updated":"2016-07-15 03:13:38.000000000","message":"Your entire commit message reflects the assumption that the filtering has to take place for multi-segment networks, when in fact we have to do it for routed networks: networks where the subnets are associated to segments","commit_id":"ae064d23ab58a770f7647f0a3e2c862cbc40f3f3"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"23b36d68997b9ea88d1ee7d4d97a7136e3f35619","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Brandon Logan \u003cbrandon.logan@rackspace.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-07-13 17:55:44 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Filter out subnets on different segments for dhcp"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The dhcp agent will request network information from neutron-server."},{"line_number":10,"context_line":"neutron-server will give the dhcp agent the entire network tree back"},{"line_number":11,"context_line":"to the agent including subnets.  If that agent is on a host that"},{"line_number":12,"context_line":"is mapped to a segment, subnets that are do not belong to that"},{"line_number":13,"context_line":"segment are also returned and the dhcp agent will attempt to create"},{"line_number":14,"context_line":"or update the dhcp port based on those subnets.  These requests"},{"line_number":15,"context_line":"will fail because the ipam logic forbids it for multiple segments."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"This patch strips out those subnets that do not belong to the host\u0027s"},{"line_number":18,"context_line":"segment.  If the segments plugin is not enabled or the network is not"},{"line_number":19,"context_line":"segmented then the neutron-server will return back to the agent what"},{"line_number":20,"context_line":"it normally would without segments."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I379b56f510853c60bb52c2d710117fecfb9aca2e"},{"line_number":23,"context_line":"Partially-Implements: blueprint routed-networks"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"1aa78d24_98deaf0e","line":20,"range":{"start_line":7,"start_character":0,"end_line":20,"end_character":35},"in_reply_to":"1aa78d24_75802e8c","updated":"2016-07-15 03:38:40.000000000","message":"I\u0027m not sure it was the entire message, just a word or two :)\n\nThe title still makes sense I believe because it is only for subnets with segments.","commit_id":"ae064d23ab58a770f7647f0a3e2c862cbc40f3f3"}],"neutron/api/rpc/handlers/dhcp_rpc.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"283f5986690e66ef18a31dfd2360fd0986aaabfb","unresolved":false,"context_lines":[{"line_number":135,"context_line":"        # to the host will be stripped out, otherwise just keep the requested"},{"line_number":136,"context_line":"        # fixed ips as is."},{"line_number":137,"context_line":"        if \u0027segments\u0027 not in manager.NeutronManager.get_service_plugins():"},{"line_number":138,"context_line":"            return False"},{"line_number":139,"context_line":"        fixed_ip_subnet_ids \u003d [fixed_ip[\u0027subnet_id\u0027] for fixed_ip in"},{"line_number":140,"context_line":"                               port[\u0027port\u0027].get(\u0027fixed_ips\u0027, [])]"},{"line_number":141,"context_line":"        subnets \u003d plugin.get_subnets("}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_807a5622","line":138,"updated":"2016-07-12 15:20:44.000000000","message":"nit: should probably import neutron.extensions.multiprovidernet as mp_net and use mp_net.SEGMENTS\n\nOr add it to the plugin/constants file?","commit_id":"fa95253dacb54163f112a1763ea677429d447c45"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"b0e6ada08cd03b554ab78b9e73fa5cba0e3375c5","unresolved":false,"context_lines":[{"line_number":135,"context_line":"        # to the host will be stripped out, otherwise just keep the requested"},{"line_number":136,"context_line":"        # fixed ips as is."},{"line_number":137,"context_line":"        if \u0027segments\u0027 not in manager.NeutronManager.get_service_plugins():"},{"line_number":138,"context_line":"            return False"},{"line_number":139,"context_line":"        fixed_ip_subnet_ids \u003d [fixed_ip[\u0027subnet_id\u0027] for fixed_ip in"},{"line_number":140,"context_line":"                               port[\u0027port\u0027].get(\u0027fixed_ips\u0027, [])]"},{"line_number":141,"context_line":"        subnets \u003d plugin.get_subnets("}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_1907a619","line":138,"in_reply_to":"1aa78d24_807a5622","updated":"2016-07-13 22:30:57.000000000","message":"Done","commit_id":"fa95253dacb54163f112a1763ea677429d447c45"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"48b1fc746f3fef7d80f00ba2f55f597a35c5dd6a","unresolved":false,"context_lines":[{"line_number":136,"context_line":"        # fixed ips as is."},{"line_number":137,"context_line":"        if \u0027segments\u0027 not in manager.NeutronManager.get_service_plugins():"},{"line_number":138,"context_line":"            return False"},{"line_number":139,"context_line":"        fixed_ip_subnet_ids \u003d [fixed_ip[\u0027subnet_id\u0027] for fixed_ip in"},{"line_number":140,"context_line":"                               port[\u0027port\u0027].get(\u0027fixed_ips\u0027, [])]"},{"line_number":141,"context_line":"        subnets \u003d plugin.get_subnets("},{"line_number":142,"context_line":"            context, filters\u003d{\u0027id\u0027: fixed_ip_subnet_ids})"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_079a296e","line":139,"range":{"start_line":139,"start_character":39,"end_line":139,"end_character":52},"updated":"2016-07-12 19:13:40.000000000","message":"Since this is called on only DHCP ports, I think you\u0027re lucky that there will be no KeyErrors.  But, it might still be a good idea to code defensively.","commit_id":"fa95253dacb54163f112a1763ea677429d447c45"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"b0e6ada08cd03b554ab78b9e73fa5cba0e3375c5","unresolved":false,"context_lines":[{"line_number":136,"context_line":"        # fixed ips as is."},{"line_number":137,"context_line":"        if \u0027segments\u0027 not in manager.NeutronManager.get_service_plugins():"},{"line_number":138,"context_line":"            return False"},{"line_number":139,"context_line":"        fixed_ip_subnet_ids \u003d [fixed_ip[\u0027subnet_id\u0027] for fixed_ip in"},{"line_number":140,"context_line":"                               port[\u0027port\u0027].get(\u0027fixed_ips\u0027, [])]"},{"line_number":141,"context_line":"        subnets \u003d plugin.get_subnets("},{"line_number":142,"context_line":"            context, filters\u003d{\u0027id\u0027: fixed_ip_subnet_ids})"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_191e865f","line":139,"range":{"start_line":139,"start_character":39,"end_line":139,"end_character":52},"in_reply_to":"1aa78d24_079a296e","updated":"2016-07-13 22:30:57.000000000","message":"good catch, but newest patch doesn\u0027t deal with fixed_ips anymore.","commit_id":"fa95253dacb54163f112a1763ea677429d447c45"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"48b1fc746f3fef7d80f00ba2f55f597a35c5dd6a","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        if \u0027segments\u0027 not in manager.NeutronManager.get_service_plugins():"},{"line_number":138,"context_line":"            return False"},{"line_number":139,"context_line":"        fixed_ip_subnet_ids \u003d [fixed_ip[\u0027subnet_id\u0027] for fixed_ip in"},{"line_number":140,"context_line":"                               port[\u0027port\u0027].get(\u0027fixed_ips\u0027, [])]"},{"line_number":141,"context_line":"        subnets \u003d plugin.get_subnets("},{"line_number":142,"context_line":"            context, filters\u003d{\u0027id\u0027: fixed_ip_subnet_ids})"},{"line_number":143,"context_line":"        for subnet in subnets:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_67904d44","line":140,"updated":"2016-07-12 19:13:40.000000000","message":"I think you might need to add \"if fixed_ip.get(\u0027subnet_id\u0027)\" to this to avoid the ones that don\u0027t have a subnet_id.","commit_id":"fa95253dacb54163f112a1763ea677429d447c45"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"283f5986690e66ef18a31dfd2360fd0986aaabfb","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        if \u0027segments\u0027 not in manager.NeutronManager.get_service_plugins():"},{"line_number":138,"context_line":"            return False"},{"line_number":139,"context_line":"        fixed_ip_subnet_ids \u003d [fixed_ip[\u0027subnet_id\u0027] for fixed_ip in"},{"line_number":140,"context_line":"                               port[\u0027port\u0027].get(\u0027fixed_ips\u0027, [])]"},{"line_number":141,"context_line":"        subnets \u003d plugin.get_subnets("},{"line_number":142,"context_line":"            context, filters\u003d{\u0027id\u0027: fixed_ip_subnet_ids})"},{"line_number":143,"context_line":"        for subnet in subnets:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_daaa8670","line":140,"updated":"2016-07-12 15:20:44.000000000","message":"If there are no fixed_ips should we be doing anything?  For example, we could return True below, but then do nothing on L162 and return [], then we just did a lot of work for nothing.","commit_id":"fa95253dacb54163f112a1763ea677429d447c45"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"b0e6ada08cd03b554ab78b9e73fa5cba0e3375c5","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        if \u0027segments\u0027 not in manager.NeutronManager.get_service_plugins():"},{"line_number":138,"context_line":"            return False"},{"line_number":139,"context_line":"        fixed_ip_subnet_ids \u003d [fixed_ip[\u0027subnet_id\u0027] for fixed_ip in"},{"line_number":140,"context_line":"                               port[\u0027port\u0027].get(\u0027fixed_ips\u0027, [])]"},{"line_number":141,"context_line":"        subnets \u003d plugin.get_subnets("},{"line_number":142,"context_line":"            context, filters\u003d{\u0027id\u0027: fixed_ip_subnet_ids})"},{"line_number":143,"context_line":"        for subnet in subnets:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_792fca88","line":140,"in_reply_to":"1aa78d24_67904d44","updated":"2016-07-13 22:30:57.000000000","message":"same as above.","commit_id":"fa95253dacb54163f112a1763ea677429d447c45"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"48b1fc746f3fef7d80f00ba2f55f597a35c5dd6a","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            # then we can assume this port is on a network without segments"},{"line_number":146,"context_line":"            # because a network cannot have a mixture of subnets on segments"},{"line_number":147,"context_line":"            # and subnets not on segments."},{"line_number":148,"context_line":"            if not subnet.get(\u0027segment_id\u0027):"},{"line_number":149,"context_line":"                return False"},{"line_number":150,"context_line":"        return True"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    def _get_port_fixed_ips_by_segment_host(self, plugin, context, host, port):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_5aa244af","line":149,"range":{"start_line":148,"start_character":12,"end_line":149,"end_character":28},"updated":"2016-07-12 19:13:40.000000000","message":"Could you just do this:\n\n  return subnet.get(\u0027segment_id\u0027)","commit_id":"fa95253dacb54163f112a1763ea677429d447c45"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"48b1fc746f3fef7d80f00ba2f55f597a35c5dd6a","unresolved":false,"context_lines":[{"line_number":147,"context_line":"            # and subnets not on segments."},{"line_number":148,"context_line":"            if not subnet.get(\u0027segment_id\u0027):"},{"line_number":149,"context_line":"                return False"},{"line_number":150,"context_line":"        return True"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    def _get_port_fixed_ips_by_segment_host(self, plugin, context, host, port):"},{"line_number":153,"context_line":"        segment_plugin \u003d manager.NeutronManager.get_service_plugins()["}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_3a8e582e","line":150,"range":{"start_line":150,"start_character":15,"end_line":150,"end_character":19},"updated":"2016-07-12 19:13:40.000000000","message":"Could subnets be empty?  If so, is True the right return value?","commit_id":"fa95253dacb54163f112a1763ea677429d447c45"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"283f5986690e66ef18a31dfd2360fd0986aaabfb","unresolved":false,"context_lines":[{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    def _get_port_fixed_ips_by_segment_host(self, plugin, context, host, port):"},{"line_number":153,"context_line":"        segment_plugin \u003d manager.NeutronManager.get_service_plugins()["},{"line_number":154,"context_line":"            \u0027segments\u0027]"},{"line_number":155,"context_line":"        fixed_ip_subnet_ids \u003d [fixed_ip[\u0027subnet_id\u0027] for fixed_ip in"},{"line_number":156,"context_line":"                               port[\u0027port\u0027].get(\u0027fixed_ips\u0027, [])]"},{"line_number":157,"context_line":"        segment_ids \u003d segment_plugin.get_segments_by_hosts(context, [host])"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_2090c242","line":154,"updated":"2016-07-12 15:20:44.000000000","message":"ditto","commit_id":"fa95253dacb54163f112a1763ea677429d447c45"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"283f5986690e66ef18a31dfd2360fd0986aaabfb","unresolved":false,"context_lines":[{"line_number":156,"context_line":"                               port[\u0027port\u0027].get(\u0027fixed_ips\u0027, [])]"},{"line_number":157,"context_line":"        segment_ids \u003d segment_plugin.get_segments_by_hosts(context, [host])"},{"line_number":158,"context_line":"        filters \u003d {\u0027segment_id\u0027: segment_ids, \u0027id\u0027: fixed_ip_subnet_ids}"},{"line_number":159,"context_line":"        filtered_subnets \u003d plugin.get_subnets(context, filters\u003dfilters)"},{"line_number":160,"context_line":"        filtered_subnet_ids \u003d [subnet[\u0027id\u0027] for subnet in filtered_subnets]"},{"line_number":161,"context_line":"        filtered_fixed_ips \u003d []"},{"line_number":162,"context_line":"        for fixed_ip in port[\u0027port\u0027].get(\u0027fixed_ips\u0027, []):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_1a61ae39","line":159,"updated":"2016-07-12 15:20:44.000000000","message":"To my eye, it seems we can almost remove _is_port_on_segment() and do everything at once, just filtering things here with a single call to get_subnets() like on L141.\n\nSo I must be missing something.","commit_id":"fa95253dacb54163f112a1763ea677429d447c45"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"2de4f53b9bf27a8c2fae28a69c82d337d7875b98","unresolved":false,"context_lines":[{"line_number":157,"context_line":"                                  \u0027enable_dhcp\u0027: [True]})"},{"line_number":158,"context_line":"            seg_subnet_ids \u003d [seg_subnet[\u0027id\u0027]"},{"line_number":159,"context_line":"                              for seg_subnet in host_seg_subnets]"},{"line_number":160,"context_line":"            seg_net_ids \u003d {seg_subnet[\u0027network_id\u0027]"},{"line_number":161,"context_line":"                           for seg_subnet in host_seg_subnets}"},{"line_number":162,"context_line":"            # Remove the subnets with segments that are not in the same"},{"line_number":163,"context_line":"            # segments as the host.  Do this only for the networks that are"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_75578e26","line":160,"range":{"start_line":160,"start_character":12,"end_line":160,"end_character":23},"updated":"2016-07-15 03:13:38.000000000","message":"I think the nomenclature needs some adjustment. We are looking for routed networks, not multi-segment networks","commit_id":"ae064d23ab58a770f7647f0a3e2c862cbc40f3f3"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"23b36d68997b9ea88d1ee7d4d97a7136e3f35619","unresolved":false,"context_lines":[{"line_number":157,"context_line":"                                  \u0027enable_dhcp\u0027: [True]})"},{"line_number":158,"context_line":"            seg_subnet_ids \u003d [seg_subnet[\u0027id\u0027]"},{"line_number":159,"context_line":"                              for seg_subnet in host_seg_subnets]"},{"line_number":160,"context_line":"            seg_net_ids \u003d {seg_subnet[\u0027network_id\u0027]"},{"line_number":161,"context_line":"                           for seg_subnet in host_seg_subnets}"},{"line_number":162,"context_line":"            # Remove the subnets with segments that are not in the same"},{"line_number":163,"context_line":"            # segments as the host.  Do this only for the networks that are"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_b8b44bd4","line":160,"range":{"start_line":160,"start_character":12,"end_line":160,"end_character":23},"in_reply_to":"1aa78d24_75578e26","updated":"2016-07-15 03:38:40.000000000","message":"Done","commit_id":"ae064d23ab58a770f7647f0a3e2c862cbc40f3f3"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"2de4f53b9bf27a8c2fae28a69c82d337d7875b98","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        seg_plug \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":196,"context_line":"            mp_net.SEGMENTS)"},{"line_number":197,"context_line":"        if seg_plug:"},{"line_number":198,"context_line":"            network_segs \u003d seg_plug.get_segments(context, filters\u003dfilters)"},{"line_number":199,"context_line":"            if network_segs:"},{"line_number":200,"context_line":"                segment_ids \u003d seg_plug.get_segments_by_hosts(context, [host])"},{"line_number":201,"context_line":"                # There might be something to do if no segment_ids exist that"},{"line_number":202,"context_line":"                # are mapped to this host.  However, it seems that if this"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_9695e589","line":199,"range":{"start_line":198,"start_character":12,"end_line":199,"end_character":28},"updated":"2016-07-15 03:13:38.000000000","message":"A network at least has 1 segment. So why do this? Why not just go ahead and get the host segments?","commit_id":"ae064d23ab58a770f7647f0a3e2c862cbc40f3f3"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"23b36d68997b9ea88d1ee7d4d97a7136e3f35619","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        seg_plug \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":196,"context_line":"            mp_net.SEGMENTS)"},{"line_number":197,"context_line":"        if seg_plug:"},{"line_number":198,"context_line":"            network_segs \u003d seg_plug.get_segments(context, filters\u003dfilters)"},{"line_number":199,"context_line":"            if network_segs:"},{"line_number":200,"context_line":"                segment_ids \u003d seg_plug.get_segments_by_hosts(context, [host])"},{"line_number":201,"context_line":"                # There might be something to do if no segment_ids exist that"},{"line_number":202,"context_line":"                # are mapped to this host.  However, it seems that if this"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_38a21b9b","line":199,"range":{"start_line":198,"start_character":12,"end_line":199,"end_character":28},"in_reply_to":"1aa78d24_9695e589","updated":"2016-07-15 03:38:40.000000000","message":"Done","commit_id":"ae064d23ab58a770f7647f0a3e2c862cbc40f3f3"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"2de4f53b9bf27a8c2fae28a69c82d337d7875b98","unresolved":false,"context_lines":[{"line_number":203,"context_line":"                # host is not mapped to any segments and this is a segmented"},{"line_number":204,"context_line":"                # network, then this host shouldn\u0027t be have even been scheduled"},{"line_number":205,"context_line":"                # to."},{"line_number":206,"context_line":"                seg_filters[\u0027segment_id\u0027] \u003d segment_ids"},{"line_number":207,"context_line":"        filters.update(seg_filters)"},{"line_number":208,"context_line":"        # NOTE(kevinbenton): we sort these because the agent builds tags"},{"line_number":209,"context_line":"        # based on position in the list and has to restart the process if"},{"line_number":210,"context_line":"        # the order changes."}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_b6a4614b","line":207,"range":{"start_line":206,"start_character":0,"end_line":207,"end_character":35},"updated":"2016-07-15 03:13:38.000000000","message":"If I understand correctly, if the network is not a routed network, the behavior should be the same as before. In other words, we should return all the subnets. So let\u0027s say you have a multi-segment network that is not a routed network (because subnets are associated to the network). With this additional filter, I think we are going to get no subnets in L212","commit_id":"ae064d23ab58a770f7647f0a3e2c862cbc40f3f3"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"23b36d68997b9ea88d1ee7d4d97a7136e3f35619","unresolved":false,"context_lines":[{"line_number":203,"context_line":"                # host is not mapped to any segments and this is a segmented"},{"line_number":204,"context_line":"                # network, then this host shouldn\u0027t be have even been scheduled"},{"line_number":205,"context_line":"                # to."},{"line_number":206,"context_line":"                seg_filters[\u0027segment_id\u0027] \u003d segment_ids"},{"line_number":207,"context_line":"        filters.update(seg_filters)"},{"line_number":208,"context_line":"        # NOTE(kevinbenton): we sort these because the agent builds tags"},{"line_number":209,"context_line":"        # based on position in the list and has to restart the process if"},{"line_number":210,"context_line":"        # the order changes."}],"source_content_type":"text/x-python","patch_set":6,"id":"1aa78d24_58a727a7","line":207,"range":{"start_line":206,"start_character":0,"end_line":207,"end_character":35},"in_reply_to":"1aa78d24_b6a4614b","updated":"2016-07-15 03:38:40.000000000","message":"Done","commit_id":"ae064d23ab58a770f7647f0a3e2c862cbc40f3f3"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"dfb22e6d4b59f5c278f8f8fbb6f8ff998c1a57e9","unresolved":false,"context_lines":[{"line_number":136,"context_line":"        LOG.debug(\u0027get_active_networks_info from %s\u0027, host)"},{"line_number":137,"context_line":"        networks \u003d self._get_active_networks(context, **kwargs)"},{"line_number":138,"context_line":"        plugin \u003d manager.NeutronManager.get_plugin()"},{"line_number":139,"context_line":"        filters \u003d {\u0027network_id\u0027: [network[\u0027id\u0027] for network in networks]}"},{"line_number":140,"context_line":"        ports \u003d plugin.get_ports(context, filters\u003dfilters)"},{"line_number":141,"context_line":"        filters[\u0027enable_dhcp\u0027] \u003d [True]"},{"line_number":142,"context_line":"        # NOTE(kevinbenton): we sort these because the agent builds tags"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_e751e7d9","line":139,"range":{"start_line":139,"start_character":8,"end_line":139,"end_character":73},"updated":"2016-07-19 23:42:19.000000000","message":"Just thinking out loud here.  I wonder if it would be worth it to filter out ports that aren\u0027t on the segment.  It doesn\u0027t really hurt to have them though.  I guess it might hurt if it becomes a scale problem.","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"181bf19811fd27ad5a347581fd626024475777e3","unresolved":false,"context_lines":[{"line_number":136,"context_line":"        LOG.debug(\u0027get_active_networks_info from %s\u0027, host)"},{"line_number":137,"context_line":"        networks \u003d self._get_active_networks(context, **kwargs)"},{"line_number":138,"context_line":"        plugin \u003d manager.NeutronManager.get_plugin()"},{"line_number":139,"context_line":"        filters \u003d {\u0027network_id\u0027: [network[\u0027id\u0027] for network in networks]}"},{"line_number":140,"context_line":"        ports \u003d plugin.get_ports(context, filters\u003dfilters)"},{"line_number":141,"context_line":"        filters[\u0027enable_dhcp\u0027] \u003d [True]"},{"line_number":142,"context_line":"        # NOTE(kevinbenton): we sort these because the agent builds tags"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_732f943e","line":139,"range":{"start_line":139,"start_character":8,"end_line":139,"end_character":73},"in_reply_to":"dada55a8_e751e7d9","updated":"2016-07-25 15:29:02.000000000","message":"i was thinking about doing this too but decided against because it doesn\u0027t do anything, other than trim down the number of ports returned.","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"dfb22e6d4b59f5c278f8f8fbb6f8ff998c1a57e9","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        # there are active dhcp enabled subnets, then filter out the subnets"},{"line_number":150,"context_line":"        # that are not on the host\u0027s segment."},{"line_number":151,"context_line":"        seg_plug \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":152,"context_line":"            mp_net.SEGMENTS)"},{"line_number":153,"context_line":"        if seg_plug and subnets:"},{"line_number":154,"context_line":"            host_segment_ids \u003d seg_plug.get_segments_by_hosts(context, [host])"},{"line_number":155,"context_line":"            host_seg_subnets \u003d plugin.get_subnets("}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_a7cd2f5f","line":152,"range":{"start_line":152,"start_character":12,"end_line":152,"end_character":27},"updated":"2016-07-19 23:42:19.000000000","message":"It is unnatural to get this constant from the multiprovidernet extension.  It is pretty much a coincidence that it fits but there isn\u0027t any hard relationship.  I think this constant should come from the service plugin code somewhere or not be a constant.  What I think we should do is follow up this patch set with a refactoring to add a proper constant.\n\nI guess it could come from somewhere related to here [1] (but I\u0027m not sure why this method is in the extension rather than the service plugin code.)\n\n [1] https://github.com/openstack/neutron/blob/122a971656671f92927d44ddd3725cca74b4e0bb/neutron/extensions/segment.py#L217-L219","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"181bf19811fd27ad5a347581fd626024475777e3","unresolved":false,"context_lines":[{"line_number":149,"context_line":"        # there are active dhcp enabled subnets, then filter out the subnets"},{"line_number":150,"context_line":"        # that are not on the host\u0027s segment."},{"line_number":151,"context_line":"        seg_plug \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":152,"context_line":"            mp_net.SEGMENTS)"},{"line_number":153,"context_line":"        if seg_plug and subnets:"},{"line_number":154,"context_line":"            host_segment_ids \u003d seg_plug.get_segments_by_hosts(context, [host])"},{"line_number":155,"context_line":"            host_seg_subnets \u003d plugin.get_subnets("}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_d3b000e0","line":152,"range":{"start_line":152,"start_character":12,"end_line":152,"end_character":27},"in_reply_to":"dada55a8_a7cd2f5f","updated":"2016-07-25 15:29:02.000000000","message":"yeah i forgot to go back and do this.  It makes sense for that to be on the base class though because the plugin type shouldn\u0027t change per implementation of the SegmentPluginBase.  And service plugin\u0027s base classes seem to always be defined the extension, that\u0027s just the way its been done, no technical reason for it.","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"4291cd1a069e5cf12270a3c909cfc02aa2ceb4ea","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        # that are not on the host\u0027s segment."},{"line_number":151,"context_line":"        seg_plug \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":152,"context_line":"            mp_net.SEGMENTS)"},{"line_number":153,"context_line":"        if seg_plug and subnets:"},{"line_number":154,"context_line":"            host_segment_ids \u003d seg_plug.get_segments_by_hosts(context, [host])"},{"line_number":155,"context_line":"            host_seg_subnets \u003d plugin.get_subnets("},{"line_number":156,"context_line":"                context, filters\u003d{\u0027segment_id\u0027: host_segment_ids,"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_27093fde","line":153,"updated":"2016-07-19 23:41:02.000000000","message":"Looking at what you did in get_network_info below got me thinking how to optimize this method. First, I would add under L153:\n\nseg_subnets \u003d [subnet for subnet in subnets if subnet.get(\u0027segment_id\u0027)]\nif seg_subnets:\n\nUnderneath this I would put L154-169\n\nThis way we save a trip to the DB to get host_segment_ids if we don\u0027t need them","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"181bf19811fd27ad5a347581fd626024475777e3","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        # that are not on the host\u0027s segment."},{"line_number":151,"context_line":"        seg_plug \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":152,"context_line":"            mp_net.SEGMENTS)"},{"line_number":153,"context_line":"        if seg_plug and subnets:"},{"line_number":154,"context_line":"            host_segment_ids \u003d seg_plug.get_segments_by_hosts(context, [host])"},{"line_number":155,"context_line":"            host_seg_subnets \u003d plugin.get_subnets("},{"line_number":156,"context_line":"                context, filters\u003d{\u0027segment_id\u0027: host_segment_ids,"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_b65a9a28","line":153,"in_reply_to":"dada55a8_27093fde","updated":"2016-07-25 15:29:02.000000000","message":"Done","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"4291cd1a069e5cf12270a3c909cfc02aa2ceb4ea","unresolved":false,"context_lines":[{"line_number":152,"context_line":"            mp_net.SEGMENTS)"},{"line_number":153,"context_line":"        if seg_plug and subnets:"},{"line_number":154,"context_line":"            host_segment_ids \u003d seg_plug.get_segments_by_hosts(context, [host])"},{"line_number":155,"context_line":"            host_seg_subnets \u003d plugin.get_subnets("},{"line_number":156,"context_line":"                context, filters\u003d{\u0027segment_id\u0027: host_segment_ids,"},{"line_number":157,"context_line":"                                  \u0027enable_dhcp\u0027: [True]})"},{"line_number":158,"context_line":"            seg_subnet_ids \u003d [seg_subnet[\u0027id\u0027]"},{"line_number":159,"context_line":"                              for seg_subnet in host_seg_subnets]"},{"line_number":160,"context_line":"            routed_net_ids \u003d {seg_subnet[\u0027network_id\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_a4f40d87","line":157,"range":{"start_line":155,"start_character":12,"end_line":157,"end_character":57},"updated":"2016-07-19 23:41:02.000000000","message":"Another optimization is to save a call to the plugin and do instead something like:\n\nhost_seg_subnets \u003d [subnet for subnet in seg_subnets if subnet[\u0027segment_id\u0027] in host_segment_ids]","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"181bf19811fd27ad5a347581fd626024475777e3","unresolved":false,"context_lines":[{"line_number":152,"context_line":"            mp_net.SEGMENTS)"},{"line_number":153,"context_line":"        if seg_plug and subnets:"},{"line_number":154,"context_line":"            host_segment_ids \u003d seg_plug.get_segments_by_hosts(context, [host])"},{"line_number":155,"context_line":"            host_seg_subnets \u003d plugin.get_subnets("},{"line_number":156,"context_line":"                context, filters\u003d{\u0027segment_id\u0027: host_segment_ids,"},{"line_number":157,"context_line":"                                  \u0027enable_dhcp\u0027: [True]})"},{"line_number":158,"context_line":"            seg_subnet_ids \u003d [seg_subnet[\u0027id\u0027]"},{"line_number":159,"context_line":"                              for seg_subnet in host_seg_subnets]"},{"line_number":160,"context_line":"            routed_net_ids \u003d {seg_subnet[\u0027network_id\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_d6574e00","line":157,"range":{"start_line":155,"start_character":12,"end_line":157,"end_character":57},"in_reply_to":"dada55a8_a4f40d87","updated":"2016-07-25 15:29:02.000000000","message":"Done","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"dfb22e6d4b59f5c278f8f8fbb6f8ff998c1a57e9","unresolved":false,"context_lines":[{"line_number":155,"context_line":"            host_seg_subnets \u003d plugin.get_subnets("},{"line_number":156,"context_line":"                context, filters\u003d{\u0027segment_id\u0027: host_segment_ids,"},{"line_number":157,"context_line":"                                  \u0027enable_dhcp\u0027: [True]})"},{"line_number":158,"context_line":"            seg_subnet_ids \u003d [seg_subnet[\u0027id\u0027]"},{"line_number":159,"context_line":"                              for seg_subnet in host_seg_subnets]"},{"line_number":160,"context_line":"            routed_net_ids \u003d {seg_subnet[\u0027network_id\u0027]"},{"line_number":161,"context_line":"                              for seg_subnet in host_seg_subnets}"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_e7e3a731","line":158,"range":{"start_line":158,"start_character":29,"end_line":158,"end_character":30},"updated":"2016-07-19 23:42:19.000000000","message":"Make this a set too because of the way it is used on L168.","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"181bf19811fd27ad5a347581fd626024475777e3","unresolved":false,"context_lines":[{"line_number":155,"context_line":"            host_seg_subnets \u003d plugin.get_subnets("},{"line_number":156,"context_line":"                context, filters\u003d{\u0027segment_id\u0027: host_segment_ids,"},{"line_number":157,"context_line":"                                  \u0027enable_dhcp\u0027: [True]})"},{"line_number":158,"context_line":"            seg_subnet_ids \u003d [seg_subnet[\u0027id\u0027]"},{"line_number":159,"context_line":"                              for seg_subnet in host_seg_subnets]"},{"line_number":160,"context_line":"            routed_net_ids \u003d {seg_subnet[\u0027network_id\u0027]"},{"line_number":161,"context_line":"                              for seg_subnet in host_seg_subnets}"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_7650220b","line":158,"range":{"start_line":158,"start_character":29,"end_line":158,"end_character":30},"in_reply_to":"dada55a8_e7e3a731","updated":"2016-07-25 15:29:02.000000000","message":"Done","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"4291cd1a069e5cf12270a3c909cfc02aa2ceb4ea","unresolved":false,"context_lines":[{"line_number":160,"context_line":"            routed_net_ids \u003d {seg_subnet[\u0027network_id\u0027]"},{"line_number":161,"context_line":"                              for seg_subnet in host_seg_subnets}"},{"line_number":162,"context_line":"            # Remove the subnets with segments that are not in the same"},{"line_number":163,"context_line":"            # segments as the host.  Do this only for the networks that are"},{"line_number":164,"context_line":"            # segmented because we want non-routed networks to work as"},{"line_number":165,"context_line":"            # before."},{"line_number":166,"context_line":"            for subnet in subnets[:]:"},{"line_number":167,"context_line":"                if (subnet[\u0027network_id\u0027] in routed_net_ids and"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_679df758","line":164,"range":{"start_line":163,"start_character":54,"end_line":164,"end_character":23},"updated":"2016-07-19 23:41:02.000000000","message":"Replace this with \u0027routed networks\u0027","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"181bf19811fd27ad5a347581fd626024475777e3","unresolved":false,"context_lines":[{"line_number":160,"context_line":"            routed_net_ids \u003d {seg_subnet[\u0027network_id\u0027]"},{"line_number":161,"context_line":"                              for seg_subnet in host_seg_subnets}"},{"line_number":162,"context_line":"            # Remove the subnets with segments that are not in the same"},{"line_number":163,"context_line":"            # segments as the host.  Do this only for the networks that are"},{"line_number":164,"context_line":"            # segmented because we want non-routed networks to work as"},{"line_number":165,"context_line":"            # before."},{"line_number":166,"context_line":"            for subnet in subnets[:]:"},{"line_number":167,"context_line":"                if (subnet[\u0027network_id\u0027] in routed_net_ids and"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_1642863e","line":164,"range":{"start_line":163,"start_character":54,"end_line":164,"end_character":23},"in_reply_to":"dada55a8_679df758","updated":"2016-07-25 15:29:02.000000000","message":"Done","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"dfb22e6d4b59f5c278f8f8fbb6f8ff998c1a57e9","unresolved":false,"context_lines":[{"line_number":163,"context_line":"            # segments as the host.  Do this only for the networks that are"},{"line_number":164,"context_line":"            # segmented because we want non-routed networks to work as"},{"line_number":165,"context_line":"            # before."},{"line_number":166,"context_line":"            for subnet in subnets[:]:"},{"line_number":167,"context_line":"                if (subnet[\u0027network_id\u0027] in routed_net_ids and"},{"line_number":168,"context_line":"                        subnet[\u0027id\u0027] not in seg_subnet_ids):"},{"line_number":169,"context_line":"                    subnets.remove(subnet)"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"        grouped_subnets \u003d self._group_by_network_id(subnets)"},{"line_number":172,"context_line":"        grouped_ports \u003d self._group_by_network_id(ports)"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_a7924f9d","line":169,"range":{"start_line":166,"start_character":12,"end_line":169,"end_character":42},"updated":"2016-07-19 23:42:19.000000000","message":"Just a nitty suggestion. Prefer this pattern because it has a little better performance characteristic than removing items while iterating over a copy:\n\n  subnets \u003d [s for s in subnets if s[\u0027network_id\u0027] not in routed_net_ids or s[\u0027id\u0027] in seg_subnet_ids)]","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"181bf19811fd27ad5a347581fd626024475777e3","unresolved":false,"context_lines":[{"line_number":163,"context_line":"            # segments as the host.  Do this only for the networks that are"},{"line_number":164,"context_line":"            # segmented because we want non-routed networks to work as"},{"line_number":165,"context_line":"            # before."},{"line_number":166,"context_line":"            for subnet in subnets[:]:"},{"line_number":167,"context_line":"                if (subnet[\u0027network_id\u0027] in routed_net_ids and"},{"line_number":168,"context_line":"                        subnet[\u0027id\u0027] not in seg_subnet_ids):"},{"line_number":169,"context_line":"                    subnets.remove(subnet)"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"        grouped_subnets \u003d self._group_by_network_id(subnets)"},{"line_number":172,"context_line":"        grouped_ports \u003d self._group_by_network_id(ports)"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_76eca229","line":169,"range":{"start_line":166,"start_character":12,"end_line":169,"end_character":42},"in_reply_to":"dada55a8_a7924f9d","updated":"2016-07-25 15:29:02.000000000","message":"Done","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"1dd8e9241a286a71596d80d2ceb3096286ac3bf1","unresolved":false,"context_lines":[{"line_number":194,"context_line":"        subnets \u003d plugin.get_subnets(context, filters\u003dfilters)"},{"line_number":195,"context_line":"        seg_plug \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":196,"context_line":"            mp_net.SEGMENTS)"},{"line_number":197,"context_line":"        if seg_plug:"},{"line_number":198,"context_line":"            seg_subnets \u003d [subnet for subnet in subnets"},{"line_number":199,"context_line":"                           if subnet.get(\u0027segment_id\u0027)]"},{"line_number":200,"context_line":"            # If there are no subnets with segments, then this is not a routed"}],"source_content_type":"text/x-python","patch_set":7,"id":"fad55976_d6cfa1b1","line":197,"updated":"2016-07-15 19:28:48.000000000","message":"nit: might as well check subnets here","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"181bf19811fd27ad5a347581fd626024475777e3","unresolved":false,"context_lines":[{"line_number":194,"context_line":"        subnets \u003d plugin.get_subnets(context, filters\u003dfilters)"},{"line_number":195,"context_line":"        seg_plug \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":196,"context_line":"            mp_net.SEGMENTS)"},{"line_number":197,"context_line":"        if seg_plug:"},{"line_number":198,"context_line":"            seg_subnets \u003d [subnet for subnet in subnets"},{"line_number":199,"context_line":"                           if subnet.get(\u0027segment_id\u0027)]"},{"line_number":200,"context_line":"            # If there are no subnets with segments, then this is not a routed"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_36686a87","line":197,"in_reply_to":"dada55a8_8710eb02","updated":"2016-07-25 15:29:02.000000000","message":"i did it anyway bc it reads better.","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"dfb22e6d4b59f5c278f8f8fbb6f8ff998c1a57e9","unresolved":false,"context_lines":[{"line_number":194,"context_line":"        subnets \u003d plugin.get_subnets(context, filters\u003dfilters)"},{"line_number":195,"context_line":"        seg_plug \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":196,"context_line":"            mp_net.SEGMENTS)"},{"line_number":197,"context_line":"        if seg_plug:"},{"line_number":198,"context_line":"            seg_subnets \u003d [subnet for subnet in subnets"},{"line_number":199,"context_line":"                           if subnet.get(\u0027segment_id\u0027)]"},{"line_number":200,"context_line":"            # If there are no subnets with segments, then this is not a routed"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_8710eb02","line":197,"in_reply_to":"fad55976_b6f2ed6d","updated":"2016-07-19 23:42:19.000000000","message":"It\u0027ll get checked on L202 after a trivial creation of an empty list on L198.  So, it isn\u0027t really necessary.  Either way is good for me.","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"dc361c8f3bfac5f7a149817da911f88e442f76fb","unresolved":false,"context_lines":[{"line_number":194,"context_line":"        subnets \u003d plugin.get_subnets(context, filters\u003dfilters)"},{"line_number":195,"context_line":"        seg_plug \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":196,"context_line":"            mp_net.SEGMENTS)"},{"line_number":197,"context_line":"        if seg_plug:"},{"line_number":198,"context_line":"            seg_subnets \u003d [subnet for subnet in subnets"},{"line_number":199,"context_line":"                           if subnet.get(\u0027segment_id\u0027)]"},{"line_number":200,"context_line":"            # If there are no subnets with segments, then this is not a routed"}],"source_content_type":"text/x-python","patch_set":7,"id":"fad55976_b6f2ed6d","line":197,"in_reply_to":"fad55976_d6cfa1b1","updated":"2016-07-15 19:45:36.000000000","message":"will do if I need to respin","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d7914bf32986cbffa628bc6d24a9223ba7f08c80","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                                if subnet[\u0027segment_id\u0027] in host_segment_ids]"},{"line_number":159,"context_line":"            seg_subnet_ids \u003d {seg_subnet[\u0027id\u0027]"},{"line_number":160,"context_line":"                              for seg_subnet in host_seg_subnets}"},{"line_number":161,"context_line":"            routed_net_ids \u003d {seg_subnet[\u0027network_id\u0027]"},{"line_number":162,"context_line":"                              for seg_subnet in host_seg_subnets}"},{"line_number":163,"context_line":"            # Remove the subnets with segments that are not in the same"},{"line_number":164,"context_line":"            # segments as the host.  Do this only for the networks that are"},{"line_number":165,"context_line":"            # routed because we want non-routed networks to work as"}],"source_content_type":"text/x-python","patch_set":8,"id":"dada55a8_e5abad1c","line":162,"range":{"start_line":161,"start_character":12,"end_line":162,"end_character":65},"updated":"2016-07-25 18:02:51.000000000","message":"I don\u0027t think this is quite right. It could miss some networks in some corner cases like if a subnet hasn\u0027t been created yet on a particular segment of a routed network that this host touches. I\u0027m thinking the following might be more accurate:\n\n  routed_net_ids \u003d {seg_subnet[\u0027network_id\u0027] for seg_subnet in seg_subnets}\n\nSorry I didn\u0027t catch it before.","commit_id":"f0ca4c2073a1ac7a1da18ba60230bf5fc73980f6"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"a8b1637bd10eeab18ddb02c2f3c5435c8bad2da8","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                                if subnet[\u0027segment_id\u0027] in host_segment_ids]"},{"line_number":159,"context_line":"            seg_subnet_ids \u003d {seg_subnet[\u0027id\u0027]"},{"line_number":160,"context_line":"                              for seg_subnet in host_seg_subnets}"},{"line_number":161,"context_line":"            routed_net_ids \u003d {seg_subnet[\u0027network_id\u0027]"},{"line_number":162,"context_line":"                              for seg_subnet in host_seg_subnets}"},{"line_number":163,"context_line":"            # Remove the subnets with segments that are not in the same"},{"line_number":164,"context_line":"            # segments as the host.  Do this only for the networks that are"},{"line_number":165,"context_line":"            # routed because we want non-routed networks to work as"}],"source_content_type":"text/x-python","patch_set":8,"id":"dada55a8_fc73354c","line":162,"range":{"start_line":161,"start_character":12,"end_line":162,"end_character":65},"in_reply_to":"dada55a8_e5abad1c","updated":"2016-07-26 03:58:58.000000000","message":"i see what you mean. good catch. thats an interesting corner case.","commit_id":"f0ca4c2073a1ac7a1da18ba60230bf5fc73980f6"}],"neutron/services/segments/db.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"48b1fc746f3fef7d80f00ba2f55f597a35c5dd6a","unresolved":false,"context_lines":[{"line_number":141,"context_line":"    @log_helpers.log_method_call"},{"line_number":142,"context_line":"    def get_segments_by_hosts(self, context, hosts):"},{"line_number":143,"context_line":"        query \u003d context.session.query(SegmentHostMapping).filter("},{"line_number":144,"context_line":"            SegmentHostMapping.host.in_(hosts))"},{"line_number":145,"context_line":"        return [mapping.segment_id for mapping in query]"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    @log_helpers.log_method_call"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_27e3e5a5","line":144,"range":{"start_line":144,"start_character":36,"end_line":144,"end_character":39},"updated":"2016-07-12 19:13:40.000000000","message":"I think the in_ operator can be inefficient if hosts happens to be an empty list.  You might add this to the beginning:\n\n  if not hosts:\n      return []","commit_id":"fa95253dacb54163f112a1763ea677429d447c45"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"b0e6ada08cd03b554ab78b9e73fa5cba0e3375c5","unresolved":false,"context_lines":[{"line_number":141,"context_line":"    @log_helpers.log_method_call"},{"line_number":142,"context_line":"    def get_segments_by_hosts(self, context, hosts):"},{"line_number":143,"context_line":"        query \u003d context.session.query(SegmentHostMapping).filter("},{"line_number":144,"context_line":"            SegmentHostMapping.host.in_(hosts))"},{"line_number":145,"context_line":"        return [mapping.segment_id for mapping in query]"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    @log_helpers.log_method_call"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_9960b6d1","line":144,"range":{"start_line":144,"start_character":36,"end_line":144,"end_character":39},"in_reply_to":"1aa78d24_27e3e5a5","updated":"2016-07-13 22:30:57.000000000","message":"Done","commit_id":"fa95253dacb54163f112a1763ea677429d447c45"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"48b1fc746f3fef7d80f00ba2f55f597a35c5dd6a","unresolved":false,"context_lines":[{"line_number":142,"context_line":"    def get_segments_by_hosts(self, context, hosts):"},{"line_number":143,"context_line":"        query \u003d context.session.query(SegmentHostMapping).filter("},{"line_number":144,"context_line":"            SegmentHostMapping.host.in_(hosts))"},{"line_number":145,"context_line":"        return [mapping.segment_id for mapping in query]"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    @log_helpers.log_method_call"},{"line_number":148,"context_line":"    def delete_segment(self, context, uuid):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_87a1f948","line":145,"range":{"start_line":145,"start_character":15,"end_line":145,"end_character":16},"updated":"2016-07-12 19:13:40.000000000","message":"Should this be a set?  Or, maybe use a set to make them unique and then convert to a list.\n\n  return list({mapping.segment_id for mapping in query})","commit_id":"fa95253dacb54163f112a1763ea677429d447c45"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"b0e6ada08cd03b554ab78b9e73fa5cba0e3375c5","unresolved":false,"context_lines":[{"line_number":142,"context_line":"    def get_segments_by_hosts(self, context, hosts):"},{"line_number":143,"context_line":"        query \u003d context.session.query(SegmentHostMapping).filter("},{"line_number":144,"context_line":"            SegmentHostMapping.host.in_(hosts))"},{"line_number":145,"context_line":"        return [mapping.segment_id for mapping in query]"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    @log_helpers.log_method_call"},{"line_number":148,"context_line":"    def delete_segment(self, context, uuid):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_795d0a15","line":145,"range":{"start_line":145,"start_character":15,"end_line":145,"end_character":16},"in_reply_to":"1aa78d24_87a1f948","updated":"2016-07-13 22:30:57.000000000","message":"Done","commit_id":"fa95253dacb54163f112a1763ea677429d447c45"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d7914bf32986cbffa628bc6d24a9223ba7f08c80","unresolved":false,"context_lines":[{"line_number":144,"context_line":"            return []"},{"line_number":145,"context_line":"        query \u003d context.session.query(SegmentHostMapping).filter("},{"line_number":146,"context_line":"            SegmentHostMapping.host.in_(hosts))"},{"line_number":147,"context_line":"        return list({mapping.segment_id for mapping in query})"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"    @log_helpers.log_method_call"},{"line_number":150,"context_line":"    def delete_segment(self, context, uuid):"}],"source_content_type":"text/x-python","patch_set":8,"id":"dada55a8_05140142","line":147,"range":{"start_line":147,"start_character":15,"end_line":147,"end_character":20},"updated":"2016-07-25 18:02:51.000000000","message":"nit:  Looking at this again, a set would be the more efficient choice just because it is used in \"in\" expressions in the other file. But, I don\u0027t imagine this list will get very long at all, especially since it is normally called with only host. So, in practice, it isn\u0027t likely to make much of a difference. So, I wouldn\u0027t change it unless you are uploading another patch and you want to.","commit_id":"f0ca4c2073a1ac7a1da18ba60230bf5fc73980f6"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"a8b1637bd10eeab18ddb02c2f3c5435c8bad2da8","unresolved":false,"context_lines":[{"line_number":144,"context_line":"            return []"},{"line_number":145,"context_line":"        query \u003d context.session.query(SegmentHostMapping).filter("},{"line_number":146,"context_line":"            SegmentHostMapping.host.in_(hosts))"},{"line_number":147,"context_line":"        return list({mapping.segment_id for mapping in query})"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"    @log_helpers.log_method_call"},{"line_number":150,"context_line":"    def delete_segment(self, context, uuid):"}],"source_content_type":"text/x-python","patch_set":8,"id":"dada55a8_3c9cadbf","line":147,"range":{"start_line":147,"start_character":15,"end_line":147,"end_character":20},"in_reply_to":"dada55a8_05140142","updated":"2016-07-26 03:58:58.000000000","message":"so on this one i was worried about returning a set because most people would expect a list from this type of call and I feel like a set is just too out of the norm to return.  I know people can look at the code at anytime to see what its doing, so I might be overthinking it.  I can change it if you feel strongly about it though.","commit_id":"f0ca4c2073a1ac7a1da18ba60230bf5fc73980f6"}],"neutron/tests/unit/api/rpc/handlers/test_dhcp_rpc.py":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"4291cd1a069e5cf12270a3c909cfc02aa2ceb4ea","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        hostseg_retval \u003d [{\u0027id\u0027: \u00271\u0027}, {\u0027id\u0027: \u00272\u0027}]"},{"line_number":86,"context_line":"        hostseg_subnets_retval \u003d [subnets[0]]"},{"line_number":87,"context_line":"        self.segment_plugin.get_segments_by_hosts.return_value \u003d hostseg_retval"},{"line_number":88,"context_line":"        self.plugin.get_subnets.side_effect \u003d [subnets, hostseg_subnets_retval]"},{"line_number":89,"context_line":"        networks \u003d self.callbacks.get_active_networks_info(mock.Mock(),"},{"line_number":90,"context_line":"                                                           host\u003d\u0027host\u0027)"},{"line_number":91,"context_line":"        expected \u003d [{\u0027id\u0027: \u0027a\u0027, \u0027subnets\u0027: [subnets[1]], \u0027ports\u0027: [port]},"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_27081f91","line":88,"updated":"2016-07-19 23:41:02.000000000","message":"If you implement the optimizations I suggest for dhcp_rpc.py, you will have to adjust this line","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"},{"author":{"_account_id":6951,"name":"Brandon Logan","email":"brandon.logan@rackspace.com","username":"brandon-logan"},"change_message_id":"181bf19811fd27ad5a347581fd626024475777e3","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        hostseg_retval \u003d [{\u0027id\u0027: \u00271\u0027}, {\u0027id\u0027: \u00272\u0027}]"},{"line_number":86,"context_line":"        hostseg_subnets_retval \u003d [subnets[0]]"},{"line_number":87,"context_line":"        self.segment_plugin.get_segments_by_hosts.return_value \u003d hostseg_retval"},{"line_number":88,"context_line":"        self.plugin.get_subnets.side_effect \u003d [subnets, hostseg_subnets_retval]"},{"line_number":89,"context_line":"        networks \u003d self.callbacks.get_active_networks_info(mock.Mock(),"},{"line_number":90,"context_line":"                                                           host\u003d\u0027host\u0027)"},{"line_number":91,"context_line":"        expected \u003d [{\u0027id\u0027: \u0027a\u0027, \u0027subnets\u0027: [subnets[1]], \u0027ports\u0027: [port]},"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_7c292189","line":88,"in_reply_to":"dada55a8_27081f91","updated":"2016-07-25 15:29:02.000000000","message":"Done","commit_id":"b714182251f9de33045d26d5350ab6f111c4aaf1"}]}