)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"cd9faf2fe739e184a1828b5953b190e631c85a0c","unresolved":false,"context_lines":[{"line_number":33,"context_line":"and because this scenario is synthetic I changed the tests to create"},{"line_number":34,"context_line":"networks through the plugin layer."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"Closes-Bug: #1478100"},{"line_number":37,"context_line":"Change-Id: I0501d47404c8adbec4bccb84ac5980e045da68b3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"fa1b9901_cce57d38","line":36,"updated":"2015-08-19 21:56:28.000000000","message":"Does it require a DocImpact flag? As dhcp placement now depends on bridge_mappings option?","commit_id":"b8cf9007a9ac9e9526e19e983ea4e07d73495258"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a2a01d0a1067950b33a67b8872e13963b00409a9","unresolved":false,"context_lines":[{"line_number":34,"context_line":"networks through the plugin layer."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"DocImpact"},{"line_number":37,"context_line":"Closes-Bug: #1478100"},{"line_number":38,"context_line":"Co-Authored-By: Cedric Brandily \u003czzelle@gmail.com\u003e"},{"line_number":39,"context_line":"Change-Id: I0501d47404c8adbec4bccb84ac5980e045da68b3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"da85f559_d01035ab","line":37,"updated":"2015-11-07 08:59:14.000000000","message":"I think that this is a feature and should consider having a BP for it.","commit_id":"0395af8db7932f9d7dc86adde5a0dbeca4a3b4c9"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"022ab0f6f62e65708354de16a5a0c13fcb498d19","unresolved":false,"context_lines":[{"line_number":34,"context_line":"networks through the plugin layer."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"DocImpact"},{"line_number":37,"context_line":"Closes-Bug: #1478100"},{"line_number":38,"context_line":"Co-Authored-By: Cedric Brandily \u003czzelle@gmail.com\u003e"},{"line_number":39,"context_line":"Change-Id: I0501d47404c8adbec4bccb84ac5980e045da68b3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"da85f559_1ce1b4fe","line":37,"in_reply_to":"da85f559_d01035ab","updated":"2015-11-09 21:49:39.000000000","message":"I have transform the bug into a rfe as i am not sure it requires a BP/spec","commit_id":"0395af8db7932f9d7dc86adde5a0dbeca4a3b4c9"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"ecc0be40355a25afdb2046efb75966cebc609f1e","unresolved":false,"context_lines":[{"line_number":33,"context_line":"and because this scenario is synthetic I changed the tests to create"},{"line_number":34,"context_line":"networks through the plugin layer."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"DocImpact"},{"line_number":37,"context_line":"Closes-Bug: #1478100"},{"line_number":38,"context_line":"Co-Authored-By: Cedric Brandily \u003czzelle@gmail.com\u003e"},{"line_number":39,"context_line":"Change-Id: I0501d47404c8adbec4bccb84ac5980e045da68b3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"7a740942_2879fefc","line":36,"updated":"2015-12-04 01:12:18.000000000","message":"Please follow guidelines as detailed here:\n\nhttp://lists.openstack.org/pipermail/openstack-dev/2015-November/080294.html","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"746bbf7621ba97faa16882610b048da524c63d8c","unresolved":false,"context_lines":[{"line_number":33,"context_line":"and because this scenario is synthetic I changed the tests to create"},{"line_number":34,"context_line":"networks through the plugin layer."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"DocImpact"},{"line_number":37,"context_line":"Closes-Bug: #1478100"},{"line_number":38,"context_line":"Co-Authored-By: Cedric Brandily \u003czzelle@gmail.com\u003e"},{"line_number":39,"context_line":"Change-Id: I0501d47404c8adbec4bccb84ac5980e045da68b3"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":19,"id":"7a740942_d458cd58","line":36,"in_reply_to":"7a740942_2879fefc","updated":"2015-12-11 00:07:34.000000000","message":"will do","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"}],"neutron/db/agents_db.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a2a01d0a1067950b33a67b8872e13963b00409a9","unresolved":false,"context_lines":[{"line_number":272,"context_line":"        A plugin can overload this method to define its own agent network_id"},{"line_number":273,"context_line":"        based filter."},{"line_number":274,"context_line":"        \"\"\""},{"line_number":275,"context_line":"        return candidate_agents"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    def _log_heartbeat(self, state, agent_db, agent_conf):"},{"line_number":278,"context_line":"        if agent_conf.get(\u0027log_agent_heartbeats\u0027):"}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_10179da4","line":275,"updated":"2015-11-07 08:59:14.000000000","message":"i think that you need to indicate the type that is retruned. Is this a list?","commit_id":"0395af8db7932f9d7dc86adde5a0dbeca4a3b4c9"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"022ab0f6f62e65708354de16a5a0c13fcb498d19","unresolved":false,"context_lines":[{"line_number":272,"context_line":"        A plugin can overload this method to define its own agent network_id"},{"line_number":273,"context_line":"        based filter."},{"line_number":274,"context_line":"        \"\"\""},{"line_number":275,"context_line":"        return candidate_agents"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":"    def _log_heartbeat(self, state, agent_db, agent_conf):"},{"line_number":278,"context_line":"        if agent_conf.get(\u0027log_agent_heartbeats\u0027):"}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_5cbe5cd5","line":275,"in_reply_to":"da85f559_10179da4","updated":"2015-11-09 21:49:39.000000000","message":"Done","commit_id":"0395af8db7932f9d7dc86adde5a0dbeca4a3b4c9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8389d4a5400df5a9435e8189197b3fd9ca67f942","unresolved":false,"context_lines":[{"line_number":288,"context_line":"        agent \u003d self._get_agent(context, id)"},{"line_number":289,"context_line":"        return self._make_agent_dict(agent, fields)"},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"    def filter_hosts_with_network_access("},{"line_number":292,"context_line":"            self, context, network_id, candidate_hosts):"},{"line_number":293,"context_line":"        \"\"\"Filter hosts with access to network_id."},{"line_number":294,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"5a710552_419a3416","line":291,"updated":"2015-12-14 23:56:26.000000000","message":"I guess this is a start at the plugin level method that I wanted.  I wonder if silently not filtering is the right thing to do by default.  Should we instead raise an exception and have the calling code handle it?  Even if it is a log message that says that the plugin doesn\u0027t support it in this case.  I tend to think it should be more explicit what we do with a plugin that doesn\u0027t support it.\n\nIt is reasonable to expect plugins to not support it for a while so breaking isn\u0027t a good option.  I\u0027m just saying that we should make it more deliberate.","commit_id":"ba0d3ef15ec022fad27cc2839e8a3de2eaa59b1c"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"4529c5be5c54b7a240bedbfecaae4a8a999ffa17","unresolved":false,"context_lines":[{"line_number":288,"context_line":"        agent \u003d self._get_agent(context, id)"},{"line_number":289,"context_line":"        return self._make_agent_dict(agent, fields)"},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"    def filter_hosts_with_network_access("},{"line_number":292,"context_line":"            self, context, network_id, candidate_hosts):"},{"line_number":293,"context_line":"        \"\"\"Filter hosts with access to network_id."},{"line_number":294,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"5a710552_5373ae80","line":291,"in_reply_to":"5a710552_419a3416","updated":"2015-12-15 22:08:21.000000000","message":"The aim of such filtering is to handle segments not available on every host. \n\nSuch usecase is perhaps not relevant for every plugin (vmware-nsx? tunnel/L3 based plugin?...). IMO, it\u0027s up to the plugin to decide to overload or not the default implementation, we should force them to overload it with a dummy implementation to avoid a log warning or an error.","commit_id":"ba0d3ef15ec022fad27cc2839e8a3de2eaa59b1c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"45ee0a52e8c44030c66b3029c2dac130685930d9","unresolved":false,"context_lines":[{"line_number":288,"context_line":"        agent \u003d self._get_agent(context, id)"},{"line_number":289,"context_line":"        return self._make_agent_dict(agent, fields)"},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"    def filter_hosts_with_network_access("},{"line_number":292,"context_line":"            self, context, network_id, candidate_hosts):"},{"line_number":293,"context_line":"        \"\"\"Filter hosts with access to network_id."},{"line_number":294,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"3a7e1126_0848835a","line":291,"in_reply_to":"5a710552_5373ae80","updated":"2015-12-17 21:53:11.000000000","message":"But it isn\u0027t the plugin trying to use this functionality.  It is the DHCP scheduler trying to use it.  So, if the functionality isn\u0027t provided by the plugin wouldn\u0027t it be reasonable to at least know that at the time that it is trying to invoke it?\n\nIs there an implicit assumption that if the plugin does not overload this then the network is reachable from all hosts?  If yes, maybe this is okay.","commit_id":"ba0d3ef15ec022fad27cc2839e8a3de2eaa59b1c"}],"neutron/neutron_plugin_base_v2.py":[{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"ff7942eb37e4e8a7fd007b0fc9b2293db2f29888","unresolved":false,"context_lines":[{"line_number":400,"context_line":"        return ()"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"    def filter_agents_with_network_access("},{"line_number":403,"context_line":"            self, context, network_id, candidate_agents):"},{"line_number":404,"context_line":"        \"\"\"Filter agents with access to network_id."},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"        A plugin can overload this method to define its own agent network_id"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a29b11f_9ff4c287","line":403,"updated":"2015-10-20 11:00:05.000000000","message":"i think AgentDbMixin is a more natural home for this.","commit_id":"e9beefb657994c928e7ff71c365ec91860756375"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"039b528daada6cc83fb77ef5ab30fb6c412ba2f9","unresolved":false,"context_lines":[{"line_number":400,"context_line":"        return ()"},{"line_number":401,"context_line":""},{"line_number":402,"context_line":"    def filter_agents_with_network_access("},{"line_number":403,"context_line":"            self, context, network_id, candidate_agents):"},{"line_number":404,"context_line":"        \"\"\"Filter agents with access to network_id."},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"        A plugin can overload this method to define its own agent network_id"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a29b11f_a778bbf9","line":403,"in_reply_to":"3a29b11f_9ff4c287","updated":"2015-10-26 22:37:59.000000000","message":"Done","commit_id":"e9beefb657994c928e7ff71c365ec91860756375"}],"neutron/plugins/ml2/driver_api.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a2a01d0a1067950b33a67b8872e13963b00409a9","unresolved":false,"context_lines":[{"line_number":911,"context_line":"        \"\"\"Check if agent has access to the segment."},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"        This method should return:"},{"line_number":914,"context_line":"        * None if the mechanism doesn\u0027t implement such feature,"},{"line_number":915,"context_line":"        * True if the agent has access to the segment,"},{"line_number":916,"context_line":"        * False if the agent has not access to the segment."},{"line_number":917,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_d0391543","line":914,"updated":"2015-11-07 08:59:14.000000000","message":"why not raise an exception NotImplemented?","commit_id":"0395af8db7932f9d7dc86adde5a0dbeca4a3b4c9"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"022ab0f6f62e65708354de16a5a0c13fcb498d19","unresolved":false,"context_lines":[{"line_number":911,"context_line":"        \"\"\"Check if agent has access to the segment."},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"        This method should return:"},{"line_number":914,"context_line":"        * None if the mechanism doesn\u0027t implement such feature,"},{"line_number":915,"context_line":"        * True if the agent has access to the segment,"},{"line_number":916,"context_line":"        * False if the agent has not access to the segment."},{"line_number":917,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_dc9fac4e","line":914,"in_reply_to":"da85f559_d0391543","updated":"2015-11-09 21:49:39.000000000","message":"afaik \"raise NotImplementedError\" means you must overload it. In this case, mechanism could overload it.","commit_id":"0395af8db7932f9d7dc86adde5a0dbeca4a3b4c9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0893e8f37b3fc6f10cc2de1f0bd5ca3dcbadf634","unresolved":false,"context_lines":[{"line_number":907,"context_line":""},{"line_number":908,"context_line":"    def filter_hosts_with_network_access("},{"line_number":909,"context_line":"            self, context, segments, candidate_hosts, agent_getter):"},{"line_number":910,"context_line":"        return None"},{"line_number":911,"context_line":""},{"line_number":912,"context_line":""},{"line_number":913,"context_line":"@six.add_metaclass(abc.ABCMeta)"}],"source_content_type":"text/x-python","patch_set":21,"id":"7a740942_cba1b895","line":910,"updated":"2015-12-13 11:45:02.000000000","message":"you do not need to specific None. That is done by default","commit_id":"a988e1860efc36fe34f7efce8a2ab0915c437880"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8389d4a5400df5a9435e8189197b3fd9ca67f942","unresolved":false,"context_lines":[{"line_number":916,"context_line":"        A driver can overload this method to return a subset of candidate_hosts"},{"line_number":917,"context_line":"        with the ones with access to at least one segment."},{"line_number":918,"context_line":"        \"\"\""},{"line_number":919,"context_line":"        raise NotImplementedError"},{"line_number":920,"context_line":""},{"line_number":921,"context_line":""},{"line_number":922,"context_line":"@six.add_metaclass(abc.ABCMeta)"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a710552_6f72c07e","line":919,"updated":"2015-12-14 23:56:26.000000000","message":"Why does this default to NotImplemented here and in agents_db it defaults to returning the complete list of candidates?\n\nUpdate:  See my other comments.  I see why you did this.","commit_id":"ba0d3ef15ec022fad27cc2839e8a3de2eaa59b1c"}],"neutron/plugins/ml2/drivers/l2pop/mech_driver.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"9483707fcec4ff8a9030980a71c5e98ae0568321","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        self.L2populationAgentNotify.remove_fdb_entries(self.rpc_ctx,"},{"line_number":55,"context_line":"            fdb_entries)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def filter_hosts_with_segment_access("},{"line_number":58,"context_line":"            self, context, segments, candidate_hosts, agent_getter):"},{"line_number":59,"context_line":"        # NOTE(cbrandily): let other mechanisms (openvswitch, linuxbridge, ...)"},{"line_number":60,"context_line":"        # perform the filtering"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_d768b1bc","line":57,"range":{"start_line":57,"start_character":8,"end_line":57,"end_character":40},"updated":"2016-01-08 22:41:37.000000000","message":"Does overriding this mean that the code in driver_api.py will report this mechanism driver as not supporting filtering?  I think it would then skip filtering for all the drivers, right?","commit_id":"5985917479a7db8d813ee13e6e1ccd1d63bfc90a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e0c2efcc83bb3469f753364454fb2b4ef7cf1984","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        self.L2populationAgentNotify.remove_fdb_entries(self.rpc_ctx,"},{"line_number":55,"context_line":"            fdb_entries)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def filter_hosts_with_segment_access("},{"line_number":58,"context_line":"            self, context, segments, candidate_hosts, agent_getter):"},{"line_number":59,"context_line":"        # NOTE(cbrandily): let other mechanisms (openvswitch, linuxbridge, ...)"},{"line_number":60,"context_line":"        # perform the filtering"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_72079381","line":57,"range":{"start_line":57,"start_character":8,"end_line":57,"end_character":40},"in_reply_to":"fa69d971_1241770b","updated":"2016-01-08 23:15:30.000000000","message":"I know what tripped me up here.  At first, I thought that it was good that this implemented this here because otherwise it would skip filtering.  Then, I got distracted, came back and remembered my train of thought incorrectly.  Review fail!","commit_id":"5985917479a7db8d813ee13e6e1ccd1d63bfc90a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"c3505dc8528079a9058f5c5dfbff33c550c82a44","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        self.L2populationAgentNotify.remove_fdb_entries(self.rpc_ctx,"},{"line_number":55,"context_line":"            fdb_entries)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def filter_hosts_with_segment_access("},{"line_number":58,"context_line":"            self, context, segments, candidate_hosts, agent_getter):"},{"line_number":59,"context_line":"        # NOTE(cbrandily): let other mechanisms (openvswitch, linuxbridge, ...)"},{"line_number":60,"context_line":"        # perform the filtering"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_1241770b","line":57,"range":{"start_line":57,"start_character":8,"end_line":57,"end_character":40},"in_reply_to":"fa69d971_37010d6b","updated":"2016-01-08 23:09:25.000000000","message":"Yep, you\u0027re right.  I was not thinking about this the right way.","commit_id":"5985917479a7db8d813ee13e6e1ccd1d63bfc90a"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"64768936989c344e00876fd6cfa8296565312c96","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        self.L2populationAgentNotify.remove_fdb_entries(self.rpc_ctx,"},{"line_number":55,"context_line":"            fdb_entries)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def filter_hosts_with_segment_access("},{"line_number":58,"context_line":"            self, context, segments, candidate_hosts, agent_getter):"},{"line_number":59,"context_line":"        # NOTE(cbrandily): let other mechanisms (openvswitch, linuxbridge, ...)"},{"line_number":60,"context_line":"        # perform the filtering"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_37010d6b","line":57,"range":{"start_line":57,"start_character":8,"end_line":57,"end_character":40},"in_reply_to":"fa69d971_d768b1bc","updated":"2016-01-08 23:02:28.000000000","message":"Filtering is not skipped because this class overloads default implementation. \n\nl2pop returns an empty set because l2pop \"only\" optimizes the control plane provided by other mechanisms (OVS, LB, etc.) so these other mechanisms provide candidates not l2pop.","commit_id":"5985917479a7db8d813ee13e6e1ccd1d63bfc90a"}],"neutron/plugins/ml2/drivers/mech_agent.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8389d4a5400df5a9435e8189197b3fd9ca67f942","unresolved":false,"context_lines":[{"line_number":168,"context_line":"            if any(self.check_segment_for_agent(s, agent) for s in segments):"},{"line_number":169,"context_line":"                hosts.add(agent[\u0027host\u0027])"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"        return candidate_hosts \u0026 hosts"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    def check_segment_for_agent(self, segment, agent):"},{"line_number":174,"context_line":"        \"\"\"Check if segment can be bound for agent."}],"source_content_type":"text/x-python","patch_set":23,"id":"5a710552_2ff118ce","line":171,"updated":"2015-12-14 23:56:26.000000000","message":"Didn\u0027t L166 already filter by candidate_hosts making this intersection unnecessary?","commit_id":"ba0d3ef15ec022fad27cc2839e8a3de2eaa59b1c"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"4529c5be5c54b7a240bedbfecaae4a8a999ffa17","unresolved":false,"context_lines":[{"line_number":168,"context_line":"            if any(self.check_segment_for_agent(s, agent) for s in segments):"},{"line_number":169,"context_line":"                hosts.add(agent[\u0027host\u0027])"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"        return candidate_hosts \u0026 hosts"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    def check_segment_for_agent(self, segment, agent):"},{"line_number":174,"context_line":"        \"\"\"Check if segment can be bound for agent."}],"source_content_type":"text/x-python","patch_set":23,"id":"5a710552_938276ff","line":171,"in_reply_to":"5a710552_2ff118ce","updated":"2015-12-15 22:08:21.000000000","message":"Will remove it","commit_id":"ba0d3ef15ec022fad27cc2839e8a3de2eaa59b1c"}],"neutron/plugins/ml2/managers.py":[{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"ff26e10c2e2b16b16490e6bf2f732a0d96cfb4f8","unresolved":false,"context_lines":[{"line_number":716,"context_line":""},{"line_number":717,"context_line":"    def check_segment_for_agent(self, segment, agent):"},{"line_number":718,"context_line":"        agent_drivers \u003d [driver.obj for driver in self.ordered_mech_drivers if"},{"line_number":719,"context_line":"                         getattr(driver.obj, \u0027check_segment_for_agent\u0027, False)]"},{"line_number":720,"context_line":"        if not agent_drivers:"},{"line_number":721,"context_line":"            return True"},{"line_number":722,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_61093e31","line":719,"updated":"2015-07-24 18:11:33.000000000","message":"its\u0027 confusing to use getattr(..., False) to check check_segment_for_agent existence (prefer hasattr) as someone could assume driver.obj.check_segment_for_agent is undefined or a boolean","commit_id":"0b3b3fe32a823001eb6a1769c7b993512fbc462e"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d2d9093e524c757d2de96d5797c7ab7cf93a3fee","unresolved":false,"context_lines":[{"line_number":716,"context_line":""},{"line_number":717,"context_line":"    def check_segment_for_agent(self, segment, agent):"},{"line_number":718,"context_line":"        agent_drivers \u003d [driver.obj for driver in self.ordered_mech_drivers if"},{"line_number":719,"context_line":"                         getattr(driver.obj, \u0027check_segment_for_agent\u0027, False)]"},{"line_number":720,"context_line":"        if not agent_drivers:"},{"line_number":721,"context_line":"            return True"},{"line_number":722,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_af116cb1","line":719,"in_reply_to":"3a50d1a3_61093e31","updated":"2015-07-24 19:52:55.000000000","message":"Done","commit_id":"0b3b3fe32a823001eb6a1769c7b993512fbc462e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"09195a2f6bb823deb1876b3c2cdfffa510f504c7","unresolved":false,"context_lines":[{"line_number":714,"context_line":"                return False"},{"line_number":715,"context_line":"        return True"},{"line_number":716,"context_line":""},{"line_number":717,"context_line":"    def check_segment_for_agent(self, segment, agent):"},{"line_number":718,"context_line":"        agent_drivers \u003d [driver.obj for driver in self.ordered_mech_drivers if"},{"line_number":719,"context_line":"                         hasattr(driver.obj, \u0027check_segment_for_agent\u0027)]"},{"line_number":720,"context_line":"        if not agent_drivers:"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_40a14c45","line":717,"updated":"2015-08-11 16:51:47.000000000","message":"The method names seems a little bit lacking but I\u0027m not sure what to suggest since it seems to wrapper around check_segment_for_agent on the mech drivers.\n\nA docstring might be helpful, I\u0027d like it to at least state why True is returned in case no driver supports \u0027check_segment_for_agent\u0027.","commit_id":"580af440dd6b7a285d969318953672b12406da08"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a3a72b2036f864273129b4b885f00363f7ef33f3","unresolved":false,"context_lines":[{"line_number":714,"context_line":"                return False"},{"line_number":715,"context_line":"        return True"},{"line_number":716,"context_line":""},{"line_number":717,"context_line":"    def check_segment_for_agent(self, segment, agent):"},{"line_number":718,"context_line":"        agent_drivers \u003d [driver.obj for driver in self.ordered_mech_drivers if"},{"line_number":719,"context_line":"                         hasattr(driver.obj, \u0027check_segment_for_agent\u0027)]"},{"line_number":720,"context_line":"        if not agent_drivers:"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa1b9901_2d9f90b5","line":717,"in_reply_to":"1a4dcd0f_40a14c45","updated":"2015-08-19 00:24:06.000000000","message":"This method actually went away because of the comment below.","commit_id":"580af440dd6b7a285d969318953672b12406da08"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"09195a2f6bb823deb1876b3c2cdfffa510f504c7","unresolved":false,"context_lines":[{"line_number":721,"context_line":"            return True"},{"line_number":722,"context_line":""},{"line_number":723,"context_line":"        return any("},{"line_number":724,"context_line":"            driver.check_segment_for_agent(segment, agent)"},{"line_number":725,"context_line":"            for driver in agent_drivers)"},{"line_number":726,"context_line":""},{"line_number":727,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_037909c8","line":724,"updated":"2015-08-11 16:51:47.000000000","message":"Do we need to check them all?  Should we be able to figure out which mech driver to check based on the segment type?  I might be showing my ignorance of how ML2 works here.  So, forgive me if I\u0027m just misunderstanding.","commit_id":"580af440dd6b7a285d969318953672b12406da08"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a3a72b2036f864273129b4b885f00363f7ef33f3","unresolved":false,"context_lines":[{"line_number":721,"context_line":"            return True"},{"line_number":722,"context_line":""},{"line_number":723,"context_line":"        return any("},{"line_number":724,"context_line":"            driver.check_segment_for_agent(segment, agent)"},{"line_number":725,"context_line":"            for driver in agent_drivers)"},{"line_number":726,"context_line":""},{"line_number":727,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"fa1b9901_3ad7c824","line":724,"in_reply_to":"1a4dcd0f_037909c8","updated":"2015-08-19 00:24:06.000000000","message":"I didn\u0027t think this was possible but you got me thinking, so the correct thing to do here is to get the correct ML2 driver according to agent[\u0027agent_type\u0027].","commit_id":"580af440dd6b7a285d969318953672b12406da08"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"d1ea86dd8db7acfcb0e2997673607f316a2f3f73","unresolved":false,"context_lines":[{"line_number":769,"context_line":"        result \u003d {}"},{"line_number":770,"context_line":"        for agent in agents:"},{"line_number":771,"context_line":"            result[agent[\u0027agent_type\u0027]] \u003d next(("},{"line_number":772,"context_line":"                driver.obj for driver in self.ordered_mech_drivers if"},{"line_number":773,"context_line":"                hasattr(driver.obj, \u0027agent_type\u0027) and"},{"line_number":774,"context_line":"                driver.obj.agent_type \u003d\u003d agent[\u0027agent_type\u0027]), None)"},{"line_number":775,"context_line":"        return result"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a2fa921_eae8ef18","line":772,"updated":"2015-10-05 15:17:20.000000000","message":"really difficult to read compared to a for loop","commit_id":"976aae4d4d3c505508be502dee463dcd9af96f67"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"ff7942eb37e4e8a7fd007b0fc9b2293db2f29888","unresolved":false,"context_lines":[{"line_number":763,"context_line":"            workers +\u003d driver.obj.get_workers()"},{"line_number":764,"context_line":"        return workers"},{"line_number":765,"context_line":""},{"line_number":766,"context_line":"    def get_agent_to_mech_driver(self, agent):"},{"line_number":767,"context_line":"        \"\"\"Return agent\u0027s ML2 mech driver.\"\"\""},{"line_number":768,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":769,"context_line":"            agent_type \u003d getattr(driver.obj, \u0027agent_type\u0027, None)"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a29b11f_ff1296b3","line":766,"range":{"start_line":766,"start_character":8,"end_line":766,"end_character":32},"updated":"2015-10-20 11:00:05.000000000","message":"i\u0027d suggest \"get_mech_driver_for_agent\"","commit_id":"e9beefb657994c928e7ff71c365ec91860756375"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"039b528daada6cc83fb77ef5ab30fb6c412ba2f9","unresolved":false,"context_lines":[{"line_number":763,"context_line":"            workers +\u003d driver.obj.get_workers()"},{"line_number":764,"context_line":"        return workers"},{"line_number":765,"context_line":""},{"line_number":766,"context_line":"    def get_agent_to_mech_driver(self, agent):"},{"line_number":767,"context_line":"        \"\"\"Return agent\u0027s ML2 mech driver.\"\"\""},{"line_number":768,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":769,"context_line":"            agent_type \u003d getattr(driver.obj, \u0027agent_type\u0027, None)"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a29b11f_2726eb16","line":766,"in_reply_to":"3a29b11f_ff1296b3","updated":"2015-10-26 22:37:59.000000000","message":"Done","commit_id":"e9beefb657994c928e7ff71c365ec91860756375"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"ff7942eb37e4e8a7fd007b0fc9b2293db2f29888","unresolved":false,"context_lines":[{"line_number":766,"context_line":"    def get_agent_to_mech_driver(self, agent):"},{"line_number":767,"context_line":"        \"\"\"Return agent\u0027s ML2 mech driver.\"\"\""},{"line_number":768,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":769,"context_line":"            agent_type \u003d getattr(driver.obj, \u0027agent_type\u0027, None)"},{"line_number":770,"context_line":"            if None !\u003d agent_type \u003d\u003d agent[\u0027agent_type\u0027]:"},{"line_number":771,"context_line":"                return driver.obj"},{"line_number":772,"context_line":"        return None"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a29b11f_7f73e680","line":769,"updated":"2015-10-20 11:00:05.000000000","message":"better to avoid using getattr.","commit_id":"e9beefb657994c928e7ff71c365ec91860756375"},{"author":{"_account_id":9459,"name":".Haifeng Yan","email":"yanheven@qq.com","username":"yan"},"change_message_id":"a12ac28581a72324678c471b81d0e7d0953fc766","unresolved":false,"context_lines":[{"line_number":766,"context_line":"    def get_mech_driver_for_agent(self, agent):"},{"line_number":767,"context_line":"        \"\"\"Return agent\u0027s ML2 mech driver.\"\"\""},{"line_number":768,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":769,"context_line":"            if None !\u003d driver.obj.agent_type \u003d\u003d agent[\u0027agent_type\u0027]:"},{"line_number":770,"context_line":"                return driver.obj"},{"line_number":771,"context_line":"        return None"},{"line_number":772,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_b0f25105","line":769,"updated":"2015-11-07 08:02:23.000000000","message":"do you mean dirver.obj.agent_type is not None and driver.obj.agent_type \u003d\u003d agent[\u0027agent_type\u0027] ?","commit_id":"0395af8db7932f9d7dc86adde5a0dbeca4a3b4c9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a2a01d0a1067950b33a67b8872e13963b00409a9","unresolved":false,"context_lines":[{"line_number":766,"context_line":"    def get_mech_driver_for_agent(self, agent):"},{"line_number":767,"context_line":"        \"\"\"Return agent\u0027s ML2 mech driver.\"\"\""},{"line_number":768,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":769,"context_line":"            if None !\u003d driver.obj.agent_type \u003d\u003d agent[\u0027agent_type\u0027]:"},{"line_number":770,"context_line":"                return driver.obj"},{"line_number":771,"context_line":"        return None"},{"line_number":772,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_f011b9b0","line":769,"in_reply_to":"da85f559_505205df","updated":"2015-11-07 08:59:14.000000000","message":"Could the driver.obj.agent_type be None?","commit_id":"0395af8db7932f9d7dc86adde5a0dbeca4a3b4c9"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"3f56d551d44295e80d55cd1563042a2e86a5d53a","unresolved":false,"context_lines":[{"line_number":766,"context_line":"    def get_mech_driver_for_agent(self, agent):"},{"line_number":767,"context_line":"        \"\"\"Return agent\u0027s ML2 mech driver.\"\"\""},{"line_number":768,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":769,"context_line":"            if None !\u003d driver.obj.agent_type \u003d\u003d agent[\u0027agent_type\u0027]:"},{"line_number":770,"context_line":"                return driver.obj"},{"line_number":771,"context_line":"        return None"},{"line_number":772,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_505205df","line":769,"in_reply_to":"da85f559_b0f25105","updated":"2015-11-07 08:06:36.000000000","message":"Yes, that\u0027s the meaning\n\nPlz, don\u0027t -1 when you ask questions","commit_id":"0395af8db7932f9d7dc86adde5a0dbeca4a3b4c9"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"7e2b9e960215ab5abdc70cf68069f6be431c6c00","unresolved":false,"context_lines":[{"line_number":766,"context_line":"    def get_mech_driver_for_agent(self, agent):"},{"line_number":767,"context_line":"        \"\"\"Return agent\u0027s ML2 mech driver.\"\"\""},{"line_number":768,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":769,"context_line":"            if None !\u003d driver.obj.agent_type \u003d\u003d agent[\u0027agent_type\u0027]:"},{"line_number":770,"context_line":"                return driver.obj"},{"line_number":771,"context_line":"        return None"},{"line_number":772,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_77ba5b25","line":769,"in_reply_to":"da85f559_f011b9b0","updated":"2015-11-09 22:07:22.000000000","message":"Yes, that\u0027s the default value defined in driver_api","commit_id":"0395af8db7932f9d7dc86adde5a0dbeca4a3b4c9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a2a01d0a1067950b33a67b8872e13963b00409a9","unresolved":false,"context_lines":[{"line_number":768,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":769,"context_line":"            if None !\u003d driver.obj.agent_type \u003d\u003d agent[\u0027agent_type\u0027]:"},{"line_number":770,"context_line":"                return driver.obj"},{"line_number":771,"context_line":"        return None"},{"line_number":772,"context_line":""},{"line_number":773,"context_line":""},{"line_number":774,"context_line":"class ExtensionManager(stevedore.named.NamedExtensionManager):"}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_703f0926","line":771,"updated":"2015-11-07 08:59:14.000000000","message":"you do not need this. by default the None is returned","commit_id":"0395af8db7932f9d7dc86adde5a0dbeca4a3b4c9"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"bb331518375ceb68e14a5e76e97edfcad4cad6c0","unresolved":false,"context_lines":[{"line_number":768,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":769,"context_line":"            if None !\u003d driver.obj.agent_type \u003d\u003d agent[\u0027agent_type\u0027]:"},{"line_number":770,"context_line":"                return driver.obj"},{"line_number":771,"context_line":"        return None"},{"line_number":772,"context_line":""},{"line_number":773,"context_line":""},{"line_number":774,"context_line":"class ExtensionManager(stevedore.named.NamedExtensionManager):"}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_dcd7acbd","line":771,"in_reply_to":"da85f559_703f0926","updated":"2015-11-09 21:57:10.000000000","message":"I disable an explicit return None clarifies that the fallback case is handled not forgotten","commit_id":"0395af8db7932f9d7dc86adde5a0dbeca4a3b4c9"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"10ec38754885a632b0cf0fb9935be0b685af68b1","unresolved":false,"context_lines":[{"line_number":766,"context_line":"    def get_mech_driver_for_agent(self, agent):"},{"line_number":767,"context_line":"        \"\"\"Return agent\u0027s ML2 mech driver.\"\"\""},{"line_number":768,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":769,"context_line":"            if None !\u003d driver.obj.agent_type \u003d\u003d agent[\u0027agent_type\u0027]:"},{"line_number":770,"context_line":"                return driver.obj"},{"line_number":771,"context_line":"        return None"},{"line_number":772,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"ba8a016a_0bb1528f","line":769,"updated":"2015-12-03 15:13:02.000000000","message":"i find this line barely readable :)","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"746bbf7621ba97faa16882610b048da524c63d8c","unresolved":false,"context_lines":[{"line_number":766,"context_line":"    def get_mech_driver_for_agent(self, agent):"},{"line_number":767,"context_line":"        \"\"\"Return agent\u0027s ML2 mech driver.\"\"\""},{"line_number":768,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":769,"context_line":"            if None !\u003d driver.obj.agent_type \u003d\u003d agent[\u0027agent_type\u0027]:"},{"line_number":770,"context_line":"                return driver.obj"},{"line_number":771,"context_line":"        return None"},{"line_number":772,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"7a740942_47f3cd0a","line":769,"in_reply_to":"7a740942_b2dd5f72","updated":"2015-12-11 00:07:34.000000000","message":"Nope, python allows to chain comparisons[1] to shorten code:\n\n if a \u003c\u003d b \u003c\u003d c:\n\nis equivalent to:\n\n if a \u003c\u003d b and b \u003c\u003d c:\n\nso this condition is equivalent to:\n\n if driver.obj.agent_type !\u003d None and driver.obj.agent_type \u003d\u003d agent[\u0027agent_type\u0027]:\n\nExample:\n\n \u003e\u003e\u003e True !\u003d 2 \u003d\u003d 2\n True\n \u003e\u003e\u003e True !\u003d (2 \u003d\u003d 2)\n False\n \u003e\u003e\u003e (True !\u003d 2) \u003d\u003d 2\n False\n\nOK, it\u0027s not really readable with \u003d\u003d\n\n[1] https://docs.python.org/2/reference/expressions.html#grammar-token-comparison","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"852083df6c7d8d4dcb8865a3d2674dc5330e2d3c","unresolved":false,"context_lines":[{"line_number":766,"context_line":"    def get_mech_driver_for_agent(self, agent):"},{"line_number":767,"context_line":"        \"\"\"Return agent\u0027s ML2 mech driver.\"\"\""},{"line_number":768,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":769,"context_line":"            if None !\u003d driver.obj.agent_type \u003d\u003d agent[\u0027agent_type\u0027]:"},{"line_number":770,"context_line":"                return driver.obj"},{"line_number":771,"context_line":"        return None"},{"line_number":772,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"7a740942_b2dd5f72","line":769,"in_reply_to":"ba8a016a_0bb1528f","updated":"2015-12-04 00:22:16.000000000","message":"Yikes, what does this even mean?  I find it completely unreadable.  My thought is that it should be equivalent to this:\n\n  if (None !\u003d driver.obj.agent_type) \u003d\u003d agent[\u0027agent_type\u0027]\n\nSince the !\u003d will return a boolean how could I ever expect this to \u003d\u003d agent[\u0027agent_type\u0027]?\n\nIf my assessment is incorrect then you managed to find a construct that I couldn\u0027t understand.  Not an easy thing to do.","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"852083df6c7d8d4dcb8865a3d2674dc5330e2d3c","unresolved":false,"context_lines":[{"line_number":768,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":769,"context_line":"            if None !\u003d driver.obj.agent_type \u003d\u003d agent[\u0027agent_type\u0027]:"},{"line_number":770,"context_line":"                return driver.obj"},{"line_number":771,"context_line":"        return None"},{"line_number":772,"context_line":""},{"line_number":773,"context_line":""},{"line_number":774,"context_line":"class ExtensionManager(stevedore.named.NamedExtensionManager):"}],"source_content_type":"text/x-python","patch_set":19,"id":"7a740942_523c3b93","line":771,"updated":"2015-12-04 00:22:16.000000000","message":"nit:  redundant because return None is implied.","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"746bbf7621ba97faa16882610b048da524c63d8c","unresolved":false,"context_lines":[{"line_number":768,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":769,"context_line":"            if None !\u003d driver.obj.agent_type \u003d\u003d agent[\u0027agent_type\u0027]:"},{"line_number":770,"context_line":"                return driver.obj"},{"line_number":771,"context_line":"        return None"},{"line_number":772,"context_line":""},{"line_number":773,"context_line":""},{"line_number":774,"context_line":"class ExtensionManager(stevedore.named.NamedExtensionManager):"}],"source_content_type":"text/x-python","patch_set":19,"id":"7a740942_2747a11e","line":771,"in_reply_to":"7a740942_523c3b93","updated":"2015-12-11 00:07:34.000000000","message":"I disagree: an explicit \"return None\" highlights that the fallback case is not forgotten.","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8389d4a5400df5a9435e8189197b3fd9ca67f942","unresolved":false,"context_lines":[{"line_number":748,"context_line":""},{"line_number":749,"context_line":"        :returns: a subset of candidate_hosts."},{"line_number":750,"context_line":""},{"line_number":751,"context_line":"        If at least one driver implements segment based filtering then this"},{"line_number":752,"context_line":"        method returns all hosts from candidate_hosts with access to a"},{"line_number":753,"context_line":"        segment according to at least one driver."},{"line_number":754,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"5a710552_c4107251","line":751,"updated":"2015-12-14 23:56:26.000000000","message":"I\u0027m not sure this makes sense to me.  IIUC, what you\u0027re saying is:\n\nWhat is the reasoning behind this?  Maybe there is something that I\u0027m missing in understanding mechanism drivers but I\u0027m suspecting that we may filter too aggressively if only a partial set of drivers implements filtering.","commit_id":"ba0d3ef15ec022fad27cc2839e8a3de2eaa59b1c"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"62ebb087bc59d646ae4f5bea190bae28a6ccbf52","unresolved":false,"context_lines":[{"line_number":748,"context_line":""},{"line_number":749,"context_line":"        :returns: a subset of candidate_hosts."},{"line_number":750,"context_line":""},{"line_number":751,"context_line":"        If at least one driver implements segment based filtering then this"},{"line_number":752,"context_line":"        method returns all hosts from candidate_hosts with access to a"},{"line_number":753,"context_line":"        segment according to at least one driver."},{"line_number":754,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"5a710552_56271c12","line":751,"in_reply_to":"5a710552_936d9606","updated":"2015-12-15 22:14:30.000000000","message":"After some thinking, for consistency with agents_db default implementation, we should perhaps disable the filtering when at least a mechanism doesn\u0027t support filtering ... it implies to define the filtering for every mechanism in neutron","commit_id":"ba0d3ef15ec022fad27cc2839e8a3de2eaa59b1c"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"4529c5be5c54b7a240bedbfecaae4a8a999ffa17","unresolved":false,"context_lines":[{"line_number":748,"context_line":""},{"line_number":749,"context_line":"        :returns: a subset of candidate_hosts."},{"line_number":750,"context_line":""},{"line_number":751,"context_line":"        If at least one driver implements segment based filtering then this"},{"line_number":752,"context_line":"        method returns all hosts from candidate_hosts with access to a"},{"line_number":753,"context_line":"        segment according to at least one driver."},{"line_number":754,"context_line":""}],"source_content_type":"text/x-python","patch_set":23,"id":"5a710552_936d9606","line":751,"in_reply_to":"5a710552_c4107251","updated":"2015-12-15 22:08:21.000000000","message":"We can return all candidates if at least a mechanism doesn\u0027t support filtering and crash during port binding because we were too optimist (backward compatible but crashs).\n\nOr we can filter using only mechanisms supporting filtering and find no candidate because we were too pessimist (doesn\u0027t crash).","commit_id":"ba0d3ef15ec022fad27cc2839e8a3de2eaa59b1c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"8389d4a5400df5a9435e8189197b3fd9ca67f942","unresolved":false,"context_lines":[{"line_number":763,"context_line":"                    context, segments, candidate_hosts, agent_getter)"},{"line_number":764,"context_line":"            except NotImplementedError:"},{"line_number":765,"context_line":"                # Mechanism not implementing the feature"},{"line_number":766,"context_line":"                pass"},{"line_number":767,"context_line":"            else:"},{"line_number":768,"context_line":"                found_filtering_mech \u003d True"},{"line_number":769,"context_line":"                hosts_with_access |\u003d hosts"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a710552_41a0f4f4","line":766,"updated":"2015-12-14 23:56:26.000000000","message":"pico nit:  continue here and forget the \"else:\" below.  Just reduce indentation one level on L768 - 772.","commit_id":"ba0d3ef15ec022fad27cc2839e8a3de2eaa59b1c"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"4529c5be5c54b7a240bedbfecaae4a8a999ffa17","unresolved":false,"context_lines":[{"line_number":763,"context_line":"                    context, segments, candidate_hosts, agent_getter)"},{"line_number":764,"context_line":"            except NotImplementedError:"},{"line_number":765,"context_line":"                # Mechanism not implementing the feature"},{"line_number":766,"context_line":"                pass"},{"line_number":767,"context_line":"            else:"},{"line_number":768,"context_line":"                found_filtering_mech \u003d True"},{"line_number":769,"context_line":"                hosts_with_access |\u003d hosts"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a710552_b39c525e","line":766,"in_reply_to":"5a710552_41a0f4f4","updated":"2015-12-15 22:08:21.000000000","message":"Will do","commit_id":"ba0d3ef15ec022fad27cc2839e8a3de2eaa59b1c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"b4782938df94db982c00d6ef2bbbe42f389dbd3f","unresolved":false,"context_lines":[{"line_number":748,"context_line":""},{"line_number":749,"context_line":"        :returns: a subset of candidate_hosts."},{"line_number":750,"context_line":""},{"line_number":751,"context_line":"        This method returns all hosts from candidate_hosts with access to a"},{"line_number":752,"context_line":"        segment according to at least one driver."},{"line_number":753,"context_line":"        \"\"\""},{"line_number":754,"context_line":"        candidate_hosts \u003d set(candidate_hosts)"},{"line_number":755,"context_line":"        hosts_with_access \u003d set()"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a7e1126_28d0df1e","line":752,"range":{"start_line":751,"start_character":8,"end_line":752,"end_character":49},"updated":"2015-12-17 22:14:47.000000000","message":"I guess I like this a little bit better.  But, you\u0027re right that you need all of the drivers in use to support it to get anything out of it.\n\nHowever, I really didn\u0027t like that it could get too aggressive in the previous patch set.","commit_id":"3a1de75ac3ebb99a8ba1b99cb4d582302654cfd9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"b4782938df94db982c00d6ef2bbbe42f389dbd3f","unresolved":false,"context_lines":[{"line_number":754,"context_line":"        candidate_hosts \u003d set(candidate_hosts)"},{"line_number":755,"context_line":"        hosts_with_access \u003d set()"},{"line_number":756,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":757,"context_line":"            hosts \u003d driver.obj.filter_hosts_with_segment_access("},{"line_number":758,"context_line":"                context, segments, candidate_hosts, agent_getter)"},{"line_number":759,"context_line":"            hosts_with_access |\u003d hosts"},{"line_number":760,"context_line":"            candidate_hosts -\u003d hosts"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a7e1126_ab4e096d","line":757,"updated":"2015-12-17 22:14:47.000000000","message":"It would be good to ask all of the drivers if they support this operation up front.  If even one doesn\u0027t, then why bother asking any of them to filter?","commit_id":"3a1de75ac3ebb99a8ba1b99cb4d582302654cfd9"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"0e49fee9bd836063d50bfdb0bd60a847190123e0","unresolved":false,"context_lines":[{"line_number":754,"context_line":"        candidate_hosts \u003d set(candidate_hosts)"},{"line_number":755,"context_line":"        hosts_with_access \u003d set()"},{"line_number":756,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":757,"context_line":"            hosts \u003d driver.obj.filter_hosts_with_segment_access("},{"line_number":758,"context_line":"                context, segments, candidate_hosts, agent_getter)"},{"line_number":759,"context_line":"            hosts_with_access |\u003d hosts"},{"line_number":760,"context_line":"            candidate_hosts -\u003d hosts"}],"source_content_type":"text/x-python","patch_set":24,"id":"1a7b0d38_8674b2fd","line":757,"in_reply_to":"1a7b0d38_c0794231","updated":"2015-12-22 22:15:30.000000000","message":"Can we move the method that would not filter anything into a base class?","commit_id":"3a1de75ac3ebb99a8ba1b99cb4d582302654cfd9"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"f3a7710eb755d50fe4eb8bd7edb43c9a86c49bcb","unresolved":false,"context_lines":[{"line_number":754,"context_line":"        candidate_hosts \u003d set(candidate_hosts)"},{"line_number":755,"context_line":"        hosts_with_access \u003d set()"},{"line_number":756,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":757,"context_line":"            hosts \u003d driver.obj.filter_hosts_with_segment_access("},{"line_number":758,"context_line":"                context, segments, candidate_hosts, agent_getter)"},{"line_number":759,"context_line":"            hosts_with_access |\u003d hosts"},{"line_number":760,"context_line":"            candidate_hosts -\u003d hosts"}],"source_content_type":"text/x-python","patch_set":24,"id":"1a7b0d38_b76556e2","line":757,"in_reply_to":"1a7b0d38_c0794231","updated":"2015-12-23 00:13:01.000000000","message":"Done","commit_id":"3a1de75ac3ebb99a8ba1b99cb4d582302654cfd9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"80d8c337c782b1b5cb12e9324b48f2706656c568","unresolved":false,"context_lines":[{"line_number":754,"context_line":"        candidate_hosts \u003d set(candidate_hosts)"},{"line_number":755,"context_line":"        hosts_with_access \u003d set()"},{"line_number":756,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":757,"context_line":"            hosts \u003d driver.obj.filter_hosts_with_segment_access("},{"line_number":758,"context_line":"                context, segments, candidate_hosts, agent_getter)"},{"line_number":759,"context_line":"            hosts_with_access |\u003d hosts"},{"line_number":760,"context_line":"            candidate_hosts -\u003d hosts"}],"source_content_type":"text/x-python","patch_set":24,"id":"1a7b0d38_c0794231","line":757,"in_reply_to":"3a7e1126_31d9db22","updated":"2015-12-22 21:06:57.000000000","message":"We could provide another new call to the interface that simply answers whether it supports it.  The base implementation would return False.  Essentially, it would formalize what you suggested doing with hasattr. \n\nIf we go with the exception then we could end up having some mech drivers do the filtering and then throw out their filtering when we hit one that doesn\u0027t implement it at all.","commit_id":"3a1de75ac3ebb99a8ba1b99cb4d582302654cfd9"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"60a85a674457f0c9756081cf4fe7d17f2ddf639e","unresolved":false,"context_lines":[{"line_number":754,"context_line":"        candidate_hosts \u003d set(candidate_hosts)"},{"line_number":755,"context_line":"        hosts_with_access \u003d set()"},{"line_number":756,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":757,"context_line":"            hosts \u003d driver.obj.filter_hosts_with_segment_access("},{"line_number":758,"context_line":"                context, segments, candidate_hosts, agent_getter)"},{"line_number":759,"context_line":"            hosts_with_access |\u003d hosts"},{"line_number":760,"context_line":"            candidate_hosts -\u003d hosts"}],"source_content_type":"text/x-python","patch_set":24,"id":"3a7e1126_31d9db22","line":757,"in_reply_to":"3a7e1126_ab4e096d","updated":"2015-12-20 13:17:03.000000000","message":"How do we ask them if they support filtering? \n\nWe could use hasattr(driver, \u0027filter_hosts_with_segment_access\u0027) but it was requested to avoid hasattr and use inheritence/overloading (which defines a clear interface).\n\nWe could learn if a driver doesn\u0027t support filtering (by raising NotImplementedError for example) during first call","commit_id":"3a1de75ac3ebb99a8ba1b99cb4d582302654cfd9"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"1ef7eaa2d4ddc5dad11961e5c120641c48c45ecc","unresolved":false,"context_lines":[{"line_number":766,"context_line":"        This method returns all hosts from candidate_hosts with access to a"},{"line_number":767,"context_line":"        segment according to at least one driver."},{"line_number":768,"context_line":"        \"\"\""},{"line_number":769,"context_line":"        if not self.host_filtering_supported:"},{"line_number":770,"context_line":"            return candidate_hosts"},{"line_number":771,"context_line":""},{"line_number":772,"context_line":"        candidate_hosts \u003d set(candidate_hosts)"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_02a1a5f7","line":769,"updated":"2016-01-07 08:41:22.000000000","message":"Is this necessary? The default methods that all of the drivers inherit just return the passed in canidates so it will just be a few extra set operations.","commit_id":"5985917479a7db8d813ee13e6e1ccd1d63bfc90a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"9483707fcec4ff8a9030980a71c5e98ae0568321","unresolved":false,"context_lines":[{"line_number":766,"context_line":"        This method returns all hosts from candidate_hosts with access to a"},{"line_number":767,"context_line":"        segment according to at least one driver."},{"line_number":768,"context_line":"        \"\"\""},{"line_number":769,"context_line":"        if not self.host_filtering_supported:"},{"line_number":770,"context_line":"            return candidate_hosts"},{"line_number":771,"context_line":""},{"line_number":772,"context_line":"        candidate_hosts \u003d set(candidate_hosts)"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_9976f23c","line":769,"in_reply_to":"fa69d971_02a1a5f7","updated":"2016-01-08 22:41:37.000000000","message":"Kevin, if there is a mix of drivers -- some with the default and some that implement filtering -- then this is a short-cut to avoid calling filter_hosts for any of the drivers since it will just be a wasted effort.  With what I know about the operation, it could involve a largish db query and iteration over the results in its current form.\n\nI don\u0027t think the set operations were ever a concern.\n\nTo give a little more context, I was thinking ahead to the case where candidate_hosts might be a list of compute hosts instead of network hosts.  I imagine this would be a much larger canditate_hosts list and it concerned me enough to justify a few extra lines of code.  What do you think?  Even without this, I think it is a decent optimization.\n\nI do realize that in the current Neutron, this list will likely be a reletively small list of network nodes.","commit_id":"5985917479a7db8d813ee13e6e1ccd1d63bfc90a"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"64768936989c344e00876fd6cfa8296565312c96","unresolved":false,"context_lines":[{"line_number":766,"context_line":"        This method returns all hosts from candidate_hosts with access to a"},{"line_number":767,"context_line":"        segment according to at least one driver."},{"line_number":768,"context_line":"        \"\"\""},{"line_number":769,"context_line":"        if not self.host_filtering_supported:"},{"line_number":770,"context_line":"            return candidate_hosts"},{"line_number":771,"context_line":""},{"line_number":772,"context_line":"        candidate_hosts \u003d set(candidate_hosts)"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_37702ddc","line":769,"in_reply_to":"fa69d971_9976f23c","updated":"2016-01-08 23:02:28.000000000","message":"Calling OVS/LB mechanism filter_hosts_with_segment_access can be costly when candidate_hosts become large, on deployments with OVS/LB mechanism + a mechanism not supporting filtering, this check avoids useless and costly call to OVS/LB mechanisms.\n\nA follow-up work is to optimize OVS/LB filtering but sadly i didn\u0027t have lot of time to work on it recently","commit_id":"5985917479a7db8d813ee13e6e1ccd1d63bfc90a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"9483707fcec4ff8a9030980a71c5e98ae0568321","unresolved":false,"context_lines":[{"line_number":767,"context_line":"        segment according to at least one driver."},{"line_number":768,"context_line":"        \"\"\""},{"line_number":769,"context_line":"        if not self.host_filtering_supported:"},{"line_number":770,"context_line":"            return candidate_hosts"},{"line_number":771,"context_line":""},{"line_number":772,"context_line":"        candidate_hosts \u003d set(candidate_hosts)"},{"line_number":773,"context_line":"        hosts_with_access \u003d set()"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_b97c16b6","line":770,"range":{"start_line":770,"start_character":19,"end_line":770,"end_character":34},"updated":"2016-01-08 22:41:37.000000000","message":"Should this return a set to be consistent with the type returned on L781?","commit_id":"5985917479a7db8d813ee13e6e1ccd1d63bfc90a"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"64768936989c344e00876fd6cfa8296565312c96","unresolved":false,"context_lines":[{"line_number":767,"context_line":"        segment according to at least one driver."},{"line_number":768,"context_line":"        \"\"\""},{"line_number":769,"context_line":"        if not self.host_filtering_supported:"},{"line_number":770,"context_line":"            return candidate_hosts"},{"line_number":771,"context_line":""},{"line_number":772,"context_line":"        candidate_hosts \u003d set(candidate_hosts)"},{"line_number":773,"context_line":"        hosts_with_access \u003d set()"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_772ea5fb","line":770,"range":{"start_line":770,"start_character":19,"end_line":770,"end_character":34},"in_reply_to":"fa69d971_b97c16b6","updated":"2016-01-08 23:02:28.000000000","message":"Done","commit_id":"5985917479a7db8d813ee13e6e1ccd1d63bfc90a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"9483707fcec4ff8a9030980a71c5e98ae0568321","unresolved":false,"context_lines":[{"line_number":769,"context_line":"        if not self.host_filtering_supported:"},{"line_number":770,"context_line":"            return candidate_hosts"},{"line_number":771,"context_line":""},{"line_number":772,"context_line":"        candidate_hosts \u003d set(candidate_hosts)"},{"line_number":773,"context_line":"        hosts_with_access \u003d set()"},{"line_number":774,"context_line":"        for driver in self.ordered_mech_drivers:"},{"line_number":775,"context_line":"            hosts \u003d driver.obj.filter_hosts_with_segment_access("}],"source_content_type":"text/x-python","patch_set":28,"id":"fa69d971_77d6a528","line":772,"range":{"start_line":772,"start_character":8,"end_line":772,"end_character":46},"updated":"2016-01-08 22:41:37.000000000","message":"Or, just move this above L769","commit_id":"5985917479a7db8d813ee13e6e1ccd1d63bfc90a"}],"neutron/plugins/ml2/plugin.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"09195a2f6bb823deb1876b3c2cdfffa510f504c7","unresolved":false,"context_lines":[{"line_number":1512,"context_line":"        return device"},{"line_number":1513,"context_line":""},{"line_number":1514,"context_line":"    def get_agents_with_access_to_network(self, context, network_id):"},{"line_number":1515,"context_line":"        segments \u003d db.get_network_segments(context.session, network_id)"},{"line_number":1516,"context_line":"        agents \u003d self.get_agents(context)"},{"line_number":1517,"context_line":"        result \u003d []"},{"line_number":1518,"context_line":"        for agent in agents:"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_d77bdfbc","line":1515,"updated":"2015-08-11 16:51:47.000000000","message":"Will this work for networks that don\u0027t use the multi-provider extension?  My hope is that it returns a single network segment in that case but what I remember about the API is that it is sort of an either/or thing.  Either you\u0027re using multi-provider and you have multiple segments or you don\u0027t and you don\u0027t have segments, you just have the network.  But, I\u0027m not as familiar with the ml2 implementation.\n\nThe functional test you added seems to indicate that it works.","commit_id":"580af440dd6b7a285d969318953672b12406da08"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a3a72b2036f864273129b4b885f00363f7ef33f3","unresolved":false,"context_lines":[{"line_number":1512,"context_line":"        return device"},{"line_number":1513,"context_line":""},{"line_number":1514,"context_line":"    def get_agents_with_access_to_network(self, context, network_id):"},{"line_number":1515,"context_line":"        segments \u003d db.get_network_segments(context.session, network_id)"},{"line_number":1516,"context_line":"        agents \u003d self.get_agents(context)"},{"line_number":1517,"context_line":"        result \u003d []"},{"line_number":1518,"context_line":"        for agent in agents:"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa1b9901_4b566cd3","line":1515,"in_reply_to":"1a4dcd0f_d77bdfbc","updated":"2015-08-19 00:24:06.000000000","message":"Yeah it\u0027s either network[\u0027segments\u0027] or (network[\u0027physical_network\u0027], network[\u0027segmentation_id\u0027], network[\u0027network_type\u0027]). This works in both cases as evident by the new ML2 plugin unit tests (I also added one for multi segment in the next PS).\n\nget_network_segments returns a list either case, with one member or more.","commit_id":"580af440dd6b7a285d969318953672b12406da08"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"8c940afdf6a5ffdf64fcc6f236d5a05ecc3ecf50","unresolved":false,"context_lines":[{"line_number":1516,"context_line":"        agents \u003d self.get_agents(context)"},{"line_number":1517,"context_line":"        result \u003d []"},{"line_number":1518,"context_line":"        for agent in agents:"},{"line_number":1519,"context_line":"            if any(self.mechanism_manager.check_segment_for_agent("},{"line_number":1520,"context_line":"                    segment, agent) for segment in segments):"},{"line_number":1521,"context_line":"                result.append(agent)"},{"line_number":1522,"context_line":"        return result"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a50d1a3_d2e8d4e4","line":1519,"updated":"2015-07-27 19:21:23.000000000","message":"Does this look correct in the case of a multi-segment network, where the agent might have access to one segment but not the other? Currently I\u0027m just checking that the agent is configured correctly for *any* segment.","commit_id":"580af440dd6b7a285d969318953672b12406da08"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a3a72b2036f864273129b4b885f00363f7ef33f3","unresolved":false,"context_lines":[{"line_number":1516,"context_line":"        agents \u003d self.get_agents(context)"},{"line_number":1517,"context_line":"        result \u003d []"},{"line_number":1518,"context_line":"        for agent in agents:"},{"line_number":1519,"context_line":"            if any(self.mechanism_manager.check_segment_for_agent("},{"line_number":1520,"context_line":"                    segment, agent) for segment in segments):"},{"line_number":1521,"context_line":"                result.append(agent)"},{"line_number":1522,"context_line":"        return result"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa1b9901_abf5c8cb","line":1519,"in_reply_to":"1a4dcd0f_801e746e","updated":"2015-08-19 00:24:06.000000000","message":"\u003e I\u0027d like to make the assumption that the segments create a continuous L2.\n\nThat is my understanding as well so it should be sufficient to check for reachability to any segment.","commit_id":"580af440dd6b7a285d969318953672b12406da08"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"09195a2f6bb823deb1876b3c2cdfffa510f504c7","unresolved":false,"context_lines":[{"line_number":1516,"context_line":"        agents \u003d self.get_agents(context)"},{"line_number":1517,"context_line":"        result \u003d []"},{"line_number":1518,"context_line":"        for agent in agents:"},{"line_number":1519,"context_line":"            if any(self.mechanism_manager.check_segment_for_agent("},{"line_number":1520,"context_line":"                    segment, agent) for segment in segments):"},{"line_number":1521,"context_line":"                result.append(agent)"},{"line_number":1522,"context_line":"        return result"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_801e746e","line":1519,"in_reply_to":"3a50d1a3_d2e8d4e4","updated":"2015-08-11 16:51:47.000000000","message":"We don\u0027t have any information about reachability between segments iiuc.\n\nI tend to think that segments are assumed to be essentially bridged together behind the scenes so that access to one segment implies access to the others.  If that assumption is incorrect then we\u0027d need to run a DHCP server for every segment, right?  I don\u0027t think we want to go that far, do we?\n\nI\u0027d like to make the assumption that the segments create a continuous L2.","commit_id":"580af440dd6b7a285d969318953672b12406da08"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"cd9faf2fe739e184a1828b5953b190e631c85a0c","unresolved":false,"context_lines":[{"line_number":1530,"context_line":""},{"line_number":1531,"context_line":"        agents \u003d self.get_agents(context)"},{"line_number":1532,"context_line":"        mapping \u003d ("},{"line_number":1533,"context_line":"            self.mechanism_manager.get_agent_to_mech_driver_mapping(agents))"},{"line_number":1534,"context_line":"        segments \u003d db.get_network_segments(context.session, network_id)"},{"line_number":1535,"context_line":"        result \u003d []"},{"line_number":1536,"context_line":"        found \u003d False"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa1b9901_ac1fe96a","line":1533,"updated":"2015-08-19 21:56:28.000000000","message":"get_agent_to_mech_driver_mapping is only used by this method.\n\nget_agent_to_mech_driver_mapping iterates over every agent to add a key/value in the mapping and get_agent_to_mech_driver_mapping iterates over every agent to use previous key/value ... so it seems we could replace get_agent_to_mech_driver_mapping by a get_agent_mech_driver method in order to avoid building mapping\n\n\n  agents \u003d ...\n  segments \u003d ...\n  result \u003d []\n  found \u003d False\n  for agent in agents:\n    mech_driver \u003d self.mechanism_manager.get_agent_mech_driver(agent)\n    if not mech_driver:\n      ...","commit_id":"b8cf9007a9ac9e9526e19e983ea4e07d73495258"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"cd9faf2fe739e184a1828b5953b190e631c85a0c","unresolved":false,"context_lines":[{"line_number":1536,"context_line":"        found \u003d False"},{"line_number":1537,"context_line":"        for agent in agents:"},{"line_number":1538,"context_line":"            for segment in segments:"},{"line_number":1539,"context_line":"                mech_driver \u003d mapping.get(agent[\u0027agent_type\u0027])"},{"line_number":1540,"context_line":"                if not mech_driver:"},{"line_number":1541,"context_line":"                    continue"},{"line_number":1542,"context_line":"                if hasattr(mech_driver, \u0027check_segment_for_agent\u0027):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa1b9901_ac32895d","line":1539,"updated":"2015-08-19 21:56:28.000000000","message":"prefer:\n\n  for agent in agents:\n    mech_driver \u003d mapping.get(agent[\u0027agent_type\u0027])\n    if not mech_driver:\n      continue\n    check \u003d getattr(mech_driver, \u0027check_segment_for_agent\u0027, None)\n    if check:\n      found \u003d True\n      if all(check(s, agent) for s in segments):\n        result.append(agent)","commit_id":"b8cf9007a9ac9e9526e19e983ea4e07d73495258"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"d1ea86dd8db7acfcb0e2997673607f316a2f3f73","unresolved":false,"context_lines":[{"line_number":1587,"context_line":"        segments \u003d db.get_network_segments(context.session, network_id)"},{"line_number":1588,"context_line":"        result \u003d []"},{"line_number":1589,"context_line":"        found \u003d False"},{"line_number":1590,"context_line":"        for agent in agents:"},{"line_number":1591,"context_line":"            for segment in segments:"},{"line_number":1592,"context_line":"                mech_driver \u003d mapping.get(agent[\u0027agent_type\u0027])"},{"line_number":1593,"context_line":"                if not mech_driver:"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a2fa921_6addbf6f","line":1590,"updated":"2015-10-05 15:17:20.000000000","message":"see PS8 comments","commit_id":"976aae4d4d3c505508be502dee463dcd9af96f67"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"ff7942eb37e4e8a7fd007b0fc9b2293db2f29888","unresolved":false,"context_lines":[{"line_number":1589,"context_line":"        found \u003d False"},{"line_number":1590,"context_line":"        for agent in self.get_agents(context):"},{"line_number":1591,"context_line":"            mech_driver \u003d self.mechanism_manager.get_agent_to_mech_driver("},{"line_number":1592,"context_line":"                agent)"},{"line_number":1593,"context_line":"            if not mech_driver:"},{"line_number":1594,"context_line":"                continue"},{"line_number":1595,"context_line":"            for segment in segments:"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a29b11f_7acf747f","line":1592,"updated":"2015-10-20 11:00:05.000000000","message":"i tend to think it\u0027s better to have a higher-level mech driver api.\nfor example, get_list_of_hosts_on_which_segment_is_served(segment).\nagent-based drivers can use check_segment_for_agent to implement it.\nhow do you think?","commit_id":"e9beefb657994c928e7ff71c365ec91860756375"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"ff7942eb37e4e8a7fd007b0fc9b2293db2f29888","unresolved":false,"context_lines":[{"line_number":1605,"context_line":"        if found:"},{"line_number":1606,"context_line":"            return list({a for a in candidate_agents if a[\u0027host\u0027] in hosts})"},{"line_number":1607,"context_line":"        else:"},{"line_number":1608,"context_line":"            return candidate_agents"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a29b11f_1f3512f6","line":1608,"updated":"2015-10-20 11:00:05.000000000","message":"there is a use-case where agent-based mech driver and controller-based mech driver exist in a single deployment.\nin that case, this logic would consider only hosts served by agent-based drivers.","commit_id":"e9beefb657994c928e7ff71c365ec91860756375"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"039b528daada6cc83fb77ef5ab30fb6c412ba2f9","unresolved":false,"context_lines":[{"line_number":1605,"context_line":"        if found:"},{"line_number":1606,"context_line":"            return list({a for a in candidate_agents if a[\u0027host\u0027] in hosts})"},{"line_number":1607,"context_line":"        else:"},{"line_number":1608,"context_line":"            return candidate_agents"}],"source_content_type":"text/x-python","patch_set":14,"id":"3a29b11f_d1869f64","line":1608,"in_reply_to":"3a29b11f_1f3512f6","updated":"2015-10-26 22:37:59.000000000","message":"Yes because currently no controller-based mech driver provides check_segment_for_agent feature","commit_id":"e9beefb657994c928e7ff71c365ec91860756375"},{"author":{"_account_id":6854,"name":"YAMAMOTO Takashi","email":"yamamoto@midokura.com","username":"yamamoto"},"change_message_id":"1ca6312ed13e1d1d192147af098ff2e5c5f77ace","unresolved":false,"context_lines":[{"line_number":1605,"context_line":"        if found:"},{"line_number":1606,"context_line":"            return list({a for a in candidate_agents if a[\u0027host\u0027] in hosts})"},{"line_number":1607,"context_line":"        else:"},{"line_number":1608,"context_line":"            return candidate_agents"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa80f949_c603460c","line":1608,"in_reply_to":"3a29b11f_d1869f64","updated":"2015-11-05 10:04:53.000000000","message":"sure.  it\u0027s the reason why i suggested higher-level api.\n\ncheck_segment_for_agent api assumes it\u0027s served by l2-agents.\nit isn\u0027t necessary the case for controller based drivers.","commit_id":"e9beefb657994c928e7ff71c365ec91860756375"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8bcd6560dfa8fab764ba9a7a613ddc850c33a9c4","unresolved":false,"context_lines":[{"line_number":1570,"context_line":"    def get_workers(self):"},{"line_number":1571,"context_line":"        return self.mechanism_manager.get_workers()"},{"line_number":1572,"context_line":""},{"line_number":1573,"context_line":"    def get_hosts_with_network_access(self, context, network_id):"},{"line_number":1574,"context_line":"        \"\"\"Return hosts with access to network_id."},{"line_number":1575,"context_line":""},{"line_number":1576,"context_line":"        A L2 agent is defined as an agent that has a ML2 mech driver with a"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba8a016a_18471478","line":1573,"updated":"2015-11-19 09:11:38.000000000","message":"I\u0027m thinking, if we could do vice versa? Usually we schedule networks on controllers which are few in comparison of all other nodes.\n\nSo what if we pass here a set of hosts and ask, if they have network access. Then we could filter agents and spend less cycles.","commit_id":"022f822b0ff259fc5a2c53aabcc14ff15bdcfbaa"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"c84f8cc9c0731a6e41c4b58404e9e6ea3e920356","unresolved":false,"context_lines":[{"line_number":1570,"context_line":"    def get_workers(self):"},{"line_number":1571,"context_line":"        return self.mechanism_manager.get_workers()"},{"line_number":1572,"context_line":""},{"line_number":1573,"context_line":"    def get_hosts_with_network_access(self, context, network_id):"},{"line_number":1574,"context_line":"        \"\"\"Return hosts with access to network_id."},{"line_number":1575,"context_line":""},{"line_number":1576,"context_line":"        A L2 agent is defined as an agent that has a ML2 mech driver with a"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba8a016a_7a7606da","line":1573,"in_reply_to":"ba8a016a_18471478","updated":"2015-11-20 18:07:14.000000000","message":"Done","commit_id":"022f822b0ff259fc5a2c53aabcc14ff15bdcfbaa"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8bcd6560dfa8fab764ba9a7a613ddc850c33a9c4","unresolved":false,"context_lines":[{"line_number":1586,"context_line":"        segments \u003d db.get_network_segments(context.session, network_id)"},{"line_number":1587,"context_line":"        hosts \u003d set()"},{"line_number":1588,"context_line":"        found \u003d False"},{"line_number":1589,"context_line":"        for agent in self.get_agents(context):"},{"line_number":1590,"context_line":"            mech_driver \u003d self.mechanism_manager.get_mech_driver_for_agent("},{"line_number":1591,"context_line":"                agent)"},{"line_number":1592,"context_line":"            if not mech_driver:"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba8a016a_b88b68c2","line":1589,"updated":"2015-11-19 09:11:38.000000000","message":"I\u0027m afraid that kind of way to determine if network can be on a host is costy.\nOn big clusters it may lead to a case when you create network/subnet/spawn a vm and it fails to get IP, because this particular place would take few additional seconds to finish.\nSee comment above.","commit_id":"022f822b0ff259fc5a2c53aabcc14ff15bdcfbaa"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"852083df6c7d8d4dcb8865a3d2674dc5330e2d3c","unresolved":false,"context_lines":[{"line_number":1579,"context_line":"        (doesn\u0027t return None)."},{"line_number":1580,"context_line":""},{"line_number":1581,"context_line":"        If no L2 agent exists:"},{"line_number":1582,"context_line":"          Return all candidate hosts"},{"line_number":1583,"context_line":"        Else:"},{"line_number":1584,"context_line":"          Return candidate hosts with L2 agents having access to at least a"},{"line_number":1585,"context_line":"          segment of network_id."}],"source_content_type":"text/x-python","patch_set":19,"id":"7a740942_123a3337","line":1582,"updated":"2015-12-04 00:22:16.000000000","message":"So, no luck for non agent-based implementations?  Maybe that is acceptable but limits the scope of the bug fix.\n\nThis might be okay to start out with.  However, I think we\u0027ll eventually want to encourage plugins and ML2 drivers to implement this ability because it seems that we\u0027ll be having more Networks that aren\u0027t available everywhere as was previously assumed.","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"746bbf7621ba97faa16882610b048da524c63d8c","unresolved":false,"context_lines":[{"line_number":1579,"context_line":"        (doesn\u0027t return None)."},{"line_number":1580,"context_line":""},{"line_number":1581,"context_line":"        If no L2 agent exists:"},{"line_number":1582,"context_line":"          Return all candidate hosts"},{"line_number":1583,"context_line":"        Else:"},{"line_number":1584,"context_line":"          Return candidate hosts with L2 agents having access to at least a"},{"line_number":1585,"context_line":"          segment of network_id."}],"source_content_type":"text/x-python","patch_set":19,"id":"7a740942_470c2d94","line":1582,"in_reply_to":"7a740942_123a3337","updated":"2015-12-11 00:07:34.000000000","message":"I am not sure i pushed the code but i tried an alternative approach using the following pattern which supports non agent-based implementations.\n\n for mechanism in ...:\n    mech_hosts \u003d mechanism.filter_hosts_with_network_access(...)\n    if mech_hosts is not None:\n      found \u003d True\n      hosts |\u003d mech_hosts\n\nBut i forgot the non-agent case and was seduced by the fact the proposed change queries once agents","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"aa5a0e88120074c453af7a471349438c12f777ca","unresolved":false,"context_lines":[{"line_number":1579,"context_line":"        (doesn\u0027t return None)."},{"line_number":1580,"context_line":""},{"line_number":1581,"context_line":"        If no L2 agent exists:"},{"line_number":1582,"context_line":"          Return all candidate hosts"},{"line_number":1583,"context_line":"        Else:"},{"line_number":1584,"context_line":"          Return candidate hosts with L2 agents having access to at least a"},{"line_number":1585,"context_line":"          segment of network_id."}],"source_content_type":"text/x-python","patch_set":19,"id":"5a710552_2cc4c63e","line":1582,"in_reply_to":"7a740942_470c2d94","updated":"2015-12-14 20:30:36.000000000","message":"@Cedric, this change is probably a good approach.  Would you be willing to follow on this work with some more to flesh out this idea?  It would be an enabler for the routed networks work that is ramping up in the L3 subteam.\n\nRight now, I\u0027m thinking of adding an optional method to the plugin API for plugins which are capable of doing this kind of host mapping.  Then, this work would become the implementation of that method in ML2.  We\u0027d want to figure out how to take care of non agent-based and we\u0027d probably want to address the comment on L1591 too.\n\nI think the scope of this change is fine to start with but I really don\u0027t want to leave it like it like this where some ML2 mechanism drivers will have this implicit bonus behavior.  Does that make sense?","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"852083df6c7d8d4dcb8865a3d2674dc5330e2d3c","unresolved":false,"context_lines":[{"line_number":1588,"context_line":"        hosts \u003d set()"},{"line_number":1589,"context_line":"        found \u003d False"},{"line_number":1590,"context_line":"        filters \u003d {\u0027host\u0027: candidate_hosts}"},{"line_number":1591,"context_line":"        for agent in self.get_agents(context, filters\u003dfilters):"},{"line_number":1592,"context_line":"            mech_driver \u003d self.mechanism_manager.get_mech_driver_for_agent("},{"line_number":1593,"context_line":"                agent)"},{"line_number":1594,"context_line":"            if not mech_driver:"}],"source_content_type":"text/x-python","patch_set":19,"id":"7a740942_52a67bc2","line":1591,"updated":"2015-12-04 00:22:16.000000000","message":"Could this be a pretty big hit in a large deployment?\n\nThis might be okay too since right now candidate_hosts is limited to DHCP agents which usually isn\u0027t a very long list.  However, what happens when I want to use this to filter compute hosts?  That could be a much longer list.  \n\nI\u0027d really like to see this get more efficient by storing the mapping in a way that a single DB query can do the filtering.  I think this means putting the mappings data in a column instead of the query opaque json blob that it seems to be stored in.\n\nI can see why you might not be motivated to solve this for me but it would be cool to see it solved right up front","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"aa5a0e88120074c453af7a471349438c12f777ca","unresolved":false,"context_lines":[{"line_number":1588,"context_line":"        hosts \u003d set()"},{"line_number":1589,"context_line":"        found \u003d False"},{"line_number":1590,"context_line":"        filters \u003d {\u0027host\u0027: candidate_hosts}"},{"line_number":1591,"context_line":"        for agent in self.get_agents(context, filters\u003dfilters):"},{"line_number":1592,"context_line":"            mech_driver \u003d self.mechanism_manager.get_mech_driver_for_agent("},{"line_number":1593,"context_line":"                agent)"},{"line_number":1594,"context_line":"            if not mech_driver:"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a710552_4c17d2ed","line":1591,"in_reply_to":"7a740942_075f0542","updated":"2015-12-14 20:30:36.000000000","message":"Fair enough.","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"746bbf7621ba97faa16882610b048da524c63d8c","unresolved":false,"context_lines":[{"line_number":1588,"context_line":"        hosts \u003d set()"},{"line_number":1589,"context_line":"        found \u003d False"},{"line_number":1590,"context_line":"        filters \u003d {\u0027host\u0027: candidate_hosts}"},{"line_number":1591,"context_line":"        for agent in self.get_agents(context, filters\u003dfilters):"},{"line_number":1592,"context_line":"            mech_driver \u003d self.mechanism_manager.get_mech_driver_for_agent("},{"line_number":1593,"context_line":"                agent)"},{"line_number":1594,"context_line":"            if not mech_driver:"}],"source_content_type":"text/x-python","patch_set":19,"id":"7a740942_075f0542","line":1591,"in_reply_to":"7a740942_52a67bc2","updated":"2015-12-11 00:07:34.000000000","message":"It\u0027s a known limitation which seems acceptable for the moment because as you said deployments should have a reasonable number of dhcp-agents.\n\nIt\u0027s in my todolist to address this limitation by filtering on DB side BUT i would prefer to do it in a follow-up change to avoid a too large change. Moreover db filtering comes with new questions: \n\n* How to migrate the db? using a db-migration script or by accepting to wait for the new report which will populate the new table\n* Using a db query implies potential inconsistency with out-of-tree (if any?) mechanisms overloading check_segment_for_agent.\n* See how we could this table to handle the migration associated to https://review.openstack.org/237218\n\nSo i propose to add a note about the limitation and open a new bug/rfe","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"c449ca1efd1f85f02a4a7b53c7abd72fca6a876b","unresolved":false,"context_lines":[{"line_number":1598,"context_line":"                    segment, agent)"},{"line_number":1599,"context_line":"                if with_access is None:"},{"line_number":1600,"context_line":"                    break"},{"line_number":1601,"context_line":"                found \u003d True"},{"line_number":1602,"context_line":"                if with_access:"},{"line_number":1603,"context_line":"                    hosts.add(agent[\u0027host\u0027])"},{"line_number":1604,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba8a016a_d386d064","line":1601,"updated":"2015-11-22 14:34:10.000000000","message":"should \u0027found\u0027 be True when if with_access is True?","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"1b8c5746b66b834e8cb9d684580e649ecc5bff96","unresolved":false,"context_lines":[{"line_number":1598,"context_line":"                    segment, agent)"},{"line_number":1599,"context_line":"                if with_access is None:"},{"line_number":1600,"context_line":"                    break"},{"line_number":1601,"context_line":"                found \u003d True"},{"line_number":1602,"context_line":"                if with_access:"},{"line_number":1603,"context_line":"                    hosts.add(agent[\u0027host\u0027])"},{"line_number":1604,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba8a016a_ca5244de","line":1601,"in_reply_to":"ba8a016a_546fdcfb","updated":"2015-11-24 10:57:00.000000000","message":"found means we found at least a L2 agent (we don\u0027t if the L2 agent has access or not)","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"b6e81b2cb353c801039180fbc91737671092d330","unresolved":false,"context_lines":[{"line_number":1598,"context_line":"                    segment, agent)"},{"line_number":1599,"context_line":"                if with_access is None:"},{"line_number":1600,"context_line":"                    break"},{"line_number":1601,"context_line":"                found \u003d True"},{"line_number":1602,"context_line":"                if with_access:"},{"line_number":1603,"context_line":"                    hosts.add(agent[\u0027host\u0027])"},{"line_number":1604,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba8a016a_546fdcfb","line":1601,"in_reply_to":"ba8a016a_c8785e92","updated":"2015-11-24 09:44:43.000000000","message":"If so, why not:\n    if with_access:\n        found \u003d True\nor found is only used for finding L2 agents, and it doesn\u0027t care whether L2 agents have access or not?","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"bc47d8cb57cce22c26bfaba3d68ff567c301392e","unresolved":false,"context_lines":[{"line_number":1598,"context_line":"                    segment, agent)"},{"line_number":1599,"context_line":"                if with_access is None:"},{"line_number":1600,"context_line":"                    break"},{"line_number":1601,"context_line":"                found \u003d True"},{"line_number":1602,"context_line":"                if with_access:"},{"line_number":1603,"context_line":"                    hosts.add(agent[\u0027host\u0027])"},{"line_number":1604,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba8a016a_c8785e92","line":1601,"in_reply_to":"ba8a016a_d386d064","updated":"2015-11-22 16:52:14.000000000","message":"Yes, see https://review.openstack.org/#/c/205631/19/neutron/plugins/ml2/driver_api.py","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"c449ca1efd1f85f02a4a7b53c7abd72fca6a876b","unresolved":false,"context_lines":[{"line_number":1603,"context_line":"                    hosts.add(agent[\u0027host\u0027])"},{"line_number":1604,"context_line":"                    break"},{"line_number":1605,"context_line":""},{"line_number":1606,"context_line":"        if found:"},{"line_number":1607,"context_line":"            return {h for h in candidate_hosts if h in hosts}"},{"line_number":1608,"context_line":"        else:"},{"line_number":1609,"context_line":"            return candidate_hosts"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba8a016a_138d387b","line":1606,"updated":"2015-11-22 14:34:10.000000000","message":"Can we directly use \"if hosts\" here? Difference of L1607 and L1609 I can see is whether hosts is empty or not.","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"bc47d8cb57cce22c26bfaba3d68ff567c301392e","unresolved":false,"context_lines":[{"line_number":1603,"context_line":"                    hosts.add(agent[\u0027host\u0027])"},{"line_number":1604,"context_line":"                    break"},{"line_number":1605,"context_line":""},{"line_number":1606,"context_line":"        if found:"},{"line_number":1607,"context_line":"            return {h for h in candidate_hosts if h in hosts}"},{"line_number":1608,"context_line":"        else:"},{"line_number":1609,"context_line":"            return candidate_hosts"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba8a016a_483eaeb9","line":1606,"in_reply_to":"ba8a016a_138d387b","updated":"2015-11-22 16:52:14.000000000","message":"No, as explained in the docstring","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"b6e81b2cb353c801039180fbc91737671092d330","unresolved":false,"context_lines":[{"line_number":1603,"context_line":"                    hosts.add(agent[\u0027host\u0027])"},{"line_number":1604,"context_line":"                    break"},{"line_number":1605,"context_line":""},{"line_number":1606,"context_line":"        if found:"},{"line_number":1607,"context_line":"            return {h for h in candidate_hosts if h in hosts}"},{"line_number":1608,"context_line":"        else:"},{"line_number":1609,"context_line":"            return candidate_hosts"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba8a016a_af17d5ec","line":1606,"in_reply_to":"ba8a016a_483eaeb9","updated":"2015-11-24 09:44:43.000000000","message":"\"Return candidate hosts with L2 agents having access to at least a segment of network_id.\", so that will return hosts:\n1) have L2 agents on them, and\n2) their L2 agents having access to a segment.\n\nI suppose \"found \u003d True\" in L1601 is a flag to check condition 1),  but it won\u0027t check condition 2) . So if all L2 agents don\u0027t have access, it will return an empty dict in L1607. Does this work as your plan, or in this case, it\u0027s better to return an empty dict than return all candidate_hosts as no L2 agents found case does?","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"24f5cd92eb6ce81ce7adc98e9d1a7b60fdccd67a","unresolved":false,"context_lines":[{"line_number":1603,"context_line":"                    hosts.add(agent[\u0027host\u0027])"},{"line_number":1604,"context_line":"                    break"},{"line_number":1605,"context_line":""},{"line_number":1606,"context_line":"        if found:"},{"line_number":1607,"context_line":"            return {h for h in candidate_hosts if h in hosts}"},{"line_number":1608,"context_line":"        else:"},{"line_number":1609,"context_line":"            return candidate_hosts"}],"source_content_type":"text/x-python","patch_set":19,"id":"9a8ffd7b_ccf2de06","line":1606,"in_reply_to":"ba8a016a_6aad589a","updated":"2015-11-24 14:12:46.000000000","message":"Thanks for your explanation, I removed my -1, I need consider more.","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"1b8c5746b66b834e8cb9d684580e649ecc5bff96","unresolved":false,"context_lines":[{"line_number":1603,"context_line":"                    hosts.add(agent[\u0027host\u0027])"},{"line_number":1604,"context_line":"                    break"},{"line_number":1605,"context_line":""},{"line_number":1606,"context_line":"        if found:"},{"line_number":1607,"context_line":"            return {h for h in candidate_hosts if h in hosts}"},{"line_number":1608,"context_line":"        else:"},{"line_number":1609,"context_line":"            return candidate_hosts"}],"source_content_type":"text/x-python","patch_set":19,"id":"ba8a016a_6aad589a","line":1606,"in_reply_to":"ba8a016a_af17d5ec","updated":"2015-11-24 10:57:00.000000000","message":"If there is no L2 agent then we filter nothing and return all candidates?\n\nIf there is L2 agents but none with network access then we return an empty list as there is no valid hosts","commit_id":"2286957caa77c5fc7eda3caa81b6b4fb4ee213e4"}],"neutron/scheduler/dhcp_agent_scheduler.py":[{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"ff26e10c2e2b16b16490e6bf2f732a0d96cfb4f8","unresolved":false,"context_lines":[{"line_number":125,"context_line":"                access_filter(context, network_id)]"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    @classmethod"},{"line_number":128,"context_line":"    def filter_unreachable_hosts(cls, agents_dict, hosts_with_access):"},{"line_number":129,"context_line":"        filtered_agents \u003d ["},{"line_number":130,"context_line":"            agent for agent in agents_dict[\u0027hostable_agents\u0027] if"},{"line_number":131,"context_line":"            agent[\u0027host\u0027] in hosts_with_access]"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_6c48552e","line":128,"updated":"2015-07-24 18:11:33.000000000","message":"i would prefer filter_reachable_hosts as it returns reachable hosts.","commit_id":"0b3b3fe32a823001eb6a1769c7b993512fbc462e"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d2d9093e524c757d2de96d5797c7ab7cf93a3fee","unresolved":false,"context_lines":[{"line_number":125,"context_line":"                access_filter(context, network_id)]"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    @classmethod"},{"line_number":128,"context_line":"    def filter_unreachable_hosts(cls, agents_dict, hosts_with_access):"},{"line_number":129,"context_line":"        filtered_agents \u003d ["},{"line_number":130,"context_line":"            agent for agent in agents_dict[\u0027hostable_agents\u0027] if"},{"line_number":131,"context_line":"            agent[\u0027host\u0027] in hosts_with_access]"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_ef1f74b5","line":128,"in_reply_to":"3a50d1a3_6c48552e","updated":"2015-07-24 19:52:55.000000000","message":"Exactly, so it filters out the unreachable ones.","commit_id":"0b3b3fe32a823001eb6a1769c7b993512fbc462e"},{"author":{"_account_id":17535,"name":"Edgar Cantu","email":"eocantu@us.ibm.com","username":"ecantu"},"change_message_id":"5d6580fb31032d0f2e7c9fe345e1664bcf88cd4c","unresolved":false,"context_lines":[{"line_number":130,"context_line":"            agent for agent in agents_dict[\u0027hostable_agents\u0027] if"},{"line_number":131,"context_line":"            agent[\u0027host\u0027] in hosts_with_access]"},{"line_number":132,"context_line":"        agents_dict[\u0027hostable_agents\u0027] \u003d filtered_agents"},{"line_number":133,"context_line":"        # Get either the old value, or a lower value of we filtered any"},{"line_number":134,"context_line":"        agents_dict[\u0027n_agents\u0027] \u003d min("},{"line_number":135,"context_line":"            len(filtered_agents), agents_dict[\u0027n_agents\u0027])"},{"line_number":136,"context_line":"        return agents_dict"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_0d2c28aa","line":133,"updated":"2015-07-24 21:21:08.000000000","message":"s/lower value of/lower value if/","commit_id":"a90e75b74e17a26e2b7ea36177763ddf93405663"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"aa828c61cb6790bf601e64cb03e93395a3770a8a","unresolved":false,"context_lines":[{"line_number":130,"context_line":"            agent for agent in agents_dict[\u0027hostable_agents\u0027] if"},{"line_number":131,"context_line":"            agent[\u0027host\u0027] in hosts_with_access]"},{"line_number":132,"context_line":"        agents_dict[\u0027hostable_agents\u0027] \u003d filtered_agents"},{"line_number":133,"context_line":"        # Get either the old value, or a lower value of we filtered any"},{"line_number":134,"context_line":"        agents_dict[\u0027n_agents\u0027] \u003d min("},{"line_number":135,"context_line":"            len(filtered_agents), agents_dict[\u0027n_agents\u0027])"},{"line_number":136,"context_line":"        return agents_dict"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_de21fafd","line":133,"in_reply_to":"3a50d1a3_0d2c28aa","updated":"2015-07-25 18:03:39.000000000","message":"Done","commit_id":"a90e75b74e17a26e2b7ea36177763ddf93405663"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"429cafe5e4ea946f86316e7bf7f21b313990997f","unresolved":false,"context_lines":[{"line_number":143,"context_line":"            return {\u0027n_agents\u0027: 0, \u0027hostable_agents\u0027: []}"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        hosts_with_access \u003d self.get_hosts_with_network_access("},{"line_number":146,"context_line":"            plugin, context, network[\u0027id\u0027])"},{"line_number":147,"context_line":"        if hosts_with_access is None:"},{"line_number":148,"context_line":"            return agents_dict"},{"line_number":149,"context_line":"        return self.filter_unreachable_hosts(agents_dict, hosts_with_access)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a50d1a3_0ce095cb","line":146,"updated":"2015-07-26 18:52:44.000000000","message":"Shouldn\u0027t this logic be part of _get_network_hostable_dhcp_agents? The description of that function is that it returns the agents that can host the  network.","commit_id":"e0202e2a95dfd3f8578f5c80ba997b6f0e929d30"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"eeefdacc0113400c058949b2d00d1519cb4f463b","unresolved":false,"context_lines":[{"line_number":143,"context_line":"            return {\u0027n_agents\u0027: 0, \u0027hostable_agents\u0027: []}"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        hosts_with_access \u003d self.get_hosts_with_network_access("},{"line_number":146,"context_line":"            plugin, context, network[\u0027id\u0027])"},{"line_number":147,"context_line":"        if hosts_with_access is None:"},{"line_number":148,"context_line":"            return agents_dict"},{"line_number":149,"context_line":"        return self.filter_unreachable_hosts(agents_dict, hosts_with_access)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a50d1a3_723d408e","line":146,"in_reply_to":"3a50d1a3_0ce095cb","updated":"2015-07-27 19:19:07.000000000","message":"This is a good point, it actually simplifies the code a bit.","commit_id":"e0202e2a95dfd3f8578f5c80ba997b6f0e929d30"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"09195a2f6bb823deb1876b3c2cdfffa510f504c7","unresolved":false,"context_lines":[{"line_number":127,"context_line":"            return None"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        return [agent[\u0027host\u0027] for agent in"},{"line_number":130,"context_line":"                plugin.get_agents_with_access_to_network(context, network_id)]"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def filter_agents(self, plugin, context, network):"},{"line_number":133,"context_line":"        \"\"\"Return the agents that can host the network.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_cd4fd032","line":130,"updated":"2015-08-11 16:51:47.000000000","message":"Given how this is used below, returning a set would be more appropriate.  The number of hosts could get pretty large as we try to push scale.\n\n(i.e. Just change [ and ] to { and })","commit_id":"580af440dd6b7a285d969318953672b12406da08"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a3a72b2036f864273129b4b885f00363f7ef33f3","unresolved":false,"context_lines":[{"line_number":127,"context_line":"            return None"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        return [agent[\u0027host\u0027] for agent in"},{"line_number":130,"context_line":"                plugin.get_agents_with_access_to_network(context, network_id)]"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    def filter_agents(self, plugin, context, network):"},{"line_number":133,"context_line":"        \"\"\"Return the agents that can host the network.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"fa1b9901_abcaa86c","line":130,"in_reply_to":"1a4dcd0f_cd4fd032","updated":"2015-08-19 00:24:06.000000000","message":"You\u0027re correct the only usage is the \u0027in\u0027 operator.","commit_id":"580af440dd6b7a285d969318953672b12406da08"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"cd9faf2fe739e184a1828b5953b190e631c85a0c","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        if not hasattr(plugin, \u0027get_agents_with_access_to_network\u0027):"},{"line_number":127,"context_line":"            return None"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        agents \u003d plugin.get_agents_with_access_to_network(context, network_id)"},{"line_number":130,"context_line":"        if agents is None:"},{"line_number":131,"context_line":"            return None"},{"line_number":132,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"fa1b9901_076b4eb8","line":129,"updated":"2015-08-19 21:56:28.000000000","message":"why not defining a default get_agents_with_access_to_network in neutron.neutron_plugin_base_v2.NeutronPluginBaseV2 returning None?","commit_id":"b8cf9007a9ac9e9526e19e983ea4e07d73495258"},{"author":{"_account_id":6072,"name":"Eugene Nikanorov","email":"enikanorov@mirantis.com","username":"enikanorov"},"change_message_id":"8bcd6560dfa8fab764ba9a7a613ddc850c33a9c4","unresolved":false,"context_lines":[{"line_number":168,"context_line":"                context, True, agent)"},{"line_number":169,"context_line":"        ]"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"        hosts_with_access \u003d plugin.get_hosts_with_network_access("},{"line_number":172,"context_line":"            context, network[\u0027id\u0027])"},{"line_number":173,"context_line":"        if hosts_with_access is not None:"},{"line_number":174,"context_line":"            hostable_dhcp_agents \u003d [agent for agent in hostable_dhcp_agents"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba8a016a_4b06281e","line":171,"updated":"2015-11-19 09:11:38.000000000","message":"This looks better, but imo, could be improved (see comments to plugin.py)","commit_id":"022f822b0ff259fc5a2c53aabcc14ff15bdcfbaa"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"1c900a22ee47cda93b0dd04ab77ca3487fe8fba5","unresolved":false,"context_lines":[{"line_number":235,"context_line":"                context, True, agent)"},{"line_number":236,"context_line":"        ]"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"        hostable_dhcp_hosts \u003d plugin.filter_hosts_with_network_access("},{"line_number":239,"context_line":"            context, network[\u0027id\u0027],"},{"line_number":240,"context_line":"            [agent[\u0027host\u0027] for agent in hostable_dhcp_agents])"},{"line_number":241,"context_line":"        hostable_dhcp_agents \u003d [agent for agent in hostable_dhcp_agents"}],"source_content_type":"text/x-python","patch_set":31,"id":"da6ed579_6bf950c8","line":238,"updated":"2016-01-12 04:38:35.000000000","message":"Just re-iterating what Assaf said, this is just for supporting networks that have limited reachability as they exist now. Not for multiple segments of the same network which are disjoint.","commit_id":"a6add8fc0a342401f2e82c3a32411ffc66bcbe9a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"32ce4b786e29088ea936c8d96d9c4cfb7dcb52c4","unresolved":false,"context_lines":[{"line_number":235,"context_line":"                context, True, agent)"},{"line_number":236,"context_line":"        ]"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"        hostable_dhcp_hosts \u003d plugin.filter_hosts_with_network_access("},{"line_number":239,"context_line":"            context, network[\u0027id\u0027],"},{"line_number":240,"context_line":"            [agent[\u0027host\u0027] for agent in hostable_dhcp_agents])"},{"line_number":241,"context_line":"        hostable_dhcp_agents \u003d [agent for agent in hostable_dhcp_agents"}],"source_content_type":"text/x-python","patch_set":31,"id":"fa69d971_d0d95757","line":238,"updated":"2016-01-11 19:39:36.000000000","message":"So, I found a reference [1] that supports what I was thinking about the implicit assumption that segments are bridged.  I think that the whole multi-segment thing was designed around the assumption that all of the segments were bridged together to provide contiguous L2 throughout.  (Otherwise, DHCP is totally broken anyway.\n\nSo, if that is true then why do we need this patch at all?\n\nDon\u0027t misunderstand this as me saying we don\u0027t need this work because I think we do and I want it to continue very much.  I\u0027m just saying that this work really doesn\u0027t make sense to me in the current Neutron context.  I wonder if we need to change the context a bit to where it makes more sense.  Specifically, do we need to add something like the l2-adjacency flag that I propose here [2]?  Otherwise, everyone is still confused about it.\n\nReading the bug for this work, it seems that the motivation for this comes from the operators who are interested in routed networks [3].  So, I think it might make sense to \n\n [1] https://bugs.launchpad.net/openstack-api-site/+bug/1242019/comments/3 (see paragraph 3 of 4)\n [2] https://review.openstack.org/#/c/225384/14/specs/mitaka/booting-vms-to-routed-networks.rst (L84)\n [3] https://etherpad.openstack.org/p/Network_Segmentation_Usecases","commit_id":"a6add8fc0a342401f2e82c3a32411ffc66bcbe9a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"ffc0a2710ae3972fd0b5c8d1bc9d49af12656095","unresolved":false,"context_lines":[{"line_number":235,"context_line":"                context, True, agent)"},{"line_number":236,"context_line":"        ]"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"        hostable_dhcp_hosts \u003d plugin.filter_hosts_with_network_access("},{"line_number":239,"context_line":"            context, network[\u0027id\u0027],"},{"line_number":240,"context_line":"            [agent[\u0027host\u0027] for agent in hostable_dhcp_agents])"},{"line_number":241,"context_line":"        hostable_dhcp_agents \u003d [agent for agent in hostable_dhcp_agents"}],"source_content_type":"text/x-python","patch_set":31,"id":"da6ed579_afc6c697","line":238,"in_reply_to":"da6ed579_6bf950c8","updated":"2016-01-14 17:27:09.000000000","message":"@Assaf, @Kevin The bug references the etherpad created by the large deployers team in the context of requesting routed networks [1].  I was the one who requested they create this etherpad page and I\u0027ve been over it a dozen times.  I\u0027ve talked to most, if not all, of the contributors to that document.\n\nThe current proposal for routed networks uses multi-segment networks to solve it.  So, if we\u0027re going to go in that direction to solve the routed networks use case, why would we consider this as a different use case?\n\nAs far as I know, limited reachability of a network is being driven by the routed networks use case.\n\n [1] https://bugs.launchpad.net/neutron/+bug/1458890","commit_id":"a6add8fc0a342401f2e82c3a32411ffc66bcbe9a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"2aa36b54a66b280bfd2fe58314f398945341e862","unresolved":false,"context_lines":[{"line_number":235,"context_line":"                context, True, agent)"},{"line_number":236,"context_line":"        ]"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"        hostable_dhcp_hosts \u003d plugin.filter_hosts_with_network_access("},{"line_number":239,"context_line":"            context, network[\u0027id\u0027],"},{"line_number":240,"context_line":"            [agent[\u0027host\u0027] for agent in hostable_dhcp_agents])"},{"line_number":241,"context_line":"        hostable_dhcp_agents \u003d [agent for agent in hostable_dhcp_agents"}],"source_content_type":"text/x-python","patch_set":31,"id":"da6ed579_e1a92e25","line":238,"in_reply_to":"da6ed579_afc6c697","updated":"2016-01-14 20:04:13.000000000","message":"First, let me offer my apologies for not communicating clearly.  My comments were based on the assumption that this work was being driven by the routed networks use case.  IMO, my assumption was justified from examining the bug linked in the commit message, its description, and the supporting links from that description.\n\nThe only use case that I know of which limits the reachability of hosts to a network\u0027s L2 domain, is the routed networks use case.  Otherwise, I think networks are generally assumed to be available to all hosts and segments within a network are generally assumed to be bridged to create a continuous L2 domain.\n\nWhat context am I missing?\n\nIn any case, I don\u0027t see how this patch stands alone.  There are a number of other problems that come up when we break the assumption that networks provide continuous L2 to all hosts.  I\u0027m trying to cover the ones that I know about in the routed networks spec.","commit_id":"a6add8fc0a342401f2e82c3a32411ffc66bcbe9a"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"1524fd6fd19bddb70d1338a23e52e2eca19055f7","unresolved":false,"context_lines":[{"line_number":235,"context_line":"                context, True, agent)"},{"line_number":236,"context_line":"        ]"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"        hostable_dhcp_hosts \u003d plugin.filter_hosts_with_network_access("},{"line_number":239,"context_line":"            context, network[\u0027id\u0027],"},{"line_number":240,"context_line":"            [agent[\u0027host\u0027] for agent in hostable_dhcp_agents])"},{"line_number":241,"context_line":"        hostable_dhcp_agents \u003d [agent for agent in hostable_dhcp_agents"}],"source_content_type":"text/x-python","patch_set":31,"id":"fa69d971_d00e373c","line":238,"in_reply_to":"fa69d971_d0d95757","updated":"2016-01-11 19:54:17.000000000","message":"\u003e I think that the whole multi-segment thing was designed around the assumption that all of the segments were bridged together [...]  So, if that is true then why do we need this patch at all?\n\nThe original use case that prompted this patch did not involve multi-segment networks (Take a look at the image attached to the commit message). It\u0027s two separate networks, one per rack, each with a different \u0027physical_network\u0027 tag. It makes sense to me. Some operators are carrying a patch that accomplishes what this patch does.\n\nSince it\u0027s possible to solve this specific problem of DHCP scheduling without any specs (Routed networks or otherwise), API changes like l2-adjacency or any other complicated solution, I think this patch makes sense. It\u0027s also backportable which is nice.\n\nCarl can you better explain what doesn\u0027t make sense here? I\u0027m having trouble following.","commit_id":"a6add8fc0a342401f2e82c3a32411ffc66bcbe9a"}],"neutron/tests/functional/scheduler/test_dhcp_agent_scheduler.py":[{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"ff26e10c2e2b16b16490e6bf2f732a0d96cfb4f8","unresolved":false,"context_lines":[{"line_number":404,"context_line":"            self._test_auto_schedule(i)"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"class TestDHCPSchedulerWithNetworkAccessability("},{"line_number":408,"context_line":"    test_plugin.Ml2PluginV2TestCase):"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"    _mechanism_drivers \u003d [\u0027openvswitch\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_6c6ff592","line":407,"updated":"2015-07-24 18:11:33.000000000","message":"TestDHCPSchedulerWithNetworkAccess-I-bility","commit_id":"0b3b3fe32a823001eb6a1769c7b993512fbc462e"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"d2d9093e524c757d2de96d5797c7ab7cf93a3fee","unresolved":false,"context_lines":[{"line_number":404,"context_line":"            self._test_auto_schedule(i)"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":""},{"line_number":407,"context_line":"class TestDHCPSchedulerWithNetworkAccessability("},{"line_number":408,"context_line":"    test_plugin.Ml2PluginV2TestCase):"},{"line_number":409,"context_line":""},{"line_number":410,"context_line":"    _mechanism_drivers \u003d [\u0027openvswitch\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a50d1a3_6f8284ca","line":407,"in_reply_to":"3a50d1a3_6c6ff592","updated":"2015-07-24 19:52:55.000000000","message":"Done","commit_id":"0b3b3fe32a823001eb6a1769c7b993512fbc462e"},{"author":{"_account_id":17535,"name":"Edgar Cantu","email":"eocantu@us.ibm.com","username":"ecantu"},"change_message_id":"5d6580fb31032d0f2e7c9fe345e1664bcf88cd4c","unresolved":false,"context_lines":[{"line_number":16,"context_line":"import six"},{"line_number":17,"context_line":"import testscenarios"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from neutron.api.v2 import attributes"},{"line_number":20,"context_line":"from neutron import context"},{"line_number":21,"context_line":"from neutron.db import agents_db"},{"line_number":22,"context_line":"from neutron.db import agentschedulers_db"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_6d3a8ce9","line":19,"updated":"2015-07-24 21:21:08.000000000","message":"Move below the next line to maintain alphabetical order.","commit_id":"a90e75b74e17a26e2b7ea36177763ddf93405663"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"aa828c61cb6790bf601e64cb03e93395a3770a8a","unresolved":false,"context_lines":[{"line_number":16,"context_line":"import six"},{"line_number":17,"context_line":"import testscenarios"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from neutron.api.v2 import attributes"},{"line_number":20,"context_line":"from neutron import context"},{"line_number":21,"context_line":"from neutron.db import agents_db"},{"line_number":22,"context_line":"from neutron.db import agentschedulers_db"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_7e20e6f7","line":19,"in_reply_to":"3a50d1a3_6d3a8ce9","updated":"2015-07-25 18:03:39.000000000","message":"Done, fixed all pep8 issues.","commit_id":"a90e75b74e17a26e2b7ea36177763ddf93405663"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"09195a2f6bb823deb1876b3c2cdfffa510f504c7","unresolved":false,"context_lines":[{"line_number":423,"context_line":"        net \u003d self.driver.create_network("},{"line_number":424,"context_line":"            admin_context,"},{"line_number":425,"context_line":"            {\u0027network\u0027: {\u0027name\u0027: \u0027net1\u0027,"},{"line_number":426,"context_line":"                         providernet.NETWORK_TYPE: \u0027vlan\u0027,"},{"line_number":427,"context_line":"                         providernet.PHYSICAL_NETWORK: \u0027physnet1\u0027,"},{"line_number":428,"context_line":"                         providernet.SEGMENTATION_ID: 1,"},{"line_number":429,"context_line":"                         \u0027tenant_id\u0027: \u0027tenant_one\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_bee4e83b","line":426,"updated":"2015-08-11 16:51:47.000000000","message":"Would there be any value in testing the multi-segment case given your question in the other file.","commit_id":"580af440dd6b7a285d969318953672b12406da08"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a3a72b2036f864273129b4b885f00363f7ef33f3","unresolved":false,"context_lines":[{"line_number":423,"context_line":"        net \u003d self.driver.create_network("},{"line_number":424,"context_line":"            admin_context,"},{"line_number":425,"context_line":"            {\u0027network\u0027: {\u0027name\u0027: \u0027net1\u0027,"},{"line_number":426,"context_line":"                         providernet.NETWORK_TYPE: \u0027vlan\u0027,"},{"line_number":427,"context_line":"                         providernet.PHYSICAL_NETWORK: \u0027physnet1\u0027,"},{"line_number":428,"context_line":"                         providernet.SEGMENTATION_ID: 1,"},{"line_number":429,"context_line":"                         \u0027tenant_id\u0027: \u0027tenant_one\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"fa1b9901_ebf660f8","line":426,"in_reply_to":"1a4dcd0f_bee4e83b","updated":"2015-08-19 00:24:06.000000000","message":"Yes indeed! Incoming in next PS in ml2.test_plugin.","commit_id":"580af440dd6b7a285d969318953672b12406da08"}],"neutron/tests/unit/plugins/ml2/drivers/mechanism_logger.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"46cd036240891d17565d14a3e0ac89f8c2dac8ad","unresolved":false,"context_lines":[{"line_number":133,"context_line":""},{"line_number":134,"context_line":"    def filter_hosts_with_segment_access("},{"line_number":135,"context_line":"            self, context, segments, candidate_hosts, agent_getter):"},{"line_number":136,"context_line":"        LOG.info(_(\"filter_hosts_with_segment_access called with segments \""},{"line_number":137,"context_line":"                   \"%(segments)s, candidate hosts %(hosts)s \"),"},{"line_number":138,"context_line":"                 {\u0027segments\u0027: segments, \u0027hosts\u0027: candidate_hosts})"},{"line_number":139,"context_line":"        return set()"}],"source_content_type":"text/x-python","patch_set":32,"id":"3a57f1b5_9a038f73","line":136,"updated":"2016-02-10 13:41:59.000000000","message":"missing log hint _LI\nwhy do we need logs in the unit tests","commit_id":"151f4dd9c13d4ef5e3a4d2df5e94cbbf2f061ee5"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"cd0eb8eff3bebb31080fe68e785d01be20b1b114","unresolved":false,"context_lines":[{"line_number":133,"context_line":""},{"line_number":134,"context_line":"    def filter_hosts_with_segment_access("},{"line_number":135,"context_line":"            self, context, segments, candidate_hosts, agent_getter):"},{"line_number":136,"context_line":"        LOG.info(_(\"filter_hosts_with_segment_access called with segments \""},{"line_number":137,"context_line":"                   \"%(segments)s, candidate hosts %(hosts)s \"),"},{"line_number":138,"context_line":"                 {\u0027segments\u0027: segments, \u0027hosts\u0027: candidate_hosts})"},{"line_number":139,"context_line":"        return set()"}],"source_content_type":"text/x-python","patch_set":32,"id":"3a57f1b5_a7edf9f0","line":136,"in_reply_to":"3a57f1b5_9a038f73","updated":"2016-02-10 21:33:28.000000000","message":"It follows _log_network/subnet/port_call logics which logs every call (that\u0027s the aim of the driver) without _LI because it\u0027s a test module (i guess)","commit_id":"151f4dd9c13d4ef5e3a4d2df5e94cbbf2f061ee5"}],"neutron/tests/unit/plugins/ml2/test_plugin.py":[{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"7f5ff39166f811b5c485ff206fcd0d295febfac3","unresolved":false,"context_lines":[{"line_number":1387,"context_line":"                         \u0027shared\u0027: True}})"},{"line_number":1388,"context_line":"        observeds \u003d self.driver.filter_agents_with_network_access("},{"line_number":1389,"context_line":"            self.context, net[\u0027id\u0027], self.dhcp_agents)"},{"line_number":1390,"context_line":"        self.assertEqual([self.dhcp_agent1], observeds)"},{"line_number":1391,"context_line":""},{"line_number":1392,"context_line":"    def test_filter_agents_with_network_access_multi_segments(self):"},{"line_number":1393,"context_line":"        net \u003d self.driver.create_network("}],"source_content_type":"text/x-python","patch_set":12,"id":"3a29b11f_052bd5ec","line":1390,"updated":"2015-10-18 14:25:02.000000000","message":"I guess you have to use\n\n        self.assertIn(self.dhcp_agent1, observeds)\n        self.assertIn(self.dhcp_agent2, observeds)\n\nhere to make python3 happy.","commit_id":"f8aa07ec9e1854224b1cd4eeee0261758f435e7e"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"3c79ee5def33a9bc5904d228c257588ae4258984","unresolved":false,"context_lines":[{"line_number":52,"context_line":"from neutron.services.qos import qos_consts"},{"line_number":53,"context_line":"from neutron.tests import base"},{"line_number":54,"context_line":"from neutron.tests.common import helpers"},{"line_number":55,"context_line":"from neutron.tests import tools"},{"line_number":56,"context_line":"from neutron.tests.unit import _test_extension_portbindings as test_bindings"},{"line_number":57,"context_line":"from neutron.tests.unit.agent import test_securitygroups_rpc as test_sg_rpc"},{"line_number":58,"context_line":"from neutron.tests.unit.db import test_allowedaddresspairs_db as test_pair"}],"source_content_type":"text/x-python","patch_set":13,"id":"3a29b11f_1b8fad24","line":55,"updated":"2015-10-19 08:11:47.000000000","message":"Since you don\u0027t use this import. It should be removed~","commit_id":"4d247e727226ec77aaa6290f912c7fe49cb7c0c7"},{"author":{"_account_id":9459,"name":".Haifeng Yan","email":"yanheven@qq.com","username":"yan"},"change_message_id":"439c494e1bb163e48f44795f27c169ad573e805e","unresolved":false,"context_lines":[{"line_number":1422,"context_line":"            constants.AGENT_TYPE_OVS,"},{"line_number":1423,"context_line":"            self.driver.mechanism_manager.mech_drivers[\u0027openvswitch\u0027].obj)"},{"line_number":1424,"context_line":""},{"line_number":1425,"context_line":"    def test_get_agent_to_no_mech_driver(self):"},{"line_number":1426,"context_line":"        helpers.register_dhcp_agent()"},{"line_number":1427,"context_line":"        self._test_get_mech_driver_for_agent("},{"line_number":1428,"context_line":"            constants.AGENT_TYPE_DHCP, None)"}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_509c4569","line":1425,"updated":"2015-11-07 08:37:19.000000000","message":"would this one be better? \"test_get_no_mech_driver_for_agent\"","commit_id":"0395af8db7932f9d7dc86adde5a0dbeca4a3b4c9"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"bb331518375ceb68e14a5e76e97edfcad4cad6c0","unresolved":false,"context_lines":[{"line_number":1422,"context_line":"            constants.AGENT_TYPE_OVS,"},{"line_number":1423,"context_line":"            self.driver.mechanism_manager.mech_drivers[\u0027openvswitch\u0027].obj)"},{"line_number":1424,"context_line":""},{"line_number":1425,"context_line":"    def test_get_agent_to_no_mech_driver(self):"},{"line_number":1426,"context_line":"        helpers.register_dhcp_agent()"},{"line_number":1427,"context_line":"        self._test_get_mech_driver_for_agent("},{"line_number":1428,"context_line":"            constants.AGENT_TYPE_DHCP, None)"}],"source_content_type":"text/x-python","patch_set":15,"id":"da85f559_1c5e9476","line":1425,"in_reply_to":"da85f559_509c4569","updated":"2015-11-09 21:57:10.000000000","message":"Done","commit_id":"0395af8db7932f9d7dc86adde5a0dbeca4a3b4c9"}],"releasenotes/notes/physical_network-aware-dhcp-scheduling-94e9fadc7c7c5fec.yaml":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"3073937d68f0ef7ad95c5f3eec92496644d2d30b","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"prelude: \u003e"},{"line_number":3,"context_line":"    Schedule dhcps on dhcp-agents with access to network"},{"line_number":4,"context_line":"features:"},{"line_number":5,"context_line":"  - DHCP schedulers use \"filter_host_with_network_access\" plugin method to"},{"line_number":6,"context_line":"    filter hosts with access to dhcp network. Plugins can overload it to"}],"source_content_type":"text/x-yaml","patch_set":30,"id":"fa69d971_e429739c","line":3,"range":{"start_line":3,"start_character":13,"end_line":3,"end_character":18},"updated":"2016-01-09 18:24:55.000000000","message":"not clear to me what dhcps are?","commit_id":"8f1b3ad60a1ace9ea20a86082127fa4a4eb1cbd0"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"56d4fd5c8ed400ec893641a374d484b9894671f6","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"prelude: \u003e"},{"line_number":3,"context_line":"    Schedule dhcps on dhcp-agents with access to network"},{"line_number":4,"context_line":"features:"},{"line_number":5,"context_line":"  - DHCP schedulers use \"filter_host_with_network_access\" plugin method to"},{"line_number":6,"context_line":"    filter hosts with access to dhcp network. Plugins can overload it to"}],"source_content_type":"text/x-yaml","patch_set":30,"id":"fa69d971_5f525adf","line":3,"range":{"start_line":3,"start_character":13,"end_line":3,"end_character":18},"in_reply_to":"fa69d971_e429739c","updated":"2016-01-11 18:48:34.000000000","message":"Done","commit_id":"8f1b3ad60a1ace9ea20a86082127fa4a4eb1cbd0"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"3073937d68f0ef7ad95c5f3eec92496644d2d30b","unresolved":false,"context_lines":[{"line_number":5,"context_line":"  - DHCP schedulers use \"filter_host_with_network_access\" plugin method to"},{"line_number":6,"context_line":"    filter hosts with access to dhcp network. Plugins can overload it to"},{"line_number":7,"context_line":"    define their own filtering logic. In particular, ML2 plugin delegates"},{"line_number":8,"context_line":"    the filtering to mechanim driver."}],"source_content_type":"text/x-yaml","patch_set":30,"id":"fa69d971_a41e1b6e","line":8,"range":{"start_line":8,"start_character":21,"end_line":8,"end_character":29},"updated":"2016-01-09 18:24:55.000000000","message":"mechanim \u003d\u003e mechanism\n\nunless mechanim is the plural of mech","commit_id":"8f1b3ad60a1ace9ea20a86082127fa4a4eb1cbd0"},{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"56d4fd5c8ed400ec893641a374d484b9894671f6","unresolved":false,"context_lines":[{"line_number":5,"context_line":"  - DHCP schedulers use \"filter_host_with_network_access\" plugin method to"},{"line_number":6,"context_line":"    filter hosts with access to dhcp network. Plugins can overload it to"},{"line_number":7,"context_line":"    define their own filtering logic. In particular, ML2 plugin delegates"},{"line_number":8,"context_line":"    the filtering to mechanim driver."}],"source_content_type":"text/x-yaml","patch_set":30,"id":"fa69d971_1f5f02e6","line":8,"range":{"start_line":8,"start_character":21,"end_line":8,"end_character":29},"in_reply_to":"fa69d971_a41e1b6e","updated":"2016-01-11 18:48:34.000000000","message":"Done","commit_id":"8f1b3ad60a1ace9ea20a86082127fa4a4eb1cbd0"}]}