)]}'
{"neutron/services/segments/db.py":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"da0bdec4659e792de4594dcb9275f4b974daa5f8","unresolved":false,"context_lines":[{"line_number":214,"context_line":"    if not segment.physical_network:"},{"line_number":215,"context_line":"        return"},{"line_number":216,"context_line":"    cp \u003d manager.NeutronManager.get_plugin()"},{"line_number":217,"context_line":"    for agent in cp.get_agents(context):"},{"line_number":218,"context_line":"        if segment.physical_network in _get_phys_nets(agent):"},{"line_number":219,"context_line":"            context.session.add(SegmentHostMapping(segment_id\u003dsegment.id,"},{"line_number":220,"context_line":"                                                   host\u003dagent[\u0027host\u0027]))"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_702ba882","line":217,"range":{"start_line":217,"start_character":4,"end_line":217,"end_character":40},"updated":"2016-06-14 09:10:34.000000000","message":"Should we account for agent-less plugins and also filter only l2 agents?","commit_id":"194665f2bba3ebb975edd9bdf242b8d0057675c1"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"697e8e11a518315f361cac3ebc279f74de97bc1d","unresolved":false,"context_lines":[{"line_number":214,"context_line":"    if not segment.physical_network:"},{"line_number":215,"context_line":"        return"},{"line_number":216,"context_line":"    cp \u003d manager.NeutronManager.get_plugin()"},{"line_number":217,"context_line":"    for agent in cp.get_agents(context):"},{"line_number":218,"context_line":"        if segment.physical_network in _get_phys_nets(agent):"},{"line_number":219,"context_line":"            context.session.add(SegmentHostMapping(segment_id\u003dsegment.id,"},{"line_number":220,"context_line":"                                                   host\u003dagent[\u0027host\u0027]))"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aa08908_30b80011","line":217,"range":{"start_line":217,"start_character":4,"end_line":217,"end_character":40},"in_reply_to":"7aa08908_702ba882","updated":"2016-06-14 09:13:05.000000000","message":"There isn\u0027t a way to filter only l2 agents that I\u0027m aware of. I do need to skip if there is no get_agents method though.","commit_id":"194665f2bba3ebb975edd9bdf242b8d0057675c1"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"aee19b71e0b627f4abd12ddc8a2f5b16150f2d46","unresolved":false,"context_lines":[{"line_number":110,"context_line":"                    db.SEGMENTATION_ID: segmentation_id}"},{"line_number":111,"context_line":"            new_segment \u003d db.NetworkSegment(**args)"},{"line_number":112,"context_line":"            context.session.add(new_segment)"},{"line_number":113,"context_line":"            registry.notify(resources.SEGMENT, events.PRECOMMIT_CREATE, self,"},{"line_number":114,"context_line":"                            context\u003dcontext, segment\u003dnew_segment)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        return self._make_segment_dict(new_segment)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7aa08908_c7c61699","line":113,"range":{"start_line":113,"start_character":47,"end_line":113,"end_character":70},"updated":"2016-06-14 12:21:17.000000000","message":"why not after create?","commit_id":"9cb1ef6dabc00da8958ea0505068efbd4ab018d2"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"a534b639f997c5f23284e27106b4cb6549836723","unresolved":false,"context_lines":[{"line_number":110,"context_line":"                    db.SEGMENTATION_ID: segmentation_id}"},{"line_number":111,"context_line":"            new_segment \u003d db.NetworkSegment(**args)"},{"line_number":112,"context_line":"            context.session.add(new_segment)"},{"line_number":113,"context_line":"            registry.notify(resources.SEGMENT, events.PRECOMMIT_CREATE, self,"},{"line_number":114,"context_line":"                            context\u003dcontext, segment\u003dnew_segment)"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        return self._make_segment_dict(new_segment)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7aa08908_a0df6267","line":113,"range":{"start_line":113,"start_character":47,"end_line":113,"end_character":70},"in_reply_to":"7aa08908_c7c61699","updated":"2016-06-14 16:18:33.000000000","message":"Just wanted to avoid potential races with other code updating segment mappings by making it part of the same transaction. (e.g. the agent report-based segment mapping being triggered right at the same time as a new segment)\n\nI could move it to AFTER_CREATE if you think that\u0027s better. I suspect ultimately ML2 will end up calling this method inside of a transaction anyway so it might be difficult to not have this happen in a transaction.","commit_id":"9cb1ef6dabc00da8958ea0505068efbd4ab018d2"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e5af07a6e4237d002e70aa49e60418a170706518","unresolved":false,"context_lines":[{"line_number":217,"context_line":"    if not hasattr(cp, \u0027get_agents\u0027):"},{"line_number":218,"context_line":"        # not an agent-supporting plugin"},{"line_number":219,"context_line":"        return"},{"line_number":220,"context_line":"    for agent in cp.get_agents(context):"},{"line_number":221,"context_line":"        if segment.physical_network in _get_phys_nets(agent):"},{"line_number":222,"context_line":"            context.session.add(SegmentHostMapping(segment_id\u003dsegment.id,"},{"line_number":223,"context_line":"                                                   host\u003dagent[\u0027host\u0027]))"}],"source_content_type":"text/x-python","patch_set":2,"id":"7aa08908_326f3393","line":220,"updated":"2016-06-15 01:48:39.000000000","message":"Premature optimization. You can certainly work on a DB mapping store in a different patch, but the performance isn\u0027t that big of a concern considering segment/network creation is significantly rarer than other neutron operations (e.g. agent state reports, port creations, etc). \n\nAlso keep in mind that we have to actually ask the core plugin if an agent can connect to a segment as Miguel pointed out below.","commit_id":"9cb1ef6dabc00da8958ea0505068efbd4ab018d2"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"f623d3ac58a11bacab621ced7eaa0d988af8a998","unresolved":false,"context_lines":[{"line_number":217,"context_line":"    if not hasattr(cp, \u0027get_agents\u0027):"},{"line_number":218,"context_line":"        # not an agent-supporting plugin"},{"line_number":219,"context_line":"        return"},{"line_number":220,"context_line":"    for agent in cp.get_agents(context):"},{"line_number":221,"context_line":"        if segment.physical_network in _get_phys_nets(agent):"},{"line_number":222,"context_line":"            context.session.add(SegmentHostMapping(segment_id\u003dsegment.id,"},{"line_number":223,"context_line":"                                                   host\u003dagent[\u0027host\u0027]))"}],"source_content_type":"text/x-python","patch_set":2,"id":"7aa08908_fdabc185","line":220,"range":{"start_line":220,"start_character":4,"end_line":220,"end_character":40},"updated":"2016-06-15 00:19:33.000000000","message":"This seems to be inefficient in routed network context, because in large deployment there will be big number of agents, and it makes no sense to check all agents in the data-center when adding a segment in the corner.\n\nCan we find a more efficient way to filter the hosts based on segment.physical_network? Would it be easier to do this if we have host-physnet mapping in neutron db?","commit_id":"9cb1ef6dabc00da8958ea0505068efbd4ab018d2"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"baffdb64ff536e36cfc4f6920937a0580944a4bb","unresolved":false,"context_lines":[{"line_number":218,"context_line":"        # not an agent-supporting plugin"},{"line_number":219,"context_line":"        return"},{"line_number":220,"context_line":"    for agent in cp.get_agents(context):"},{"line_number":221,"context_line":"        if segment.physical_network in _get_phys_nets(agent):"},{"line_number":222,"context_line":"            context.session.add(SegmentHostMapping(segment_id\u003dsegment.id,"},{"line_number":223,"context_line":"                                                   host\u003dagent[\u0027host\u0027]))"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"def subscribe():"}],"source_content_type":"text/x-python","patch_set":2,"id":"7aa08908_4b097330","line":223,"range":{"start_line":221,"start_character":8,"end_line":223,"end_character":71},"updated":"2016-06-14 21:35:40.000000000","message":"Wouldn\u0027t it be better to ask the plugin drivers whether the segment is bound to a host or not? That is what I learned in an IRC conversation with you and Bob Kukura a few weeks ago when I was starting to implement segment host mapping.\n\nIf we agree on this, you could do the same check as in line 193 above and then call the plugin \u0027check_segment_for_agent\u0027 method, that is implemented for the ml2 case here: https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/plugin.py#L1698","commit_id":"9cb1ef6dabc00da8958ea0505068efbd4ab018d2"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e5af07a6e4237d002e70aa49e60418a170706518","unresolved":false,"context_lines":[{"line_number":220,"context_line":"    for agent in cp.get_agents(context):"},{"line_number":221,"context_line":"        if segment.physical_network in _get_phys_nets(agent):"},{"line_number":222,"context_line":"            context.session.add(SegmentHostMapping(segment_id\u003dsegment.id,"},{"line_number":223,"context_line":"                                                   host\u003dagent[\u0027host\u0027]))"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"def subscribe():"}],"source_content_type":"text/x-python","patch_set":2,"id":"7aa08908_926a677f","line":223,"updated":"2016-06-15 01:48:39.000000000","message":"yep, need to ask the plugin explicitly. will adjust","commit_id":"9cb1ef6dabc00da8958ea0505068efbd4ab018d2"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"a6625a8d5a00c9ee5a21bea59e7c323468455283","unresolved":false,"context_lines":[{"line_number":224,"context_line":"             if check_segment_for_agent(segment, agent)}"},{"line_number":225,"context_line":"    for host in hosts:"},{"line_number":226,"context_line":"        context.session.add(SegmentHostMapping(segment_id\u003dsegment.id,"},{"line_number":227,"context_line":"                                               host\u003dhost))"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"def subscribe():"}],"source_content_type":"text/x-python","patch_set":3,"id":"7aa08908_6fe6c2b1","line":227,"updated":"2016-06-15 20:28:55.000000000","message":"Agentless can subscribe to the callback and do whatever it wants. This function isn\u0027t for agentless topologies so it makes no sense to make it try to accomadate them. That\u0027s why I added the callback event.","commit_id":"d435887e37d9f3c9a22f10c1c4bea107eca9d4ba"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"e1df74f77047263f6ded9af5ff1dc5632a4fa5a2","unresolved":false,"context_lines":[{"line_number":222,"context_line":"        return"},{"line_number":223,"context_line":"    hosts \u003d {agent[\u0027host\u0027] for agent in cp.get_agents(context)"},{"line_number":224,"context_line":"             if check_segment_for_agent(segment, agent)}"},{"line_number":225,"context_line":"    for host in hosts:"},{"line_number":226,"context_line":"        context.session.add(SegmentHostMapping(segment_id\u003dsegment.id,"},{"line_number":227,"context_line":"                                               host\u003dhost))"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"def subscribe():"}],"source_content_type":"text/x-python","patch_set":3,"id":"7aa08908_14d81fee","line":227,"range":{"start_line":225,"start_character":0,"end_line":227,"end_character":58},"updated":"2016-06-15 06:35:05.000000000","message":"I think it is better to put the code in a function with its own DB transaction, so the potential agentless topology can reuse that function to update SegmentHostMapping.","commit_id":"d435887e37d9f3c9a22f10c1c4bea107eca9d4ba"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"c2090f5b5acec30665896fd8d8bab376bb23dbf4","unresolved":false,"context_lines":[{"line_number":224,"context_line":"             if check_segment_for_agent(segment, agent)}"},{"line_number":225,"context_line":"    for host in hosts:"},{"line_number":226,"context_line":"        context.session.add(SegmentHostMapping(segment_id\u003dsegment.id,"},{"line_number":227,"context_line":"                                               host\u003dhost))"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"def subscribe():"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a9d85d2_8e54b721","line":227,"in_reply_to":"7aa08908_14af4a2c","updated":"2016-06-21 03:58:35.000000000","message":"Extracting a function would add little value.  This code is pretty trivial and duplicating it, in my opinion, isn\u0027t too bad.  Making it reusable has its own costs which I doubt are justified in this case.","commit_id":"d435887e37d9f3c9a22f10c1c4bea107eca9d4ba"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ae8c40cf98b67467b6c95309cf187f364f92d053","unresolved":false,"context_lines":[{"line_number":224,"context_line":"             if check_segment_for_agent(segment, agent)}"},{"line_number":225,"context_line":"    for host in hosts:"},{"line_number":226,"context_line":"        context.session.add(SegmentHostMapping(segment_id\u003dsegment.id,"},{"line_number":227,"context_line":"                                               host\u003dhost))"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"def subscribe():"}],"source_content_type":"text/x-python","patch_set":3,"id":"7aa08908_14af4a2c","line":227,"in_reply_to":"7aa08908_6fe6c2b1","updated":"2016-06-16 02:24:06.000000000","message":"I mean just wrap line 225 and line 227 into a function, I can imagine that the subscriber in agentless toplology can find out which hosts should be updated, and reuse that function. But that is your decision about how to shape the code, I can add follow up patch for that.","commit_id":"d435887e37d9f3c9a22f10c1c4bea107eca9d4ba"},{"author":{"_account_id":5367,"name":"boden","email":"bodenvmw@gmail.com","username":"boden"},"change_message_id":"4282ab3028a7eed50a301276bef2f5a976db323a","unresolved":false,"context_lines":[{"line_number":215,"context_line":"        return"},{"line_number":216,"context_line":"    cp \u003d manager.NeutronManager.get_plugin()"},{"line_number":217,"context_line":"    check_segment_for_agent \u003d getattr(cp, \u0027check_segment_for_agent\u0027, None)"},{"line_number":218,"context_line":"    if not hasattr(cp, \u0027get_agents\u0027) or not check_segment_for_agent:"},{"line_number":219,"context_line":"        # not an agent-supporting plugin"},{"line_number":220,"context_line":"        registry.unsubscribe(_add_segment_host_mapping_for_segment,"},{"line_number":221,"context_line":"                             resources.SEGMENT, events.PRECOMMIT_CREATE)"},{"line_number":222,"context_line":"        return"},{"line_number":223,"context_line":"    hosts \u003d {agent[\u0027host\u0027] for agent in cp.get_agents(context)"},{"line_number":224,"context_line":"             if check_segment_for_agent(segment, agent)}"},{"line_number":225,"context_line":"    for host in hosts:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3aaa91ec_de3274a6","line":222,"range":{"start_line":218,"start_character":4,"end_line":222,"end_character":14},"updated":"2016-06-24 14:51:29.000000000","message":"Maybe I\u0027m missing it, but I don\u0027t see this \"unsubscribe\" flow covered in the unit tests; should we add a test?","commit_id":"8a0f23e8efafc75c739fea13a350eb17b1337f4d"}]}