)]}'
{"neutron/db/agents_db.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"2631e570e0e0409b87a429af6729dbf1bdc820bf","unresolved":false,"context_lines":[{"line_number":43,"context_line":"from neutron.extensions import agent as ext_agent"},{"line_number":44,"context_line":"from neutron.extensions import availability_zone as az_ext"},{"line_number":45,"context_line":"from neutron import manager"},{"line_number":46,"context_line":"from neutron.services.segments import db as segments_db"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9abb7d3a_ceb772d6","side":"PARENT","line":46,"range":{"start_line":46,"start_character":0,"end_line":46,"end_character":55},"updated":"2016-05-31 22:03:59.000000000","message":"I think this might be your problem with functional tests.  Since this is coming out, we might need an entry here [1] to make sure that the model is imported.\n\n [1] https://github.com/openstack/neutron/blob/ddfe09c5ddb7eebf0ca62e38cf593f7482910c21/neutron/db/migration/models/head.py#L60","commit_id":"46bdf59d8aaade42f509b6977b3a32f13d08b6cd"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"2fce24b9020b1ce367dffe3ee83100ae19fff832","unresolved":false,"context_lines":[{"line_number":43,"context_line":"from neutron.extensions import agent as ext_agent"},{"line_number":44,"context_line":"from neutron.extensions import availability_zone as az_ext"},{"line_number":45,"context_line":"from neutron import manager"},{"line_number":46,"context_line":"from neutron.services.segments import db as segments_db"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7aa08908_cddb8c1a","side":"PARENT","line":46,"range":{"start_line":46,"start_character":0,"end_line":46,"end_character":55},"in_reply_to":"9abb7d3a_ceb772d6","updated":"2016-06-06 06:58:06.000000000","message":"Thanks Carl, it works!","commit_id":"46bdf59d8aaade42f509b6977b3a32f13d08b6cd"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"45d8874db75f531386ae1df06ad4310e68179c63","unresolved":false,"context_lines":[{"line_number":398,"context_line":"                self._log_heartbeat(agent_state, agent_db, configurations_dict)"},{"line_number":399,"context_line":"                status \u003d n_const.AGENT_NEW"},{"line_number":400,"context_line":"            greenthread.sleep(0)"},{"line_number":401,"context_line":"            registry.notify(resources.AGENT, event_type, self, context\u003dcontext,"},{"line_number":402,"context_line":"                            host\u003dagent_state[\u0027host\u0027], plugin\u003dself,"},{"line_number":403,"context_line":"                            agent\u003dagent_state)"},{"line_number":404,"context_line":"        return status"}],"source_content_type":"text/x-python","patch_set":3,"id":"9abb7d3a_bf572c01","line":401,"updated":"2016-05-29 09:16:14.000000000","message":"This change is not related to the functionality, just to eliminate the circle import.","commit_id":"4fe7bd27ffb030b976a46ff1e77e12901648145e"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"41b9a560ea61a5cca9c148f1b2cd24167cd2cae6","unresolved":false,"context_lines":[{"line_number":398,"context_line":"                self._log_heartbeat(agent_state, agent_db, configurations_dict)"},{"line_number":399,"context_line":"                status \u003d n_const.AGENT_NEW"},{"line_number":400,"context_line":"            greenthread.sleep(0)"},{"line_number":401,"context_line":"            registry.notify(resources.AGENT, event_type, self, context\u003dcontext,"},{"line_number":402,"context_line":"                            host\u003dagent_state[\u0027host\u0027], plugin\u003dself,"},{"line_number":403,"context_line":"                            agent\u003dagent_state)"},{"line_number":404,"context_line":"        return status"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    def create_or_update_agent(self, context, agent):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_4661c4d2","line":403,"range":{"start_line":401,"start_character":12,"end_line":403,"end_character":46},"updated":"2016-06-07 21:06:23.000000000","message":"I think this change might make sense.  Would you mind breaking it out in to a patch on which this one depends?","commit_id":"30910c6f978986af70e2eb6fb4be3b01b466f6c3"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ece46ac5aab233863397d33d106b5369f77550bb","unresolved":false,"context_lines":[{"line_number":398,"context_line":"                self._log_heartbeat(agent_state, agent_db, configurations_dict)"},{"line_number":399,"context_line":"                status \u003d n_const.AGENT_NEW"},{"line_number":400,"context_line":"            greenthread.sleep(0)"},{"line_number":401,"context_line":"            registry.notify(resources.AGENT, event_type, self, context\u003dcontext,"},{"line_number":402,"context_line":"                            host\u003dagent_state[\u0027host\u0027], plugin\u003dself,"},{"line_number":403,"context_line":"                            agent\u003dagent_state)"},{"line_number":404,"context_line":"        return status"},{"line_number":405,"context_line":""},{"line_number":406,"context_line":"    def create_or_update_agent(self, context, agent):"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_1898b477","line":403,"range":{"start_line":401,"start_character":12,"end_line":403,"end_character":46},"in_reply_to":"7aa08908_4661c4d2","updated":"2016-06-08 02:17:21.000000000","message":"Will do it in next patch.","commit_id":"30910c6f978986af70e2eb6fb4be3b01b466f6c3"}],"neutron/db/segments_db.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"92eb213b2a1f21e3bb30c4a9c16e4fda395afdf3","unresolved":false,"context_lines":[{"line_number":151,"context_line":"def _prevent_segment_delete_with_subnet_associated(resource, event,"},{"line_number":152,"context_line":"                                                   trigger, **kwargs):"},{"line_number":153,"context_line":"    \"\"\"Raise exception if there are any subnets associated with segment_id.\"\"\""},{"line_number":154,"context_line":"    context \u003d kwargs[\u0027context\u0027]"},{"line_number":155,"context_line":"    segment_id \u003d kwargs[\u0027segment_id\u0027]"},{"line_number":156,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":157,"context_line":"        query \u003d (context.session.query(models_v2.Subnet.id)."}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_f72f2f9f","line":154,"range":{"start_line":154,"start_character":4,"end_line":154,"end_character":11},"updated":"2016-05-27 22:26:39.000000000","message":"Why not add context and segment_id to the arguments directly?","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ef0381be3f3da724d70e71103d0f95c8dfec2a23","unresolved":false,"context_lines":[{"line_number":151,"context_line":"def _prevent_segment_delete_with_subnet_associated(resource, event,"},{"line_number":152,"context_line":"                                                   trigger, **kwargs):"},{"line_number":153,"context_line":"    \"\"\"Raise exception if there are any subnets associated with segment_id.\"\"\""},{"line_number":154,"context_line":"    context \u003d kwargs[\u0027context\u0027]"},{"line_number":155,"context_line":"    segment_id \u003d kwargs[\u0027segment_id\u0027]"},{"line_number":156,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":157,"context_line":"        query \u003d (context.session.query(models_v2.Subnet.id)."}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_496db297","line":154,"range":{"start_line":154,"start_character":4,"end_line":154,"end_character":11},"in_reply_to":"9abb7d3a_f72f2f9f","updated":"2016-05-29 09:11:48.000000000","message":"Done","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"92eb213b2a1f21e3bb30c4a9c16e4fda395afdf3","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                 filter(models_v2.Subnet.segment_id \u003d\u003d segment_id))"},{"line_number":159,"context_line":"        subnet_ids \u003d [s[0] for s in query]"},{"line_number":160,"context_line":"        if subnet_ids:"},{"line_number":161,"context_line":"            LOG.error(_LE(\"Trying to delete a segment %(segment_id)s which \""},{"line_number":162,"context_line":"                          \"associates with subnets %(subnets)s\"),"},{"line_number":163,"context_line":"                      {\u0027segment_id\u0027: segment_id,"},{"line_number":164,"context_line":"                       \u0027subnets\u0027: \", \".join(subnet_ids)})"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_aa689ebb","line":161,"range":{"start_line":161,"start_character":12,"end_line":161,"end_character":21},"updated":"2016-05-27 22:26:39.000000000","message":"Don\u0027t log this as an error.  I wouldn\u0027t log it at all but if you want to, I\u0027d do it at debug level.\n\nThe reason is that this is a situation that should be taken care of by the API caller when they received the error.  This is different than the admin reacting to errors seen in logs.","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ef0381be3f3da724d70e71103d0f95c8dfec2a23","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                 filter(models_v2.Subnet.segment_id \u003d\u003d segment_id))"},{"line_number":159,"context_line":"        subnet_ids \u003d [s[0] for s in query]"},{"line_number":160,"context_line":"        if subnet_ids:"},{"line_number":161,"context_line":"            LOG.error(_LE(\"Trying to delete a segment %(segment_id)s which \""},{"line_number":162,"context_line":"                          \"associates with subnets %(subnets)s\"),"},{"line_number":163,"context_line":"                      {\u0027segment_id\u0027: segment_id,"},{"line_number":164,"context_line":"                       \u0027subnets\u0027: \", \".join(subnet_ids)})"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_296aee80","line":161,"range":{"start_line":161,"start_character":12,"end_line":161,"end_character":21},"in_reply_to":"9abb7d3a_aa689ebb","updated":"2016-05-29 09:11:48.000000000","message":"Done","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"41803a94e313f31a0a1c2ab58aee497e654a10b6","unresolved":false,"context_lines":[{"line_number":162,"context_line":"                          \"associates with subnets %(subnets)s\"),"},{"line_number":163,"context_line":"                      {\u0027segment_id\u0027: segment_id,"},{"line_number":164,"context_line":"                       \u0027subnets\u0027: \", \".join(subnet_ids)})"},{"line_number":165,"context_line":"            reason \u003d _(\"The segment is still associated with subnet(s)\")"},{"line_number":166,"context_line":"            raise exceptions.SegmentInUse(segment_id\u003dsegment_id, reason\u003dreason)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"bab6814e_6c8386cd","line":165,"updated":"2016-05-18 11:43:22.000000000","message":"can you also add the actual subnets here like above?","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ef0381be3f3da724d70e71103d0f95c8dfec2a23","unresolved":false,"context_lines":[{"line_number":162,"context_line":"                          \"associates with subnets %(subnets)s\"),"},{"line_number":163,"context_line":"                      {\u0027segment_id\u0027: segment_id,"},{"line_number":164,"context_line":"                       \u0027subnets\u0027: \", \".join(subnet_ids)})"},{"line_number":165,"context_line":"            reason \u003d _(\"The segment is still associated with subnet(s)\")"},{"line_number":166,"context_line":"            raise exceptions.SegmentInUse(segment_id\u003dsegment_id, reason\u003dreason)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_690a36d4","line":165,"in_reply_to":"9abb7d3a_4a81324a","updated":"2016-05-29 09:11:48.000000000","message":"I will include the subnets in the exception.","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"f83cee03927da7295d17a0489d222f71150c82c3","unresolved":false,"context_lines":[{"line_number":162,"context_line":"                          \"associates with subnets %(subnets)s\"),"},{"line_number":163,"context_line":"                      {\u0027segment_id\u0027: segment_id,"},{"line_number":164,"context_line":"                       \u0027subnets\u0027: \", \".join(subnet_ids)})"},{"line_number":165,"context_line":"            reason \u003d _(\"The segment is still associated with subnet(s)\")"},{"line_number":166,"context_line":"            raise exceptions.SegmentInUse(segment_id\u003dsegment_id, reason\u003dreason)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"bab6814e_fd74139d","line":165,"in_reply_to":"bab6814e_6c8386cd","updated":"2016-05-19 05:03:18.000000000","message":"In PS1, I added the subnets here. But this message will be seen by the cli/rest response, and when the subnets are too many, the output will be mess. So I changed to a simple exception here with a detailed error log. If the subnets are expected in the output of cli/rest response, I can combine these 2 messages into 1 again.","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"92eb213b2a1f21e3bb30c4a9c16e4fda395afdf3","unresolved":false,"context_lines":[{"line_number":162,"context_line":"                          \"associates with subnets %(subnets)s\"),"},{"line_number":163,"context_line":"                      {\u0027segment_id\u0027: segment_id,"},{"line_number":164,"context_line":"                       \u0027subnets\u0027: \", \".join(subnet_ids)})"},{"line_number":165,"context_line":"            reason \u003d _(\"The segment is still associated with subnet(s)\")"},{"line_number":166,"context_line":"            raise exceptions.SegmentInUse(segment_id\u003dsegment_id, reason\u003dreason)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_4a81324a","line":165,"in_reply_to":"bab6814e_fd74139d","updated":"2016-05-27 22:26:39.000000000","message":"It might be nice to have.  But, without it, they can list subnets on a segment with an API call like this:\n\n  GET http://{{api}}/v2.0/subnets?segment_id\u003d{{segment_id}}\n\nI think there is a CLI equivalent but I don\u0027t know it.","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"41b9a560ea61a5cca9c148f1b2cd24167cd2cae6","unresolved":false,"context_lines":[{"line_number":167,"context_line":""},{"line_number":168,"context_line":"def subscribe():"},{"line_number":169,"context_line":"    registry.subscribe("},{"line_number":170,"context_line":"        _prevent_segment_delete_with_subnet_associated, resources.SEGMENT,"},{"line_number":171,"context_line":"        events.BEFORE_DELETE)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"subscribe()"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_66896075","line":170,"range":{"start_line":170,"start_character":56,"end_line":170,"end_character":73},"updated":"2016-06-07 21:06:23.000000000","message":"nit:  This might just my personal preference, but how would you feel about putting each argument on its own line?","commit_id":"30910c6f978986af70e2eb6fb4be3b01b466f6c3"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ece46ac5aab233863397d33d106b5369f77550bb","unresolved":false,"context_lines":[{"line_number":167,"context_line":""},{"line_number":168,"context_line":"def subscribe():"},{"line_number":169,"context_line":"    registry.subscribe("},{"line_number":170,"context_line":"        _prevent_segment_delete_with_subnet_associated, resources.SEGMENT,"},{"line_number":171,"context_line":"        events.BEFORE_DELETE)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"subscribe()"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_78961840","line":170,"range":{"start_line":170,"start_character":56,"end_line":170,"end_character":73},"in_reply_to":"7aa08908_66896075","updated":"2016-06-08 02:17:21.000000000","message":"will update it","commit_id":"30910c6f978986af70e2eb6fb4be3b01b466f6c3"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"73b6ea8f547ef738925a611ec8502a50c59369fd","unresolved":false,"context_lines":[{"line_number":155,"context_line":"                                                   context, segment):"},{"line_number":156,"context_line":"    \"\"\"Raise exception if there are any subnets associated with segment_id.\"\"\""},{"line_number":157,"context_line":"    segment_id \u003d segment[\u0027id\u0027]"},{"line_number":158,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":159,"context_line":"        query \u003d (context.session.query(models_v2.Subnet.id)."},{"line_number":160,"context_line":"                 filter(models_v2.Subnet.segment_id \u003d\u003d segment_id))"},{"line_number":161,"context_line":"        subnet_ids \u003d [s[0] for s in query]"}],"source_content_type":"text/x-python","patch_set":7,"id":"3aaa91ec_51cff97e","line":158,"updated":"2016-06-23 01:24:46.000000000","message":"i don\u0027t think the transaction is accomplishing anything at all here. it\u0027s a single query with no mutating operations...","commit_id":"f9e02f7e71352de90d69165b25dccdde24c392e1"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"4bf98d19d86e85d3b4f1c126685e0787d7ad8d53","unresolved":false,"context_lines":[{"line_number":155,"context_line":"                                                   context, segment):"},{"line_number":156,"context_line":"    \"\"\"Raise exception if there are any subnets associated with segment_id.\"\"\""},{"line_number":157,"context_line":"    segment_id \u003d segment[\u0027id\u0027]"},{"line_number":158,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":159,"context_line":"        query \u003d (context.session.query(models_v2.Subnet.id)."},{"line_number":160,"context_line":"                 filter(models_v2.Subnet.segment_id \u003d\u003d segment_id))"},{"line_number":161,"context_line":"        subnet_ids \u003d [s[0] for s in query]"}],"source_content_type":"text/x-python","patch_set":7,"id":"3aaa91ec_6ebe8fd5","line":158,"in_reply_to":"3aaa91ec_51cff97e","updated":"2016-06-24 06:29:56.000000000","message":"Thanks, you are right, will remove it.","commit_id":"f9e02f7e71352de90d69165b25dccdde24c392e1"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"03a791de2d068b2b26a8008d2899d41a2ab188f3","unresolved":false,"context_lines":[{"line_number":155,"context_line":"                                                   context, segment):"},{"line_number":156,"context_line":"    \"\"\"Raise exception if there are any subnets associated with segment_id.\"\"\""},{"line_number":157,"context_line":"    segment_id \u003d segment[\u0027id\u0027]"},{"line_number":158,"context_line":"    query \u003d (context.session.query(models_v2.Subnet.id)."},{"line_number":159,"context_line":"             filter(models_v2.Subnet.segment_id \u003d\u003d segment_id))"},{"line_number":160,"context_line":"    subnet_ids \u003d [s[0] for s in query]"},{"line_number":161,"context_line":"    if subnet_ids:"}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_23d6b0c5","line":158,"updated":"2016-07-03 06:47:09.000000000","message":"do we need the context to be elevated here - for example the admin created a subnet on the tenant network","commit_id":"ca8c79f97c9e2c7f45d8c3fa97fad8e792f0d192"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"882a60160c6ff68920edde2d32cf715131dab482","unresolved":false,"context_lines":[{"line_number":155,"context_line":"                                                   context, segment):"},{"line_number":156,"context_line":"    \"\"\"Raise exception if there are any subnets associated with segment_id.\"\"\""},{"line_number":157,"context_line":"    segment_id \u003d segment[\u0027id\u0027]"},{"line_number":158,"context_line":"    query \u003d (context.session.query(models_v2.Subnet.id)."},{"line_number":159,"context_line":"             filter(models_v2.Subnet.segment_id \u003d\u003d segment_id))"},{"line_number":160,"context_line":"    subnet_ids \u003d [s[0] for s in query]"},{"line_number":161,"context_line":"    if subnet_ids:"}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_1fb65cdc","line":158,"in_reply_to":"1aa78d24_23d6b0c5","updated":"2016-07-04 03:20:41.000000000","message":"I don\u0027t think we need to elevate the context here, because the code here is for segment deletion, which is an admin only operation. So, the context here will be admin context.\n\nhttps://github.com/openstack/neutron/blob/b05d2f6fb52480f3d3f9c64fbacf6ad9d3cea094/etc/policy.json#L69","commit_id":"ca8c79f97c9e2c7f45d8c3fa97fad8e792f0d192"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"e38d9ed47a20ba8883df1864ab2610970379b722","unresolved":false,"context_lines":[{"line_number":160,"context_line":"                                                   context, segment):"},{"line_number":161,"context_line":"    \"\"\"Raise exception if there are any subnets associated with segment_id.\"\"\""},{"line_number":162,"context_line":"    segment_id \u003d segment[\u0027id\u0027]"},{"line_number":163,"context_line":"    query \u003d (context.session.query(models_v2.Subnet.id)."},{"line_number":164,"context_line":"             filter(models_v2.Subnet.segment_id \u003d\u003d segment_id))"},{"line_number":165,"context_line":"    subnet_ids \u003d [s[0] for s in query]"},{"line_number":166,"context_line":"    if subnet_ids:"},{"line_number":167,"context_line":"        reason \u003d _(\"The segment is still associated with subnet(s) \""}],"source_content_type":"text/x-python","patch_set":22,"id":"3ac371cc_2f25e6ad","line":164,"range":{"start_line":163,"start_character":0,"end_line":164,"end_character":63},"updated":"2016-08-19 10:28:49.000000000","message":"nit: I prefer to split this to avoid the parentheses:\n\nquery \u003d context.session.query(models_v2.Subnet.id)\nquery \u003d query.filter(models_v2.Subnet.segment_id \u003d\u003d segment_id)","commit_id":"8b4b206c2d3bb8679a7b83e26b66561b5c17c308"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"95905a7b096aee55950f42f1f7300c12d2dbf37e","unresolved":false,"context_lines":[{"line_number":160,"context_line":"                                                   context, segment):"},{"line_number":161,"context_line":"    \"\"\"Raise exception if there are any subnets associated with segment_id.\"\"\""},{"line_number":162,"context_line":"    segment_id \u003d segment[\u0027id\u0027]"},{"line_number":163,"context_line":"    query \u003d (context.session.query(models_v2.Subnet.id)."},{"line_number":164,"context_line":"             filter(models_v2.Subnet.segment_id \u003d\u003d segment_id))"},{"line_number":165,"context_line":"    subnet_ids \u003d [s[0] for s in query]"},{"line_number":166,"context_line":"    if subnet_ids:"},{"line_number":167,"context_line":"        reason \u003d _(\"The segment is still associated with subnet(s) \""}],"source_content_type":"text/x-python","patch_set":22,"id":"1ac06dbe_08de156d","line":164,"range":{"start_line":163,"start_character":0,"end_line":164,"end_character":63},"in_reply_to":"3ac371cc_2f25e6ad","updated":"2016-08-20 06:12:34.000000000","message":"Done","commit_id":"8b4b206c2d3bb8679a7b83e26b66561b5c17c308"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"9fb58bc82a479447b487c3920b145e3861a96d16","unresolved":false,"context_lines":[{"line_number":174,"context_line":"                       resources.SEGMENT,"},{"line_number":175,"context_line":"                       events.BEFORE_DELETE)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"subscribe()"}],"source_content_type":"text/x-python","patch_set":26,"id":"fa7ab95a_2aa00390","line":177,"updated":"2016-08-28 04:40:38.000000000","message":"Is this something we really want to do if the segment plugin is not loaded? If not, maybe this subscribe call should be moved to the segments plugin __init__.","commit_id":"b7dc55e6fdf21450124fbd640500e5626b847765"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"3f496fdb5d52a534736c2350bc5f5a1a860efe38","unresolved":false,"context_lines":[{"line_number":174,"context_line":"                       resources.SEGMENT,"},{"line_number":175,"context_line":"                       events.BEFORE_DELETE)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"subscribe()"}],"source_content_type":"text/x-python","patch_set":26,"id":"fa7ab95a_6ac12b2e","line":177,"in_reply_to":"fa7ab95a_2aa00390","updated":"2016-08-28 04:50:52.000000000","message":"I think you are right. If segment plugin is not loaded, there is no way to delete a segment without clearing subnets\u0026ports in network first. And this will be a redundant code path.\n\nI will move it to segment plugin.","commit_id":"b7dc55e6fdf21450124fbd640500e5626b847765"}],"neutron/plugins/ml2/db.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"92eb213b2a1f21e3bb30c4a9c16e4fda395afdf3","unresolved":false,"context_lines":[{"line_number":314,"context_line":"def _prevent_segment_delete_with_port_bound(resource, event,"},{"line_number":315,"context_line":"                                            trigger, **kwargs):"},{"line_number":316,"context_line":"    \"\"\"Raise exception if there are any ports bound with segment_id.\"\"\""},{"line_number":317,"context_line":"    context \u003d kwargs[\u0027context\u0027]"},{"line_number":318,"context_line":"    segment_id \u003d kwargs[\u0027segment_id\u0027]"},{"line_number":319,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":320,"context_line":"        query \u003d context.session.query(models_v2.Port)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_2a2cae49","line":317,"range":{"start_line":317,"start_character":4,"end_line":317,"end_character":11},"updated":"2016-05-27 22:26:39.000000000","message":"Ditto (make an argument)","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ef0381be3f3da724d70e71103d0f95c8dfec2a23","unresolved":false,"context_lines":[{"line_number":314,"context_line":"def _prevent_segment_delete_with_port_bound(resource, event,"},{"line_number":315,"context_line":"                                            trigger, **kwargs):"},{"line_number":316,"context_line":"    \"\"\"Raise exception if there are any ports bound with segment_id.\"\"\""},{"line_number":317,"context_line":"    context \u003d kwargs[\u0027context\u0027]"},{"line_number":318,"context_line":"    segment_id \u003d kwargs[\u0027segment_id\u0027]"},{"line_number":319,"context_line":"    with context.session.begin(subtransactions\u003dTrue):"},{"line_number":320,"context_line":"        query \u003d context.session.query(models_v2.Port)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_e9f626d6","line":317,"range":{"start_line":317,"start_character":4,"end_line":317,"end_character":11},"in_reply_to":"9abb7d3a_2a2cae49","updated":"2016-05-29 09:11:48.000000000","message":"Done","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"92eb213b2a1f21e3bb30c4a9c16e4fda395afdf3","unresolved":false,"context_lines":[{"line_number":329,"context_line":"    # There are still some tenant-owned ports in the segment, segment should"},{"line_number":330,"context_line":"    # not be deleted."},{"line_number":331,"context_line":"    if not_auto_del_port_ids:"},{"line_number":332,"context_line":"        LOG.error(_LE(\"Trying to delete a segment %(segment_id)s which still \""},{"line_number":333,"context_line":"                      \"binds with ports %(ports)s\"),"},{"line_number":334,"context_line":"                  {\u0027segment_id\u0027: segment_id,"},{"line_number":335,"context_line":"                   \u0027ports\u0027: \", \".join(not_auto_del_port_ids)})"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_aac41e92","line":332,"range":{"start_line":332,"start_character":12,"end_line":332,"end_character":17},"updated":"2016-05-27 22:26:39.000000000","message":"ditto (don\u0027t log at error level)","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ef0381be3f3da724d70e71103d0f95c8dfec2a23","unresolved":false,"context_lines":[{"line_number":329,"context_line":"    # There are still some tenant-owned ports in the segment, segment should"},{"line_number":330,"context_line":"    # not be deleted."},{"line_number":331,"context_line":"    if not_auto_del_port_ids:"},{"line_number":332,"context_line":"        LOG.error(_LE(\"Trying to delete a segment %(segment_id)s which still \""},{"line_number":333,"context_line":"                      \"binds with ports %(ports)s\"),"},{"line_number":334,"context_line":"                  {\u0027segment_id\u0027: segment_id,"},{"line_number":335,"context_line":"                   \u0027ports\u0027: \", \".join(not_auto_del_port_ids)})"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_490492bf","line":332,"range":{"start_line":332,"start_character":12,"end_line":332,"end_character":17},"in_reply_to":"9abb7d3a_aac41e92","updated":"2016-05-29 09:11:48.000000000","message":"Done","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"41803a94e313f31a0a1c2ab58aee497e654a10b6","unresolved":false,"context_lines":[{"line_number":333,"context_line":"                      \"binds with ports %(ports)s\"),"},{"line_number":334,"context_line":"                  {\u0027segment_id\u0027: segment_id,"},{"line_number":335,"context_line":"                   \u0027ports\u0027: \", \".join(not_auto_del_port_ids)})"},{"line_number":336,"context_line":"        reason \u003d _(\"The segment is still bound with port(s)\")"},{"line_number":337,"context_line":"        raise seg_exc.SegmentInUse(segment_id\u003dsegment_id, reason\u003dreason)"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"    # Delete the ports that can be auto-deleted."}],"source_content_type":"text/x-python","patch_set":2,"id":"bab6814e_6c5a6685","line":336,"updated":"2016-05-18 11:43:22.000000000","message":"same for the ports here","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"41b9a560ea61a5cca9c148f1b2cd24167cd2cae6","unresolved":false,"context_lines":[{"line_number":337,"context_line":"    port_ids \u003d [port.id for port in ports]"},{"line_number":338,"context_line":"    plugin \u003d manager.NeutronManager.get_plugin()"},{"line_number":339,"context_line":"    for port_id in port_ids:"},{"line_number":340,"context_line":"        plugin.delete_port(context, port_id)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"def subscribe():"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_a7e6b812","line":340,"range":{"start_line":340,"start_character":15,"end_line":340,"end_character":26},"updated":"2016-06-07 21:06:23.000000000","message":"Draft:  So, the question here is if this code needs to be smart about whether this is a routed network or not.","commit_id":"30910c6f978986af70e2eb6fb4be3b01b466f6c3"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"057dffabbc2b387932e2d8ab26e043a828bd99fd","unresolved":false,"context_lines":[{"line_number":337,"context_line":"    port_ids \u003d [port.id for port in ports]"},{"line_number":338,"context_line":"    plugin \u003d manager.NeutronManager.get_plugin()"},{"line_number":339,"context_line":"    for port_id in port_ids:"},{"line_number":340,"context_line":"        plugin.delete_port(context, port_id)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"def subscribe():"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_6b6f5d2b","line":340,"range":{"start_line":340,"start_character":15,"end_line":340,"end_character":26},"in_reply_to":"7aa08908_82670eb6","updated":"2016-06-08 04:18:20.000000000","message":"I give it a rethink, I think we could just stop deleting segment with any port(including dhcp port) first in this patch. Because there will be a bunch of things to be considered if we are going to delete the dhcp port automatically.\n1) The potential \u0027race\u0027 between checking port and checking subnet with segment. There is no guarantee who comes first. Checking port might come first, and the dhcp port is deleted, and then found it is a routed network, and there is a subnet using this segment. As a result, deleting segment is prevented, but the dhcp port is deleted.\n\n2) For a L2-adjacent network, deleting segment means the network will not cover that segment anymore. This might happen when the admin do a maintenance for part of the physical network. Admin can use  remove_network_from_dhcp_agent and add_network_to_dhcp_agent (as workaround) to move dhcp service to another available host. And then delete the segment from network.\n\n3) If we are going to do it automatically under the hood, it is more than just deleting a dhcp port. We should remove_network_from_dhcp_agent first, and then scheduler the network again. That might be out of the scope of this (ml2)patch.\n\nGiven the reasons above, I will remove the automatic deleting port code in next patch and add a REVISIT instead.","commit_id":"30910c6f978986af70e2eb6fb4be3b01b466f6c3"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ece46ac5aab233863397d33d106b5369f77550bb","unresolved":false,"context_lines":[{"line_number":337,"context_line":"    port_ids \u003d [port.id for port in ports]"},{"line_number":338,"context_line":"    plugin \u003d manager.NeutronManager.get_plugin()"},{"line_number":339,"context_line":"    for port_id in port_ids:"},{"line_number":340,"context_line":"        plugin.delete_port(context, port_id)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"def subscribe():"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_82670eb6","line":340,"range":{"start_line":340,"start_character":15,"end_line":340,"end_character":26},"in_reply_to":"7aa08908_a7e6b812","updated":"2016-06-08 02:17:21.000000000","message":"For a routed network, deleting segment will be prevented because the SegmentInUse exception at [1]. For a l2-adjacent network, if we prevent deleting the segment(s) that host the dhcp service, that will make the segment(s) hard to be deleted. If we allow deleting the segment(s), maybe we can reschedule the dhcp service to other host. But that will make things complicating.\n\nI think we have to choose either way.\n\n[1] https://review.openstack.org/#/c/317358/4/neutron/db/segments_db.py@154","commit_id":"30910c6f978986af70e2eb6fb4be3b01b466f6c3"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"eb5d7dff5e6f3b2970f667210a981abd80905a6a","unresolved":false,"context_lines":[{"line_number":320,"context_line":"            models.PortBindingLevel,"},{"line_number":321,"context_line":"            models.PortBindingLevel.port_id \u003d\u003d models_v2.Port.id)"},{"line_number":322,"context_line":"        query \u003d query.filter(models.PortBindingLevel.segment_id \u003d\u003d segment_id)"},{"line_number":323,"context_line":"        ports \u003d query.all()"},{"line_number":324,"context_line":"        port_ids \u003d [p.id for p in ports]"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"    # There are still some ports in the segment, segment should not be deleted"}],"source_content_type":"text/x-python","patch_set":6,"id":"7aa08908_0347121e","line":323,"range":{"start_line":323,"start_character":8,"end_line":323,"end_character":27},"updated":"2016-06-15 23:37:36.000000000","message":"You don\u0027t need this line:\n\n  port_ids \u003d [port.id for port in query]","commit_id":"f49cfef4891eba16e8bbd174d2e42ee6403ffdcc"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"36a2d8d81df50beb31ae9f7a2b469939465ad59a","unresolved":false,"context_lines":[{"line_number":320,"context_line":"            models.PortBindingLevel,"},{"line_number":321,"context_line":"            models.PortBindingLevel.port_id \u003d\u003d models_v2.Port.id)"},{"line_number":322,"context_line":"        query \u003d query.filter(models.PortBindingLevel.segment_id \u003d\u003d segment_id)"},{"line_number":323,"context_line":"        ports \u003d query.all()"},{"line_number":324,"context_line":"        port_ids \u003d [p.id for p in ports]"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"    # There are still some ports in the segment, segment should not be deleted"}],"source_content_type":"text/x-python","patch_set":6,"id":"7aa08908_ddd84282","line":323,"range":{"start_line":323,"start_character":8,"end_line":323,"end_character":27},"in_reply_to":"7aa08908_0347121e","updated":"2016-06-16 07:19:53.000000000","message":"Done","commit_id":"f49cfef4891eba16e8bbd174d2e42ee6403ffdcc"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"eb5d7dff5e6f3b2970f667210a981abd80905a6a","unresolved":false,"context_lines":[{"line_number":324,"context_line":"        port_ids \u003d [p.id for p in ports]"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"    # There are still some ports in the segment, segment should not be deleted"},{"line_number":327,"context_line":"    # REVISIT(xiaohhui): Should we delete the dhcp port automatically here?"},{"line_number":328,"context_line":"    if port_ids:"},{"line_number":329,"context_line":"        reason \u003d _(\"The segment is still bound with port(s) \""},{"line_number":330,"context_line":"                   \"%s\") % \", \".join(port_ids)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7aa08908_43e3da33","line":327,"range":{"start_line":327,"start_character":6,"end_line":327,"end_character":13},"updated":"2016-06-15 23:37:36.000000000","message":"TODO is customary here instead of REVISIT.","commit_id":"f49cfef4891eba16e8bbd174d2e42ee6403ffdcc"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"36a2d8d81df50beb31ae9f7a2b469939465ad59a","unresolved":false,"context_lines":[{"line_number":324,"context_line":"        port_ids \u003d [p.id for p in ports]"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"    # There are still some ports in the segment, segment should not be deleted"},{"line_number":327,"context_line":"    # REVISIT(xiaohhui): Should we delete the dhcp port automatically here?"},{"line_number":328,"context_line":"    if port_ids:"},{"line_number":329,"context_line":"        reason \u003d _(\"The segment is still bound with port(s) \""},{"line_number":330,"context_line":"                   \"%s\") % \", \".join(port_ids)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7aa08908_1de3ea34","line":327,"range":{"start_line":327,"start_character":6,"end_line":327,"end_character":13},"in_reply_to":"7aa08908_43e3da33","updated":"2016-06-16 07:19:53.000000000","message":"Done","commit_id":"f49cfef4891eba16e8bbd174d2e42ee6403ffdcc"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"73b6ea8f547ef738925a611ec8502a50c59369fd","unresolved":false,"context_lines":[{"line_number":320,"context_line":"            models.PortBindingLevel,"},{"line_number":321,"context_line":"            models.PortBindingLevel.port_id \u003d\u003d models_v2.Port.id)"},{"line_number":322,"context_line":"        query \u003d query.filter(models.PortBindingLevel.segment_id \u003d\u003d segment_id)"},{"line_number":323,"context_line":"        port_ids \u003d [p.id for p in query]"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"    # There are still some ports in the segment, segment should not be deleted"},{"line_number":326,"context_line":"    # TODO(xiaohhui): Should we delete the dhcp port automatically here?"}],"source_content_type":"text/x-python","patch_set":7,"id":"3aaa91ec_31ca2d6f","line":323,"updated":"2016-06-23 01:24:46.000000000","message":"don\u0027t enumerate the query if we don\u0027t actually need the data. we just need the count. Get rid of L323 and make L327 \u0027if query.count()\u0027","commit_id":"f9e02f7e71352de90d69165b25dccdde24c392e1"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"4bf98d19d86e85d3b4f1c126685e0787d7ad8d53","unresolved":false,"context_lines":[{"line_number":320,"context_line":"            models.PortBindingLevel,"},{"line_number":321,"context_line":"            models.PortBindingLevel.port_id \u003d\u003d models_v2.Port.id)"},{"line_number":322,"context_line":"        query \u003d query.filter(models.PortBindingLevel.segment_id \u003d\u003d segment_id)"},{"line_number":323,"context_line":"        port_ids \u003d [p.id for p in query]"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"    # There are still some ports in the segment, segment should not be deleted"},{"line_number":326,"context_line":"    # TODO(xiaohhui): Should we delete the dhcp port automatically here?"}],"source_content_type":"text/x-python","patch_set":7,"id":"3aaa91ec_eef55fcb","line":323,"in_reply_to":"3aaa91ec_31ca2d6f","updated":"2016-06-24 06:29:56.000000000","message":"I want to add port ids in the exception(line 329), so that the end user can see exactly what is happening. Or else, end user will need to check port one by one to find out which ones prevent the segment from being deleted.\n\nFor that reason, I would reserve this iteration here.","commit_id":"f9e02f7e71352de90d69165b25dccdde24c392e1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e8f523cfc46b985350c4f215ba8615714ceb288c","unresolved":false,"context_lines":[{"line_number":320,"context_line":"            models.PortBindingLevel,"},{"line_number":321,"context_line":"            models.PortBindingLevel.port_id \u003d\u003d models_v2.Port.id)"},{"line_number":322,"context_line":"        query \u003d query.filter(models.PortBindingLevel.segment_id \u003d\u003d segment_id)"},{"line_number":323,"context_line":"        port_ids \u003d [p.id for p in query]"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"    # There are still some ports in the segment, segment should not be deleted"},{"line_number":326,"context_line":"    # TODO(xiaohhui): Should we delete the dhcp port automatically here?"}],"source_content_type":"text/x-python","patch_set":7,"id":"3aaa91ec_11129162","line":323,"in_reply_to":"3aaa91ec_eef55fcb","updated":"2016-07-08 19:30:56.000000000","message":"I had thought about providing the same feedback but noticed that port_ids was included in the exception.  I didn\u0027t feel strongly enough to ask for this change.\n\nI suppose we could defer enumeration until just before raising the exception.  That would cost an extra DB round-trip in the exceptional case.  I don\u0027t feel strongly either way.","commit_id":"f9e02f7e71352de90d69165b25dccdde24c392e1"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"03a791de2d068b2b26a8008d2899d41a2ab188f3","unresolved":false,"context_lines":[{"line_number":314,"context_line":"                                            context, segment):"},{"line_number":315,"context_line":"    \"\"\"Raise exception if there are any ports bound with segment_id.\"\"\""},{"line_number":316,"context_line":"    segment_id \u003d segment[\u0027id\u0027]"},{"line_number":317,"context_line":"    query \u003d context.session.query(models_v2.Port)"},{"line_number":318,"context_line":"    query \u003d query.join("},{"line_number":319,"context_line":"        models.PortBindingLevel,"},{"line_number":320,"context_line":"        models.PortBindingLevel.port_id \u003d\u003d models_v2.Port.id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_43d13ccd","line":317,"updated":"2016-07-03 06:47:09.000000000","message":"do we need the context to be elevated here - for example and admin created a port on the network","commit_id":"ca8c79f97c9e2c7f45d8c3fa97fad8e792f0d192"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"882a60160c6ff68920edde2d32cf715131dab482","unresolved":false,"context_lines":[{"line_number":314,"context_line":"                                            context, segment):"},{"line_number":315,"context_line":"    \"\"\"Raise exception if there are any ports bound with segment_id.\"\"\""},{"line_number":316,"context_line":"    segment_id \u003d segment[\u0027id\u0027]"},{"line_number":317,"context_line":"    query \u003d context.session.query(models_v2.Port)"},{"line_number":318,"context_line":"    query \u003d query.join("},{"line_number":319,"context_line":"        models.PortBindingLevel,"},{"line_number":320,"context_line":"        models.PortBindingLevel.port_id \u003d\u003d models_v2.Port.id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_3fb518cd","line":317,"in_reply_to":"1aa78d24_43d13ccd","updated":"2016-07-04 03:20:41.000000000","message":"The same as last comment.","commit_id":"ca8c79f97c9e2c7f45d8c3fa97fad8e792f0d192"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"e38d9ed47a20ba8883df1864ab2610970379b722","unresolved":false,"context_lines":[{"line_number":319,"context_line":"    query \u003d query.join("},{"line_number":320,"context_line":"        models.PortBindingLevel,"},{"line_number":321,"context_line":"        models.PortBindingLevel.port_id \u003d\u003d models_v2.Port.id)"},{"line_number":322,"context_line":"    query \u003d query.filter(models.PortBindingLevel.segment_id \u003d\u003d segment_id)"},{"line_number":323,"context_line":"    port_ids \u003d [p.id for p in query]"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"    # There are still some ports in the segment, segment should not be deleted"}],"source_content_type":"text/x-python","patch_set":22,"id":"3ac371cc_d25155da","line":322,"updated":"2016-08-19 10:28:49.000000000","message":"You can collapse this into the join:\n\nquery \u003d query.join(\n        models.PortBindingLevel,\n        and_(models.PortBindingLevel.port_id \u003d\u003d models_v2.Port.id,\n             models.PortBindingLevel.segment_id \u003d\u003d segment_id))","commit_id":"8b4b206c2d3bb8679a7b83e26b66561b5c17c308"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"e6ffe570c211076796877a2d5428614ae4a4af21","unresolved":false,"context_lines":[{"line_number":319,"context_line":"    query \u003d query.join("},{"line_number":320,"context_line":"        models.PortBindingLevel,"},{"line_number":321,"context_line":"        models.PortBindingLevel.port_id \u003d\u003d models_v2.Port.id)"},{"line_number":322,"context_line":"    query \u003d query.filter(models.PortBindingLevel.segment_id \u003d\u003d segment_id)"},{"line_number":323,"context_line":"    port_ids \u003d [p.id for p in query]"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"    # There are still some ports in the segment, segment should not be deleted"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ac06dbe_7ecb1e73","line":322,"in_reply_to":"1ac06dbe_2f121763","updated":"2016-08-19 15:36:24.000000000","message":"I don\u0027t think this will have much (or any) effect on the generated SQL, but I don\u0027t think the correct distinction between join and filter conditions is being made here. The end result is that we want to include PortBindingLevel entries only if they have both a matching port_id and a matching segment_id, so why include those without a matching segment_id in the join to begin with? Why not join based on the segment_id and then filter on port_id? Why use a join condition at all, and just filter for both conditions? In my opinion, a filter should be used here only for the purpose of excluding Port entries, since this can\u0027t be done in the left-hand join.","commit_id":"8b4b206c2d3bb8679a7b83e26b66561b5c17c308"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"95905a7b096aee55950f42f1f7300c12d2dbf37e","unresolved":false,"context_lines":[{"line_number":319,"context_line":"    query \u003d query.join("},{"line_number":320,"context_line":"        models.PortBindingLevel,"},{"line_number":321,"context_line":"        models.PortBindingLevel.port_id \u003d\u003d models_v2.Port.id)"},{"line_number":322,"context_line":"    query \u003d query.filter(models.PortBindingLevel.segment_id \u003d\u003d segment_id)"},{"line_number":323,"context_line":"    port_ids \u003d [p.id for p in query]"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"    # There are still some ports in the segment, segment should not be deleted"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ac06dbe_e81d7910","line":322,"in_reply_to":"1ac06dbe_7ecb1e73","updated":"2016-08-20 06:12:34.000000000","message":"I will use your suggestion here, as it looks more close to the actual logic here.  Thanks.","commit_id":"8b4b206c2d3bb8679a7b83e26b66561b5c17c308"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"dc105a9b2f9d16af2c6fa8e3ed06be70ba556632","unresolved":false,"context_lines":[{"line_number":319,"context_line":"    query \u003d query.join("},{"line_number":320,"context_line":"        models.PortBindingLevel,"},{"line_number":321,"context_line":"        models.PortBindingLevel.port_id \u003d\u003d models_v2.Port.id)"},{"line_number":322,"context_line":"    query \u003d query.filter(models.PortBindingLevel.segment_id \u003d\u003d segment_id)"},{"line_number":323,"context_line":"    port_ids \u003d [p.id for p in query]"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"    # There are still some ports in the segment, segment should not be deleted"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ac06dbe_dde574dc","line":322,"in_reply_to":"1ac06dbe_94f49787","updated":"2016-08-23 06:50:04.000000000","message":"Thanks Carl, I will revert to the previous way.","commit_id":"8b4b206c2d3bb8679a7b83e26b66561b5c17c308"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a3ec951c91f8367c8b2e27f9aab88624810fb098","unresolved":false,"context_lines":[{"line_number":319,"context_line":"    query \u003d query.join("},{"line_number":320,"context_line":"        models.PortBindingLevel,"},{"line_number":321,"context_line":"        models.PortBindingLevel.port_id \u003d\u003d models_v2.Port.id)"},{"line_number":322,"context_line":"    query \u003d query.filter(models.PortBindingLevel.segment_id \u003d\u003d segment_id)"},{"line_number":323,"context_line":"    port_ids \u003d [p.id for p in query]"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"    # There are still some ports in the segment, segment should not be deleted"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ac06dbe_94f49787","line":322,"in_reply_to":"1ac06dbe_aa5a144e","updated":"2016-08-22 17:17:47.000000000","message":"Actually, this review [1] made me change my mind about the -1. I think that my intuition about what belongs in a join is correct and isn\u0027t merely a cosmetic thing. It can actually make the query more difficult to reason about.\n\n [1] https://review.openstack.org/#/c/350613/6/neutron/db/ipam_backend_mixin.py","commit_id":"8b4b206c2d3bb8679a7b83e26b66561b5c17c308"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"08f46cc040b7dfedd5c2a5e5d6c632f38d07a534","unresolved":false,"context_lines":[{"line_number":319,"context_line":"    query \u003d query.join("},{"line_number":320,"context_line":"        models.PortBindingLevel,"},{"line_number":321,"context_line":"        models.PortBindingLevel.port_id \u003d\u003d models_v2.Port.id)"},{"line_number":322,"context_line":"    query \u003d query.filter(models.PortBindingLevel.segment_id \u003d\u003d segment_id)"},{"line_number":323,"context_line":"    port_ids \u003d [p.id for p in query]"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"    # There are still some ports in the segment, segment should not be deleted"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ac06dbe_aa5a144e","line":322,"in_reply_to":"1ac06dbe_e81d7910","updated":"2016-08-22 16:09:43.000000000","message":"Port rows aren\u0027t joined to portbindinglevel on segmentid. They are joined on port id alone. The segment_id is part of the data brought it by the join that you can filter on. I liked the way it was before because it reflected this more naturally, IMO. But, the db probably handles this in the same way underneath and it works fine. So, I won\u0027t -1.","commit_id":"8b4b206c2d3bb8679a7b83e26b66561b5c17c308"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"3f07dec6c2117b6e89f72e6eadd943ee7829777b","unresolved":false,"context_lines":[{"line_number":319,"context_line":"    query \u003d query.join("},{"line_number":320,"context_line":"        models.PortBindingLevel,"},{"line_number":321,"context_line":"        models.PortBindingLevel.port_id \u003d\u003d models_v2.Port.id)"},{"line_number":322,"context_line":"    query \u003d query.filter(models.PortBindingLevel.segment_id \u003d\u003d segment_id)"},{"line_number":323,"context_line":"    port_ids \u003d [p.id for p in query]"},{"line_number":324,"context_line":""},{"line_number":325,"context_line":"    # There are still some ports in the segment, segment should not be deleted"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ac06dbe_2f121763","line":322,"in_reply_to":"3ac371cc_d25155da","updated":"2016-08-19 15:16:53.000000000","message":"Does this change the SQL generated in a significant way? I prefer to think of it the way it is currently written with the join condition in the join and this being a filter condition. But, if there is a significant advantage to doing this, I\u0027m okay with it.","commit_id":"8b4b206c2d3bb8679a7b83e26b66561b5c17c308"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"9bb97564ad7f9fc0faf9fb628a8c9c677516c17a","unresolved":false,"context_lines":[{"line_number":320,"context_line":"    query \u003d query.join("},{"line_number":321,"context_line":"        models.PortBindingLevel,"},{"line_number":322,"context_line":"        and_(models.PortBindingLevel.port_id \u003d\u003d models_v2.Port.id,"},{"line_number":323,"context_line":"             models.PortBindingLevel.segment_id \u003d\u003d segment_id))"},{"line_number":324,"context_line":"    port_ids \u003d [p.id for p in query]"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":"    # There are still some ports in the segment, segment should not be deleted"}],"source_content_type":"text/x-python","patch_set":23,"id":"1ac06dbe_14642747","line":323,"range":{"start_line":323,"start_character":13,"end_line":323,"end_character":61},"updated":"2016-08-22 17:19:44.000000000","message":"See my latest comment in the previous patch set. I\u0027ve changed my opinion about this being a -1. I think there is good reason to properly separate join conditions from filter conditions.","commit_id":"2efe941d3ac53b1feca7acae1a4daf049918bf49"}],"neutron/plugins/ml2/managers.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"92eb213b2a1f21e3bb30c4a9c16e4fda395afdf3","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            raise exc.InvalidInput(error_message\u003dmsg)"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        segment \u003d {"},{"line_number":222,"context_line":"            api.NETWORK_TYPE: self._get_attribute("},{"line_number":223,"context_line":"                segment_data, api.NETWORK_TYPE),"},{"line_number":224,"context_line":"            api.PHYSICAL_NETWORK: self._get_attribute("},{"line_number":225,"context_line":"                segment_data, api.PHYSICAL_NETWORK),"},{"line_number":226,"context_line":"            api.SEGMENTATION_ID: self._get_attribute("}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_ea9f465d","line":223,"range":{"start_line":222,"start_character":30,"end_line":223,"end_character":47},"updated":"2016-05-27 22:26:39.000000000","message":"nit:  This might look cleaner if you capture these values in local vars:\n\n  type \u003d self._get_attribute(segment_data, api.NETWORK_TYPE)\n  phys_net \u003d self._get_attribute(segment_data, api.PHYSICAL_NETWORK)\n  seg_id \u003d self._get_attribute(segment_data, api.SEGMENTATION_ID)\n  segment \u003d {api.NETWORK_TYPE: type,\n             api.PHYSICAL_NETWORK: phys_net,\n             api.SEGMENTATION_ID: seg_id}","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ef0381be3f3da724d70e71103d0f95c8dfec2a23","unresolved":false,"context_lines":[{"line_number":219,"context_line":"            raise exc.InvalidInput(error_message\u003dmsg)"},{"line_number":220,"context_line":""},{"line_number":221,"context_line":"        segment \u003d {"},{"line_number":222,"context_line":"            api.NETWORK_TYPE: self._get_attribute("},{"line_number":223,"context_line":"                segment_data, api.NETWORK_TYPE),"},{"line_number":224,"context_line":"            api.PHYSICAL_NETWORK: self._get_attribute("},{"line_number":225,"context_line":"                segment_data, api.PHYSICAL_NETWORK),"},{"line_number":226,"context_line":"            api.SEGMENTATION_ID: self._get_attribute("}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_e91d8610","line":223,"range":{"start_line":222,"start_character":30,"end_line":223,"end_character":47},"in_reply_to":"9abb7d3a_ea9f465d","updated":"2016-05-29 09:11:48.000000000","message":"Done","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"92eb213b2a1f21e3bb30c4a9c16e4fda395afdf3","unresolved":false,"context_lines":[{"line_number":229,"context_line":""},{"line_number":230,"context_line":"        # Reserve segment in type driver"},{"line_number":231,"context_line":"        with session.begin(subtransactions\u003dTrue):"},{"line_number":232,"context_line":"            segment \u003d self.reserve_provider_segment(session, segment)"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        return segment"},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_eaad8689","line":232,"range":{"start_line":232,"start_character":12,"end_line":232,"end_character":21},"updated":"2016-05-27 22:26:39.000000000","message":"nit:  Just return self.reserve...","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ef0381be3f3da724d70e71103d0f95c8dfec2a23","unresolved":false,"context_lines":[{"line_number":229,"context_line":""},{"line_number":230,"context_line":"        # Reserve segment in type driver"},{"line_number":231,"context_line":"        with session.begin(subtransactions\u003dTrue):"},{"line_number":232,"context_line":"            segment \u003d self.reserve_provider_segment(session, segment)"},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        return segment"},{"line_number":235,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_491bf21b","line":232,"range":{"start_line":232,"start_character":12,"end_line":232,"end_character":21},"in_reply_to":"9abb7d3a_eaad8689","updated":"2016-05-29 09:11:48.000000000","message":"Done","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"03a791de2d068b2b26a8008d2899d41a2ab188f3","unresolved":false,"context_lines":[{"line_number":285,"context_line":"        if driver:"},{"line_number":286,"context_line":"            driver.obj.release_segment(session, segment)"},{"line_number":287,"context_line":"        else:"},{"line_number":288,"context_line":"            LOG.error(_LE(\"Failed to release segment \u0027%s\u0027 because \""},{"line_number":289,"context_line":"                          \"network type is not supported.\"), segment)"},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"    def allocate_dynamic_segment(self, session, network_id, segment):"}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_63d8b8f0","line":288,"updated":"2016-07-03 06:47:09.000000000","message":"any reason why we are not raising an exception here?","commit_id":"ca8c79f97c9e2c7f45d8c3fa97fad8e792f0d192"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"882a60160c6ff68920edde2d32cf715131dab482","unresolved":false,"context_lines":[{"line_number":285,"context_line":"        if driver:"},{"line_number":286,"context_line":"            driver.obj.release_segment(session, segment)"},{"line_number":287,"context_line":"        else:"},{"line_number":288,"context_line":"            LOG.error(_LE(\"Failed to release segment \u0027%s\u0027 because \""},{"line_number":289,"context_line":"                          \"network type is not supported.\"), segment)"},{"line_number":290,"context_line":""},{"line_number":291,"context_line":"    def allocate_dynamic_segment(self, session, network_id, segment):"}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_3c408656","line":288,"in_reply_to":"1aa78d24_63d8b8f0","updated":"2016-07-04 03:20:41.000000000","message":"I am OK with either way. I just don\u0027t want to change the original behavior here.","commit_id":"ca8c79f97c9e2c7f45d8c3fa97fad8e792f0d192"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"03a791de2d068b2b26a8008d2899d41a2ab188f3","unresolved":false,"context_lines":[{"line_number":320,"context_line":"    def get_mtu(self, segment):"},{"line_number":321,"context_line":"        \"\"\"Get the mtu of a segment.\"\"\""},{"line_number":322,"context_line":"        network_type \u003d segment.get(api.NETWORK_TYPE)"},{"line_number":323,"context_line":"        driver \u003d self.drivers.get(network_type)"},{"line_number":324,"context_line":"        return driver.obj.get_mtu(segment.get(api.PHYSICAL_NETWORK))"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_03ee148b","line":323,"updated":"2016-07-03 06:47:09.000000000","message":"what if the driver is None?","commit_id":"ca8c79f97c9e2c7f45d8c3fa97fad8e792f0d192"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"882a60160c6ff68920edde2d32cf715131dab482","unresolved":false,"context_lines":[{"line_number":320,"context_line":"    def get_mtu(self, segment):"},{"line_number":321,"context_line":"        \"\"\"Get the mtu of a segment.\"\"\""},{"line_number":322,"context_line":"        network_type \u003d segment.get(api.NETWORK_TYPE)"},{"line_number":323,"context_line":"        driver \u003d self.drivers.get(network_type)"},{"line_number":324,"context_line":"        return driver.obj.get_mtu(segment.get(api.PHYSICAL_NETWORK))"},{"line_number":325,"context_line":""},{"line_number":326,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1aa78d24_bf1aa86c","line":323,"in_reply_to":"1aa78d24_03ee148b","updated":"2016-07-04 03:20:41.000000000","message":"Good catch, will update it in next patch.","commit_id":"ca8c79f97c9e2c7f45d8c3fa97fad8e792f0d192"}],"neutron/plugins/ml2/plugin.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"92eb213b2a1f21e3bb30c4a9c16e4fda395afdf3","unresolved":false,"context_lines":[{"line_number":1693,"context_line":"        segments \u003d segments_db.get_network_segments(session, network_id)"},{"line_number":1694,"context_line":"        for segment in segments:"},{"line_number":1695,"context_line":"            segment_mtu \u003d self.type_manager.get_mtu(segment)"},{"line_number":1696,"context_line":"            if isinstance(segment_mtu, int) and segment_mtu \u003e 0:"},{"line_number":1697,"context_line":"                mtu.append(segment_mtu)"},{"line_number":1698,"context_line":""},{"line_number":1699,"context_line":"        return mtu"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_eadbc619","line":1696,"range":{"start_line":1696,"start_character":39,"end_line":1696,"end_character":42},"updated":"2016-05-27 22:26:39.000000000","message":"I\u0027d avoid checking for integer types.  Is None the only other possibility?  If so, then I\u0027d check \"if segment_mtu is not None\" instead of doing this.","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ef0381be3f3da724d70e71103d0f95c8dfec2a23","unresolved":false,"context_lines":[{"line_number":1693,"context_line":"        segments \u003d segments_db.get_network_segments(session, network_id)"},{"line_number":1694,"context_line":"        for segment in segments:"},{"line_number":1695,"context_line":"            segment_mtu \u003d self.type_manager.get_mtu(segment)"},{"line_number":1696,"context_line":"            if isinstance(segment_mtu, int) and segment_mtu \u003e 0:"},{"line_number":1697,"context_line":"                mtu.append(segment_mtu)"},{"line_number":1698,"context_line":""},{"line_number":1699,"context_line":"        return mtu"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_89d31a38","line":1696,"range":{"start_line":1696,"start_character":39,"end_line":1696,"end_character":42},"in_reply_to":"9abb7d3a_eadbc619","updated":"2016-05-29 09:11:48.000000000","message":"yeah, None is the only other possibility, will change it in next patch.","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":6558,"name":"Sukhdev Kapur","email":"sukhdevkapur@gmail.com","username":"sukhdev-8"},"change_message_id":"dde80acb8975ecce79cf9c2981388a14f5924eed","unresolved":false,"context_lines":[{"line_number":1727,"context_line":"            mech_context \u003d driver_context.NetworkContext("},{"line_number":1728,"context_line":"                self, context, updated_network,"},{"line_number":1729,"context_line":"                original_network\u003dnetwork)"},{"line_number":1730,"context_line":"            self.mechanism_manager.update_network_precommit(mech_context)"},{"line_number":1731,"context_line":""},{"line_number":1732,"context_line":"        self.mechanism_manager.update_network_postcommit(mech_context)"},{"line_number":1733,"context_line":"        segment[api.SEGMENTATION_ID] \u003d updated_segment[api.SEGMENTATION_ID]"}],"source_content_type":"text/x-python","patch_set":5,"id":"7aa08908_115d2a79","line":1730,"updated":"2016-06-08 05:44:31.000000000","message":"Would you not want to deal with the condition if update_network_precommit() fails? Shouldn\u0027t we rollback the transaction?\nIf not rollback, do you still want to proceed with the postcommit?","commit_id":"afb3cc23f2ece297fe480835d031a1e1e69c784a"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"99d94beb61063520d9e08c78348c5dc97d4c5e7f","unresolved":false,"context_lines":[{"line_number":1727,"context_line":"            mech_context \u003d driver_context.NetworkContext("},{"line_number":1728,"context_line":"                self, context, updated_network,"},{"line_number":1729,"context_line":"                original_network\u003dnetwork)"},{"line_number":1730,"context_line":"            self.mechanism_manager.update_network_precommit(mech_context)"},{"line_number":1731,"context_line":""},{"line_number":1732,"context_line":"        self.mechanism_manager.update_network_postcommit(mech_context)"},{"line_number":1733,"context_line":"        segment[api.SEGMENTATION_ID] \u003d updated_segment[api.SEGMENTATION_ID]"}],"source_content_type":"text/x-python","patch_set":5,"id":"7aa08908_6fd7d723","line":1730,"in_reply_to":"7aa08908_115d2a79","updated":"2016-06-10 03:45:10.000000000","message":"Thanks Sukhdev, it is good catch.\n\nIf update_network_precommit fails, because it is inside the transaction, I think nothing will be set in DB, so no need to rollback. And it will not reach postcommit.\n\nFor update_network_postcommit, if it fails, I think it might need rollback the mtu update and the reserved segmentation_id.","commit_id":"afb3cc23f2ece297fe480835d031a1e1e69c784a"},{"author":{"_account_id":6558,"name":"Sukhdev Kapur","email":"sukhdevkapur@gmail.com","username":"sukhdev-8"},"change_message_id":"dde80acb8975ecce79cf9c2981388a14f5924eed","unresolved":false,"context_lines":[{"line_number":1751,"context_line":"            mech_context \u003d driver_context.NetworkContext("},{"line_number":1752,"context_line":"                self, context, updated_network,"},{"line_number":1753,"context_line":"                original_network\u003dnetwork)"},{"line_number":1754,"context_line":"            self.mechanism_manager.update_network_precommit(mech_context)"},{"line_number":1755,"context_line":""},{"line_number":1756,"context_line":"        self.mechanism_manager.update_network_postcommit(mech_context)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7aa08908_b14c16c9","line":1754,"updated":"2016-06-08 05:44:31.000000000","message":"Ditto here","commit_id":"afb3cc23f2ece297fe480835d031a1e1e69c784a"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"99d94beb61063520d9e08c78348c5dc97d4c5e7f","unresolved":false,"context_lines":[{"line_number":1751,"context_line":"            mech_context \u003d driver_context.NetworkContext("},{"line_number":1752,"context_line":"                self, context, updated_network,"},{"line_number":1753,"context_line":"                original_network\u003dnetwork)"},{"line_number":1754,"context_line":"            self.mechanism_manager.update_network_precommit(mech_context)"},{"line_number":1755,"context_line":""},{"line_number":1756,"context_line":"        self.mechanism_manager.update_network_postcommit(mech_context)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7aa08908_6fa517b4","line":1754,"in_reply_to":"7aa08908_b14c16c9","updated":"2016-06-10 03:45:10.000000000","message":"The same as above, we don\u0027t need to take care about precommit.\n\nFor the postcommit, take the existing handle method as reference(line 1500, 1058, 902), I would log an error here, instead of rollback.","commit_id":"afb3cc23f2ece297fe480835d031a1e1e69c784a"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"73b6ea8f547ef738925a611ec8502a50c59369fd","unresolved":false,"context_lines":[{"line_number":1750,"context_line":"            segments.append(updated_segment)"},{"line_number":1751,"context_line":"            self.type_manager._extend_network_dict_provider("},{"line_number":1752,"context_line":"                updated_network, segments)"},{"line_number":1753,"context_line":""},{"line_number":1754,"context_line":"            mech_context \u003d driver_context.NetworkContext("},{"line_number":1755,"context_line":"                self, context, updated_network,"},{"line_number":1756,"context_line":"                original_network\u003dnetwork)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3aaa91ec_91c9617b","line":1753,"updated":"2016-06-23 01:24:46.000000000","message":"There is so much of this that is almost duplicated code of \u0027update_network\u0027. Why can\u0027t you just make the changes required to enable this _reserve_segment function call \u0027update_network\u0027?","commit_id":"f9e02f7e71352de90d69165b25dccdde24c392e1"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"4bf98d19d86e85d3b4f1c126685e0787d7ad8d53","unresolved":false,"context_lines":[{"line_number":1750,"context_line":"            segments.append(updated_segment)"},{"line_number":1751,"context_line":"            self.type_manager._extend_network_dict_provider("},{"line_number":1752,"context_line":"                updated_network, segments)"},{"line_number":1753,"context_line":""},{"line_number":1754,"context_line":"            mech_context \u003d driver_context.NetworkContext("},{"line_number":1755,"context_line":"                self, context, updated_network,"},{"line_number":1756,"context_line":"                original_network\u003dnetwork)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3aaa91ec_727ae9f2","line":1753,"in_reply_to":"3aaa91ec_91c9617b","updated":"2016-06-24 06:29:56.000000000","message":"Yeah, this method and _release_segment can be seen as \"update the segment of network\". So, it is nature to have duplicated logic with update_network. Since it is brought out, I will give it a try to eliminate the duplication.","commit_id":"f9e02f7e71352de90d69165b25dccdde24c392e1"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"b510dd0a0230ee153673787d2f33958f3aa2afab","unresolved":false,"context_lines":[{"line_number":1743,"context_line":""},{"line_number":1744,"context_line":"    def _handle_segment_change(self, rtype, event, trigger, context, segment):"},{"line_number":1745,"context_line":"        if (event \u003d\u003d events.PRECOMMIT_CREATE and"},{"line_number":1746,"context_line":"            not isinstance(trigger, segments_plugin.Plugin)):"},{"line_number":1747,"context_line":"            # TODO(xiaohhui): Now, when create network, ml2 will reserve"},{"line_number":1748,"context_line":"            # segment and trigger this event handler. This event handler"},{"line_number":1749,"context_line":"            # will reserve segment again, which will lead to error as the"}],"source_content_type":"text/x-python","patch_set":16,"id":"dada55a8_3158198a","line":1746,"updated":"2016-07-21 05:54:33.000000000","message":"This is really a workaround here. The right fix should be unifying the segment creation code path. However, I checked the logic of creating segment in ml2, it is a bit complicating and tight-coupling with ml2. I don\u0027t want to add any complexity to this patch. So I just add workaround here and will address the problem in following patch.","commit_id":"c73978dca25d1aadbb2d56147fcd95e15b0712d1"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"8f5ece0fe2eb44a60b144e8108627a7a8345da6d","unresolved":false,"context_lines":[{"line_number":928,"context_line":"                            mech_context)"},{"line_number":929,"context_line":""},{"line_number":930,"context_line":"                        registry.notify(resources.NETWORK,"},{"line_number":931,"context_line":"                                        events.BEFORE_DELETE,"},{"line_number":932,"context_line":"                                        self,"},{"line_number":933,"context_line":"                                        context\u003dcontext,"},{"line_number":934,"context_line":"                                        network_id\u003did)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9ad45d7e_608af31c","line":931,"updated":"2016-08-10 12:07:51.000000000","message":"The event we should emit here is PRECOMMIT_DELETE. BEFORE_DELETE is meant to happen before any delete logic even starts and subscribers of BEFORE events are just supposed to use that for validation. By the time we reach this point we have already done a lot of delete work and it\u0027s about to be committed.","commit_id":"0751ee0abca5a4b943a54e2e64cb8de1c9cce8fa"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"b75c545510ec219453f821a9ccfa2a06678a84f3","unresolved":false,"context_lines":[{"line_number":928,"context_line":"                            mech_context)"},{"line_number":929,"context_line":""},{"line_number":930,"context_line":"                        registry.notify(resources.NETWORK,"},{"line_number":931,"context_line":"                                        events.BEFORE_DELETE,"},{"line_number":932,"context_line":"                                        self,"},{"line_number":933,"context_line":"                                        context\u003dcontext,"},{"line_number":934,"context_line":"                                        network_id\u003did)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9ad45d7e_8b91e4f3","line":931,"in_reply_to":"9ad45d7e_608af31c","updated":"2016-08-11 02:45:27.000000000","message":"Done","commit_id":"0751ee0abca5a4b943a54e2e64cb8de1c9cce8fa"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"8f5ece0fe2eb44a60b144e8108627a7a8345da6d","unresolved":false,"context_lines":[{"line_number":1808,"context_line":""},{"line_number":1809,"context_line":"        try:"},{"line_number":1810,"context_line":"            # Call update_network to trigger mechanism event for segment udpate"},{"line_number":1811,"context_line":"            self.update_network(context, network_id, {attributes.NETWORK: {}})"},{"line_number":1812,"context_line":"        except ml2_exc.MechanismDriverError:"},{"line_number":1813,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1814,"context_line":"                LOG.error(_LE(\"mechanism_manager error occurred when \""}],"source_content_type":"text/x-python","patch_set":18,"id":"9ad45d7e_408fef29","line":1811,"updated":"2016-08-10 12:07:51.000000000","message":"we can\u0027t call update_network inside of an uncommitted transaction (i.e. in a PRECOMMIT event) because it can send information to backends that won\u0027t be notified if the transaction is rolled back.\n\nI think you are going to need to explicitly call the internal component of update_network you need during PRECOMMIT without notifying the mechanism drivers and then call update_network during the segment AFTER_CREATE/AFTER_UPDATE events so the drivers get their notifications.","commit_id":"0751ee0abca5a4b943a54e2e64cb8de1c9cce8fa"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"b75c545510ec219453f821a9ccfa2a06678a84f3","unresolved":false,"context_lines":[{"line_number":1808,"context_line":""},{"line_number":1809,"context_line":"        try:"},{"line_number":1810,"context_line":"            # Call update_network to trigger mechanism event for segment udpate"},{"line_number":1811,"context_line":"            self.update_network(context, network_id, {attributes.NETWORK: {}})"},{"line_number":1812,"context_line":"        except ml2_exc.MechanismDriverError:"},{"line_number":1813,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":1814,"context_line":"                LOG.error(_LE(\"mechanism_manager error occurred when \""}],"source_content_type":"text/x-python","patch_set":18,"id":"9ad45d7e_41c6ad9a","line":1811,"in_reply_to":"9ad45d7e_408fef29","updated":"2016-08-11 02:45:27.000000000","message":"Since the mtu is handled by a984f9554cdcbe93c840a1d8f5c04302e9331e79, the component in update_network I need is just to notify the mechanism driver.\n\nSo, I should notify update_network_precommit in PRECOMMIT event, and notify update_network_postcommit in the AFTER event.","commit_id":"0751ee0abca5a4b943a54e2e64cb8de1c9cce8fa"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"3050568ab462c53536b5363bd01470b47313852e","unresolved":false,"context_lines":[{"line_number":1813,"context_line":""},{"line_number":1814,"context_line":"    def _handle_segment_change(self, rtype, event, trigger, context, segment):"},{"line_number":1815,"context_line":"        if (event \u003d\u003d events.PRECOMMIT_CREATE and"},{"line_number":1816,"context_line":"            not isinstance(trigger, segments_plugin.Plugin)):"},{"line_number":1817,"context_line":"            # TODO(xiaohhui): Now, when create network, ml2 will reserve"},{"line_number":1818,"context_line":"            # segment and trigger this event handler. This event handler"},{"line_number":1819,"context_line":"            # will reserve segment again, which will lead to error as the"}],"source_content_type":"text/x-python","patch_set":23,"id":"1ac06dbe_e21548e5","line":1816,"range":{"start_line":1816,"start_character":12,"end_line":1816,"end_character":61},"updated":"2016-08-22 19:29:28.000000000","message":"This isn\u0027t really obvious to me what this is for. It also feels a little bit funny to get an event and care about who sent the event. How much work will it be to do it \"right\"?","commit_id":"2efe941d3ac53b1feca7acae1a4daf049918bf49"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"f4b080f8b2d739c361f21db61c2eb5036d9ebb26","unresolved":false,"context_lines":[{"line_number":1813,"context_line":""},{"line_number":1814,"context_line":"    def _handle_segment_change(self, rtype, event, trigger, context, segment):"},{"line_number":1815,"context_line":"        if (event \u003d\u003d events.PRECOMMIT_CREATE and"},{"line_number":1816,"context_line":"            not isinstance(trigger, segments_plugin.Plugin)):"},{"line_number":1817,"context_line":"            # TODO(xiaohhui): Now, when create network, ml2 will reserve"},{"line_number":1818,"context_line":"            # segment and trigger this event handler. This event handler"},{"line_number":1819,"context_line":"            # will reserve segment again, which will lead to error as the"}],"source_content_type":"text/x-python","patch_set":23,"id":"1ac06dbe_e5436911","line":1816,"range":{"start_line":1816,"start_character":12,"end_line":1816,"end_character":61},"in_reply_to":"1ac06dbe_1d8c2cd9","updated":"2016-08-23 11:46:15.000000000","message":"I add this patch to address the issue here. It is just a start, but I hope it can demonstrate the issue.\n\nhttps://review.openstack.org/#/c/359147/","commit_id":"2efe941d3ac53b1feca7acae1a4daf049918bf49"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"c5de1dca63c8c2da5c461aa0813896de2040c3fc","unresolved":false,"context_lines":[{"line_number":1813,"context_line":""},{"line_number":1814,"context_line":"    def _handle_segment_change(self, rtype, event, trigger, context, segment):"},{"line_number":1815,"context_line":"        if (event \u003d\u003d events.PRECOMMIT_CREATE and"},{"line_number":1816,"context_line":"            not isinstance(trigger, segments_plugin.Plugin)):"},{"line_number":1817,"context_line":"            # TODO(xiaohhui): Now, when create network, ml2 will reserve"},{"line_number":1818,"context_line":"            # segment and trigger this event handler. This event handler"},{"line_number":1819,"context_line":"            # will reserve segment again, which will lead to error as the"}],"source_content_type":"text/x-python","patch_set":23,"id":"1ac06dbe_1d8c2cd9","line":1816,"range":{"start_line":1816,"start_character":12,"end_line":1816,"end_character":61},"in_reply_to":"1ac06dbe_e21548e5","updated":"2016-08-23 07:09:36.000000000","message":"This is actually a workaround after [1] was merged. When creating a network, line 746 will create segment(s) and reserve the segmentation id. That is the old logic. After [1], the new event PRECOMMIT_CREATE will be triggered. And this event handler will accept the event and try to reserve the segmentation id again and send mechanism event again, which is not right and will have exception.\n\n[1] https://review.openstack.org/#/c/334648\n\nTo be honest, I don\u0027t recall how much work there is. It has been a long time since I added this workaround here. It leaves me an impression that it is not a several lines code thing. I would add another patch to address this issue these days and then we can see if we could merge them together.","commit_id":"2efe941d3ac53b1feca7acae1a4daf049918bf49"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"71afc3d5284f1aca5d1c8146b9aa1b50cfbb608d","unresolved":false,"context_lines":[{"line_number":1815,"context_line":"        if (event \u003d\u003d events.PRECOMMIT_CREATE and"},{"line_number":1816,"context_line":"            not isinstance(trigger, segments_plugin.Plugin)):"},{"line_number":1817,"context_line":"            # TODO(xiaohhui): Now, when create network, ml2 will reserve"},{"line_number":1818,"context_line":"            # segment and trigger this event handler. This event handler"},{"line_number":1819,"context_line":"            # will reserve segment again, which will lead to error as the"},{"line_number":1820,"context_line":"            # segment has already been reserved. This check could be removed"},{"line_number":1821,"context_line":"            # by unifying segment creation procedure."}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_6aee8bed","line":1818,"updated":"2016-08-28 04:27:17.000000000","message":"Ack. As long as it\u0027s already being addressed with another patch that\u0027s fine.","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e14089d51bfa63745698be1af138c6687a441ad4","unresolved":false,"context_lines":[{"line_number":1815,"context_line":"        if (event \u003d\u003d events.PRECOMMIT_CREATE and"},{"line_number":1816,"context_line":"            not isinstance(trigger, segments_plugin.Plugin)):"},{"line_number":1817,"context_line":"            # TODO(xiaohhui): Now, when create network, ml2 will reserve"},{"line_number":1818,"context_line":"            # segment and trigger this event handler. This event handler"},{"line_number":1819,"context_line":"            # will reserve segment again, which will lead to error as the"},{"line_number":1820,"context_line":"            # segment has already been reserved. This check could be removed"},{"line_number":1821,"context_line":"            # by unifying segment creation procedure."}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_0f835c05","line":1818,"updated":"2016-08-27 06:10:13.000000000","message":"I can\u0027t see where PRECOMMIT_CREATE is emitted by anything other than neutron.services.segments.db  @L123. So where else is this emitted?\n\nIt seems to me that whenever logic like this is present (ensuring an event came from a particular location) that it\u0027s compensating for the fact that the events are not the same even though they share the same event and type. So can you elaborate a bit more on why this is required?","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"1e961c8371435ab19244243f6d44d6b4fe4b9ba6","unresolved":false,"context_lines":[{"line_number":1815,"context_line":"        if (event \u003d\u003d events.PRECOMMIT_CREATE and"},{"line_number":1816,"context_line":"            not isinstance(trigger, segments_plugin.Plugin)):"},{"line_number":1817,"context_line":"            # TODO(xiaohhui): Now, when create network, ml2 will reserve"},{"line_number":1818,"context_line":"            # segment and trigger this event handler. This event handler"},{"line_number":1819,"context_line":"            # will reserve segment again, which will lead to error as the"},{"line_number":1820,"context_line":"            # segment has already been reserved. This check could be removed"},{"line_number":1821,"context_line":"            # by unifying segment creation procedure."}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_f6b0cc45","line":1818,"updated":"2016-08-27 18:32:53.000000000","message":"So if this is triggered on the segment event, get rid of the old logic that is duplicating this so it all happens in one place.","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"86622d054a6ed63c9569354eca86be344eed5ab3","unresolved":false,"context_lines":[{"line_number":1815,"context_line":"        if (event \u003d\u003d events.PRECOMMIT_CREATE and"},{"line_number":1816,"context_line":"            not isinstance(trigger, segments_plugin.Plugin)):"},{"line_number":1817,"context_line":"            # TODO(xiaohhui): Now, when create network, ml2 will reserve"},{"line_number":1818,"context_line":"            # segment and trigger this event handler. This event handler"},{"line_number":1819,"context_line":"            # will reserve segment again, which will lead to error as the"},{"line_number":1820,"context_line":"            # segment has already been reserved. This check could be removed"},{"line_number":1821,"context_line":"            # by unifying segment creation procedure."}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_280d7637","line":1818,"in_reply_to":"fa7ab95a_0f835c05","updated":"2016-08-27 11:06:49.000000000","message":"I added comment at PS 23, copied as follow, I also make a fix for it at [2]\n\nThis is actually a workaround after [1] was merged. When creating a network, line 746 will create segment(s) and reserve the segmentation id. That is the old logic. After [1], the new event PRECOMMIT_CREATE will be triggered. And this event handler will accept the event and try to reserve the segmentation id again and send mechanism event again, which is not right and will have exception.\n[1] https://review.openstack.org/#/c/334648\n[2] https://review.openstack.org/#/c/359147/","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"4addbc941833a91b3128c9be3764aebe928e949b","unresolved":false,"context_lines":[{"line_number":1815,"context_line":"        if (event \u003d\u003d events.PRECOMMIT_CREATE and"},{"line_number":1816,"context_line":"            not isinstance(trigger, segments_plugin.Plugin)):"},{"line_number":1817,"context_line":"            # TODO(xiaohhui): Now, when create network, ml2 will reserve"},{"line_number":1818,"context_line":"            # segment and trigger this event handler. This event handler"},{"line_number":1819,"context_line":"            # will reserve segment again, which will lead to error as the"},{"line_number":1820,"context_line":"            # segment has already been reserved. This check could be removed"},{"line_number":1821,"context_line":"            # by unifying segment creation procedure."}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_3f3687e6","line":1818,"in_reply_to":"fa7ab95a_f6b0cc45","updated":"2016-08-28 00:31:11.000000000","message":"Yeah, that is the plan of TODO. Since this needs plenty of code change in ml2/managers, and I don\u0027t want to make this patch more complex, I made this separate patch[1] for it. But if you think they should be one patch, I can do that :)\n\n[1] https://review.openstack.org/#/c/359147/","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"1e961c8371435ab19244243f6d44d6b4fe4b9ba6","unresolved":false,"context_lines":[{"line_number":1825,"context_line":"        network_id \u003d segment.get(\u0027network_id\u0027)"},{"line_number":1826,"context_line":""},{"line_number":1827,"context_line":"        try:"},{"line_number":1828,"context_line":"            self._notify_mechanism_driver_for_segment_change("},{"line_number":1829,"context_line":"                event, context, network_id)"},{"line_number":1830,"context_line":"        except ml2_exc.MechanismDriverError:"},{"line_number":1831,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_16ac50ec","line":1828,"updated":"2016-08-27 18:32:53.000000000","message":"1838 is making changes to the segments associated with a network. The point of precommit events is to allow mech drivers to see things like this. You can\u0027t call the update events before you actually do anything, that doesn\u0027t help the mech drivers at all.","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e14089d51bfa63745698be1af138c6687a441ad4","unresolved":false,"context_lines":[{"line_number":1825,"context_line":"        network_id \u003d segment.get(\u0027network_id\u0027)"},{"line_number":1826,"context_line":""},{"line_number":1827,"context_line":"        try:"},{"line_number":1828,"context_line":"            self._notify_mechanism_driver_for_segment_change("},{"line_number":1829,"context_line":"                event, context, network_id)"},{"line_number":1830,"context_line":"        except ml2_exc.MechanismDriverError:"},{"line_number":1831,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_2f866013","line":1828,"updated":"2016-08-27 06:10:13.000000000","message":"you\u0027re going to call update_network_precommit here in the precommit cases but you\u0027re not doing any of the work until L1838 and L1844. That makes the mech driver call useless because the mech drivers won\u0027t see any change.\n\nLook through the precommit events in the plugin code (e.g. https://github.com/openstack/neutron/blob/c5fb78dfdddf09fccc51362d3984cd25ba57eecc/neutron/plugins/ml2/plugin.py#L810). They occur after all of the changes, but before the commit of those changes. This should be the same.","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"4addbc941833a91b3128c9be3764aebe928e949b","unresolved":false,"context_lines":[{"line_number":1825,"context_line":"        network_id \u003d segment.get(\u0027network_id\u0027)"},{"line_number":1826,"context_line":""},{"line_number":1827,"context_line":"        try:"},{"line_number":1828,"context_line":"            self._notify_mechanism_driver_for_segment_change("},{"line_number":1829,"context_line":"                event, context, network_id)"},{"line_number":1830,"context_line":"        except ml2_exc.MechanismDriverError:"},{"line_number":1831,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_1f3e23b8","line":1828,"in_reply_to":"fa7ab95a_16ac50ec","updated":"2016-08-28 00:31:11.000000000","message":"I see, will move line 1838 ahead.","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"86622d054a6ed63c9569354eca86be344eed5ab3","unresolved":false,"context_lines":[{"line_number":1825,"context_line":"        network_id \u003d segment.get(\u0027network_id\u0027)"},{"line_number":1826,"context_line":""},{"line_number":1827,"context_line":"        try:"},{"line_number":1828,"context_line":"            self._notify_mechanism_driver_for_segment_change("},{"line_number":1829,"context_line":"                event, context, network_id)"},{"line_number":1830,"context_line":"        except ml2_exc.MechanismDriverError:"},{"line_number":1831,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_0858b231","line":1828,"in_reply_to":"fa7ab95a_2f866013","updated":"2016-08-27 11:06:49.000000000","message":"Actually, even line 1838 and line 1844 will not affect the content of the network. The notify here means to tell mechanism driver about the segment change, which I will explain in the following comment.","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"1e961c8371435ab19244243f6d44d6b4fe4b9ba6","unresolved":false,"context_lines":[{"line_number":1847,"context_line":"                                                    context, network_id):"},{"line_number":1848,"context_line":"        db_network \u003d super(Ml2Plugin, self).get_network("},{"line_number":1849,"context_line":"            context, network_id)"},{"line_number":1850,"context_line":"        network_with_segments \u003d dict(db_network)"},{"line_number":1851,"context_line":"        self.type_manager.extend_network_dict_provider(context,"},{"line_number":1852,"context_line":"                                                       network_with_segments)"},{"line_number":1853,"context_line":"        mech_context \u003d driver_context.NetworkContext("}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_36bb1429","line":1850,"updated":"2016-08-27 18:32:53.000000000","message":"_make_network_dict makes a copy of the network, which both super().get_network and self.get_network do","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e14089d51bfa63745698be1af138c6687a441ad4","unresolved":false,"context_lines":[{"line_number":1847,"context_line":"                                                    context, network_id):"},{"line_number":1848,"context_line":"        db_network \u003d super(Ml2Plugin, self).get_network("},{"line_number":1849,"context_line":"            context, network_id)"},{"line_number":1850,"context_line":"        network_with_segments \u003d dict(db_network)"},{"line_number":1851,"context_line":"        self.type_manager.extend_network_dict_provider(context,"},{"line_number":1852,"context_line":"                                                       network_with_segments)"},{"line_number":1853,"context_line":"        mech_context \u003d driver_context.NetworkContext("}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_4f7d641b","line":1850,"updated":"2016-08-27 06:10:13.000000000","message":"super().get_network already returns a dict. Why is this line present?","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"86622d054a6ed63c9569354eca86be344eed5ab3","unresolved":false,"context_lines":[{"line_number":1847,"context_line":"                                                    context, network_id):"},{"line_number":1848,"context_line":"        db_network \u003d super(Ml2Plugin, self).get_network("},{"line_number":1849,"context_line":"            context, network_id)"},{"line_number":1850,"context_line":"        network_with_segments \u003d dict(db_network)"},{"line_number":1851,"context_line":"        self.type_manager.extend_network_dict_provider(context,"},{"line_number":1852,"context_line":"                                                       network_with_segments)"},{"line_number":1853,"context_line":"        mech_context \u003d driver_context.NetworkContext("}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_c8824a52","line":1850,"in_reply_to":"fa7ab95a_4f7d641b","updated":"2016-08-27 11:06:49.000000000","message":"This wants to make a copy of network.","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"1e961c8371435ab19244243f6d44d6b4fe4b9ba6","unresolved":false,"context_lines":[{"line_number":1849,"context_line":"            context, network_id)"},{"line_number":1850,"context_line":"        network_with_segments \u003d dict(db_network)"},{"line_number":1851,"context_line":"        self.type_manager.extend_network_dict_provider(context,"},{"line_number":1852,"context_line":"                                                       network_with_segments)"},{"line_number":1853,"context_line":"        mech_context \u003d driver_context.NetworkContext("},{"line_number":1854,"context_line":"            self, context, network_with_segments,"},{"line_number":1855,"context_line":"            original_network\u003ddb_network)"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_56b6d83d","line":1852,"updated":"2016-08-27 18:32:53.000000000","message":"Just call self.get_network, you are duplicating its logic for no reason that I can see and it makes this code more complex than it needs to be.","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e14089d51bfa63745698be1af138c6687a441ad4","unresolved":false,"context_lines":[{"line_number":1849,"context_line":"            context, network_id)"},{"line_number":1850,"context_line":"        network_with_segments \u003d dict(db_network)"},{"line_number":1851,"context_line":"        self.type_manager.extend_network_dict_provider(context,"},{"line_number":1852,"context_line":"                                                       network_with_segments)"},{"line_number":1853,"context_line":"        mech_context \u003d driver_context.NetworkContext("},{"line_number":1854,"context_line":"            self, context, network_with_segments,"},{"line_number":1855,"context_line":"            original_network\u003ddb_network)"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_6f78682c","line":1852,"updated":"2016-08-27 06:10:13.000000000","message":"L1848-1852 seem to be a duplication of get_network in the ML2 plugin. Why?","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"4addbc941833a91b3128c9be3764aebe928e949b","unresolved":false,"context_lines":[{"line_number":1849,"context_line":"            context, network_id)"},{"line_number":1850,"context_line":"        network_with_segments \u003d dict(db_network)"},{"line_number":1851,"context_line":"        self.type_manager.extend_network_dict_provider(context,"},{"line_number":1852,"context_line":"                                                       network_with_segments)"},{"line_number":1853,"context_line":"        mech_context \u003d driver_context.NetworkContext("},{"line_number":1854,"context_line":"            self, context, network_with_segments,"},{"line_number":1855,"context_line":"            original_network\u003ddb_network)"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_3f4da76c","line":1852,"in_reply_to":"fa7ab95a_56b6d83d","updated":"2016-08-28 00:31:11.000000000","message":"Done","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"86622d054a6ed63c9569354eca86be344eed5ab3","unresolved":false,"context_lines":[{"line_number":1849,"context_line":"            context, network_id)"},{"line_number":1850,"context_line":"        network_with_segments \u003d dict(db_network)"},{"line_number":1851,"context_line":"        self.type_manager.extend_network_dict_provider(context,"},{"line_number":1852,"context_line":"                                                       network_with_segments)"},{"line_number":1853,"context_line":"        mech_context \u003d driver_context.NetworkContext("},{"line_number":1854,"context_line":"            self, context, network_with_segments,"},{"line_number":1855,"context_line":"            original_network\u003ddb_network)"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_e8d38e5d","line":1852,"in_reply_to":"fa7ab95a_6f78682c","updated":"2016-08-27 11:06:49.000000000","message":"It is for constructing the NetworkContext below. This actually follow the procedure of [1], which will not have segments in original network, however, will have segments in the updated network. The handling here just wants to make sure the mechanism driver get the same content.\n\n[1] https://github.com/openstack/neutron/blob/58666054062f537977c4ade5347a645bed567a5a/neutron/plugins/ml2/plugin.py#L783-L810","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"9fb58bc82a479447b487c3920b145e3861a96d16","unresolved":false,"context_lines":[{"line_number":1845,"context_line":""},{"line_number":1846,"context_line":"    def _notify_mechanism_driver_for_segment_change(self, event,"},{"line_number":1847,"context_line":"                                                    context, network_id):"},{"line_number":1848,"context_line":"        db_network \u003d super(Ml2Plugin, self).get_network(context, network_id)"},{"line_number":1849,"context_line":"        network_with_segments \u003d self.get_network(context, network_id)"},{"line_number":1850,"context_line":"        mech_context \u003d driver_context.NetworkContext("},{"line_number":1851,"context_line":"            self, context, network_with_segments,"}],"source_content_type":"text/x-python","patch_set":26,"id":"fa7ab95a_4aaf0781","line":1848,"updated":"2016-08-28 04:40:38.000000000","message":"I understand you are mirroring the behavior of update_network here, but this double-lookup is expensive. I would suggest just passing in network_with_segments as original_network as well.","commit_id":"b7dc55e6fdf21450124fbd640500e5626b847765"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"3f496fdb5d52a534736c2350bc5f5a1a860efe38","unresolved":false,"context_lines":[{"line_number":1845,"context_line":""},{"line_number":1846,"context_line":"    def _notify_mechanism_driver_for_segment_change(self, event,"},{"line_number":1847,"context_line":"                                                    context, network_id):"},{"line_number":1848,"context_line":"        db_network \u003d super(Ml2Plugin, self).get_network(context, network_id)"},{"line_number":1849,"context_line":"        network_with_segments \u003d self.get_network(context, network_id)"},{"line_number":1850,"context_line":"        mech_context \u003d driver_context.NetworkContext("},{"line_number":1851,"context_line":"            self, context, network_with_segments,"}],"source_content_type":"text/x-python","patch_set":26,"id":"fa7ab95a_8ade6f0f","line":1848,"in_reply_to":"fa7ab95a_4aaf0781","updated":"2016-08-28 04:50:52.000000000","message":"Done","commit_id":"b7dc55e6fdf21450124fbd640500e5626b847765"}],"neutron/services/segments/db.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"92eb213b2a1f21e3bb30c4a9c16e4fda395afdf3","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    \"\"\"Mixin class to add segment.\"\"\""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    @property"},{"line_number":53,"context_line":"    def _core_plugin(self):"},{"line_number":54,"context_line":"        return manager.NeutronManager.get_plugin()"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def _make_segment_dict(self, segment_db, fields\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_ca9082ef","line":53,"range":{"start_line":53,"start_character":8,"end_line":53,"end_character":20},"updated":"2016-05-27 22:26:39.000000000","message":"I\u0027m not sure about this calling in to the core plugin.  Especially since the core plugin calls you\u0027re calling appear to be ML2 only.\n\nI\u0027m hoping for something a little more generic (adding callbacks) in this file and then all of the ML2 stuff gets handled by callbacks in ML2.","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ef0381be3f3da724d70e71103d0f95c8dfec2a23","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    \"\"\"Mixin class to add segment.\"\"\""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    @property"},{"line_number":53,"context_line":"    def _core_plugin(self):"},{"line_number":54,"context_line":"        return manager.NeutronManager.get_plugin()"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def _make_segment_dict(self, segment_db, fields\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_69a6b6c2","line":53,"range":{"start_line":53,"start_character":8,"end_line":53,"end_character":20},"in_reply_to":"9abb7d3a_ca9082ef","updated":"2016-05-29 09:11:48.000000000","message":"make sense to me, will change it in next patch.","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"92eb213b2a1f21e3bb30c4a9c16e4fda395afdf3","unresolved":false,"context_lines":[{"line_number":98,"context_line":"            if segmentation_id \u003d\u003d constants.ATTR_NOT_SPECIFIED:"},{"line_number":99,"context_line":"                segmentation_id \u003d None"},{"line_number":100,"context_line":"            # Calculate the index of segment"},{"line_number":101,"context_line":"            segment_index \u003d 0"},{"line_number":102,"context_line":"            segments \u003d self.get_segments("},{"line_number":103,"context_line":"                context,"},{"line_number":104,"context_line":"                filters\u003d{\u0027network_id\u0027: [network_id]},"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_0ac1eae4","line":101,"range":{"start_line":101,"start_character":12,"end_line":101,"end_character":25},"updated":"2016-05-27 22:26:39.000000000","message":"I\u0027m not sure if segment_index is an ML2 detail.  I actually don\u0027t really have my head wrapped around what it is for.","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ef0381be3f3da724d70e71103d0f95c8dfec2a23","unresolved":false,"context_lines":[{"line_number":98,"context_line":"            if segmentation_id \u003d\u003d constants.ATTR_NOT_SPECIFIED:"},{"line_number":99,"context_line":"                segmentation_id \u003d None"},{"line_number":100,"context_line":"            # Calculate the index of segment"},{"line_number":101,"context_line":"            segment_index \u003d 0"},{"line_number":102,"context_line":"            segments \u003d self.get_segments("},{"line_number":103,"context_line":"                context,"},{"line_number":104,"context_line":"                filters\u003d{\u0027network_id\u0027: [network_id]},"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_89a15aac","line":101,"range":{"start_line":101,"start_character":12,"end_line":101,"end_character":25},"in_reply_to":"9abb7d3a_0ac1eae4","updated":"2016-05-29 09:11:48.000000000","message":"I found segment_index was added in this patch [1]. I think it would be necessary for the legacy use case.\n\n\n[1] https://review.openstack.org/#/c/103546/","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"92eb213b2a1f21e3bb30c4a9c16e4fda395afdf3","unresolved":false,"context_lines":[{"line_number":121,"context_line":"        result \u003d self._make_segment_dict(new_segment)"},{"line_number":122,"context_line":"        try:"},{"line_number":123,"context_line":"            # Reserve segment in core plugin"},{"line_number":124,"context_line":"            updated_segment \u003d self._core_plugin.reserve_segment(context,"},{"line_number":125,"context_line":"                                                                result)"},{"line_number":126,"context_line":"            new_segmentation_id \u003d updated_segment[db.SEGMENTATION_ID]"},{"line_number":127,"context_line":"            if (segmentation_id is None and new_segmentation_id):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_cae2429f","line":124,"range":{"start_line":124,"start_character":48,"end_line":124,"end_character":63},"updated":"2016-05-27 22:26:39.000000000","message":"This isn\u0027t defined on the core plugin interface.  As far as I can tell, it is ML2 only.  Shouldn\u0027t it be called from ML2 by a callback or something?","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ef0381be3f3da724d70e71103d0f95c8dfec2a23","unresolved":false,"context_lines":[{"line_number":121,"context_line":"        result \u003d self._make_segment_dict(new_segment)"},{"line_number":122,"context_line":"        try:"},{"line_number":123,"context_line":"            # Reserve segment in core plugin"},{"line_number":124,"context_line":"            updated_segment \u003d self._core_plugin.reserve_segment(context,"},{"line_number":125,"context_line":"                                                                result)"},{"line_number":126,"context_line":"            new_segmentation_id \u003d updated_segment[db.SEGMENTATION_ID]"},{"line_number":127,"context_line":"            if (segmentation_id is None and new_segmentation_id):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_4977723a","line":124,"range":{"start_line":124,"start_character":48,"end_line":124,"end_character":63},"in_reply_to":"9abb7d3a_cae2429f","updated":"2016-05-29 09:11:48.000000000","message":"Done","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"92eb213b2a1f21e3bb30c4a9c16e4fda395afdf3","unresolved":false,"context_lines":[{"line_number":124,"context_line":"            updated_segment \u003d self._core_plugin.reserve_segment(context,"},{"line_number":125,"context_line":"                                                                result)"},{"line_number":126,"context_line":"            new_segmentation_id \u003d updated_segment[db.SEGMENTATION_ID]"},{"line_number":127,"context_line":"            if (segmentation_id is None and new_segmentation_id):"},{"line_number":128,"context_line":"                LOG.debug(\"Get segmentation_id %s from core plugin for \""},{"line_number":129,"context_line":"                          \"segment %s\", new_segmentation_id, result[\u0027id\u0027])"},{"line_number":130,"context_line":"                result \u003d self._update_segment("}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_aa025e3c","line":127,"range":{"start_line":127,"start_character":15,"end_line":127,"end_character":16},"updated":"2016-05-27 22:26:39.000000000","message":"nit:  redundant parenthesis","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ef0381be3f3da724d70e71103d0f95c8dfec2a23","unresolved":false,"context_lines":[{"line_number":124,"context_line":"            updated_segment \u003d self._core_plugin.reserve_segment(context,"},{"line_number":125,"context_line":"                                                                result)"},{"line_number":126,"context_line":"            new_segmentation_id \u003d updated_segment[db.SEGMENTATION_ID]"},{"line_number":127,"context_line":"            if (segmentation_id is None and new_segmentation_id):"},{"line_number":128,"context_line":"                LOG.debug(\"Get segmentation_id %s from core plugin for \""},{"line_number":129,"context_line":"                          \"segment %s\", new_segmentation_id, result[\u0027id\u0027])"},{"line_number":130,"context_line":"                result \u003d self._update_segment("}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_2974ae33","line":127,"range":{"start_line":127,"start_character":15,"end_line":127,"end_character":16},"in_reply_to":"9abb7d3a_aa025e3c","updated":"2016-05-29 09:11:48.000000000","message":"Done","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"92eb213b2a1f21e3bb30c4a9c16e4fda395afdf3","unresolved":false,"context_lines":[{"line_number":140,"context_line":"    @log_helpers.log_method_call"},{"line_number":141,"context_line":"    def update_segment(self, context, uuid, segment):"},{"line_number":142,"context_line":"        \"\"\"Update an existing segment.\"\"\""},{"line_number":143,"context_line":"        raise exceptions.SegmentUpdateNotSupport()"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    @log_helpers.log_method_call"},{"line_number":146,"context_line":"    def get_segment(self, context, uuid, fields\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_4a72d2e6","line":143,"range":{"start_line":143,"start_character":8,"end_line":143,"end_character":48},"updated":"2016-05-27 22:26:39.000000000","message":"We\u0027ll soon have some things to update (like name).  I think this should only prevent update of certain fields.  Or, the core plugin could register for callbacks and prevent update for them.","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ef0381be3f3da724d70e71103d0f95c8dfec2a23","unresolved":false,"context_lines":[{"line_number":140,"context_line":"    @log_helpers.log_method_call"},{"line_number":141,"context_line":"    def update_segment(self, context, uuid, segment):"},{"line_number":142,"context_line":"        \"\"\"Update an existing segment.\"\"\""},{"line_number":143,"context_line":"        raise exceptions.SegmentUpdateNotSupport()"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    @log_helpers.log_method_call"},{"line_number":146,"context_line":"    def get_segment(self, context, uuid, fields\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_dc515e64","line":143,"range":{"start_line":143,"start_character":8,"end_line":143,"end_character":48},"in_reply_to":"9abb7d3a_4a72d2e6","updated":"2016-05-29 09:11:48.000000000","message":"To make it simple, I think prevent certain fields is enough for now. And we can leverage the attribute_map defined in the extension to achieve it. So, no need to change here.","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"92eb213b2a1f21e3bb30c4a9c16e4fda395afdf3","unresolved":false,"context_lines":[{"line_number":185,"context_line":"            raise e.errors[0].error"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"        # Release segment in core plugin"},{"line_number":188,"context_line":"        self._core_plugin.release_segment(context, segment)"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        # Delete segment in DB"},{"line_number":191,"context_line":"        self._delete_segment(context, uuid)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_aa541e54","line":188,"range":{"start_line":188,"start_character":26,"end_line":188,"end_character":41},"updated":"2016-05-27 22:26:39.000000000","message":"ditto (L124)","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ef0381be3f3da724d70e71103d0f95c8dfec2a23","unresolved":false,"context_lines":[{"line_number":185,"context_line":"            raise e.errors[0].error"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"        # Release segment in core plugin"},{"line_number":188,"context_line":"        self._core_plugin.release_segment(context, segment)"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        # Delete segment in DB"},{"line_number":191,"context_line":"        self._delete_segment(context, uuid)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9abb7d3a_697eb6e5","line":188,"range":{"start_line":188,"start_character":26,"end_line":188,"end_character":41},"in_reply_to":"9abb7d3a_aa541e54","updated":"2016-05-29 09:11:48.000000000","message":"Done","commit_id":"f3c0f1a765f7489e9cf6c027cfd843675f5eb3e9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"41b9a560ea61a5cca9c148f1b2cd24167cd2cae6","unresolved":false,"context_lines":[{"line_number":223,"context_line":"reported_hosts \u003d set()"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"def _update_segment_host_mapping_for_agent(resource, event, trigger, context,"},{"line_number":227,"context_line":"                                           host, plugin, agent):"},{"line_number":228,"context_line":"    check_segment_for_agent \u003d getattr(plugin, \u0027check_segment_for_agent\u0027, None)"},{"line_number":229,"context_line":"    if not check_segment_for_agent:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_86972ceb","line":226,"range":{"start_line":226,"start_character":4,"end_line":226,"end_character":42},"updated":"2016-06-07 21:06:23.000000000","message":"I think this should go in the separate patch.","commit_id":"30910c6f978986af70e2eb6fb4be3b01b466f6c3"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"ece46ac5aab233863397d33d106b5369f77550bb","unresolved":false,"context_lines":[{"line_number":223,"context_line":"reported_hosts \u003d set()"},{"line_number":224,"context_line":""},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"def _update_segment_host_mapping_for_agent(resource, event, trigger, context,"},{"line_number":227,"context_line":"                                           host, plugin, agent):"},{"line_number":228,"context_line":"    check_segment_for_agent \u003d getattr(plugin, \u0027check_segment_for_agent\u0027, None)"},{"line_number":229,"context_line":"    if not check_segment_for_agent:"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_22245a5e","line":226,"range":{"start_line":226,"start_character":4,"end_line":226,"end_character":42},"in_reply_to":"7aa08908_86972ceb","updated":"2016-06-08 02:17:21.000000000","message":"Will do","commit_id":"30910c6f978986af70e2eb6fb4be3b01b466f6c3"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"41b9a560ea61a5cca9c148f1b2cd24167cd2cae6","unresolved":false,"context_lines":[{"line_number":244,"context_line":"        update_segment_host_mapping(context, host, current_segment_ids)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"def subscribe():"},{"line_number":248,"context_line":"    registry.subscribe("},{"line_number":249,"context_line":"        _update_segment_host_mapping_for_agent, resources.AGENT,"},{"line_number":250,"context_line":"        events.AFTER_CREATE)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7aa08908_a6b9887b","line":247,"range":{"start_line":247,"start_character":4,"end_line":247,"end_character":13},"updated":"2016-06-07 21:06:23.000000000","message":"ditto","commit_id":"30910c6f978986af70e2eb6fb4be3b01b466f6c3"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"5bcb34793613e0ae4ea4cc4cf1b8b20066cf4665","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                registry.notify(resources.SEGMENT, events.PRECOMMIT_CREATE,"},{"line_number":125,"context_line":"                                self.create_segment, context\u003dcontext,"},{"line_number":126,"context_line":"                                segment\u003dnew_segment)"},{"line_number":127,"context_line":"            except c_exc.CallbackFailure as e:"},{"line_number":128,"context_line":"                raise e.errors[0].error"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"            if not segmentation_id and args[db.SEGMENTATION_ID] is not None:"}],"source_content_type":"text/x-python","patch_set":12,"id":"1aa78d24_2d03db0e","line":127,"updated":"2016-07-05 17:39:44.000000000","message":"why are you raising the first error here? Is this for user facing errors? If so, it\u0027s not necessary because the API already unpacks callbackfailure since it\u0027s a multiple exception type.\n\nOr is it because some caller is catching a specific exception? If that\u0027s the case this is brittle because we don\u0027t know who else is subscribed and might be raising an error.","commit_id":"70d25fa634d84e46ad18494390f553215f1ac1a1"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"72937761b1652b633e1c14b95068fc816eb6a050","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                registry.notify(resources.SEGMENT, events.PRECOMMIT_CREATE,"},{"line_number":125,"context_line":"                                self.create_segment, context\u003dcontext,"},{"line_number":126,"context_line":"                                segment\u003dnew_segment)"},{"line_number":127,"context_line":"            except c_exc.CallbackFailure as e:"},{"line_number":128,"context_line":"                raise e.errors[0].error"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"            if not segmentation_id and args[db.SEGMENTATION_ID] is not None:"}],"source_content_type":"text/x-python","patch_set":12,"id":"1aa78d24_12d22528","line":127,"in_reply_to":"1aa78d24_2d03db0e","updated":"2016-07-07 06:13:55.000000000","message":"It is for user facing error, and you are right, it doesn\u0027t need be raised explicitly here. I will remove it and the same exception at line 180 and line 194.","commit_id":"70d25fa634d84e46ad18494390f553215f1ac1a1"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"7c451f6e62a04264c4a63671ee5757baca33de15","unresolved":false,"context_lines":[{"line_number":244,"context_line":"def _add_segment_host_mapping_for_segment(resource, event, trigger,"},{"line_number":245,"context_line":"                                          context, segment):"},{"line_number":246,"context_line":"    if not context.session.is_active:"},{"line_number":247,"context_line":"        # The session might be in partial rollback state, due to errors in"},{"line_number":248,"context_line":"        # peer callback. In that case, there is no need to add the mapping."},{"line_number":249,"context_line":"        # Just return here."},{"line_number":250,"context_line":"        return"}],"source_content_type":"text/x-python","patch_set":13,"id":"1aa78d24_a301b306","line":247,"updated":"2016-07-11 07:01:12.000000000","message":"we shouldn\u0027t need this here to guess what errors might come from other callbacks. what happens if you continue without this check?","commit_id":"a015dda173c44cf61715ab6a8129377b81980354"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"9db7c0b0c7897b65988bd3e36ff3593221aa39e2","unresolved":false,"context_lines":[{"line_number":244,"context_line":"def _add_segment_host_mapping_for_segment(resource, event, trigger,"},{"line_number":245,"context_line":"                                          context, segment):"},{"line_number":246,"context_line":"    if not context.session.is_active:"},{"line_number":247,"context_line":"        # The session might be in partial rollback state, due to errors in"},{"line_number":248,"context_line":"        # peer callback. In that case, there is no need to add the mapping."},{"line_number":249,"context_line":"        # Just return here."},{"line_number":250,"context_line":"        return"}],"source_content_type":"text/x-python","patch_set":13,"id":"1aa78d24_6702360b","line":247,"in_reply_to":"1aa78d24_a301b306","updated":"2016-07-11 07:39:19.000000000","message":"Without this check, when there is exception in ml2, there will be such exception [1] observed. And the api will report 2 exceptions[2], since the MultipleException will render them both.\n\n\n[1] http://paste.openstack.org/show/529603/\n[2] http://paste.openstack.org/show/529604/","commit_id":"a015dda173c44cf61715ab6a8129377b81980354"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"8f5ece0fe2eb44a60b144e8108627a7a8345da6d","unresolved":false,"context_lines":[{"line_number":297,"context_line":"    admin_ctx \u003d context.elevated()"},{"line_number":298,"context_line":"    segment_plugin_cls \u003d manager.NeutronManager.load_class_for_provider("},{"line_number":299,"context_line":"        \u0027neutron.service_plugins\u0027, \u0027segments\u0027)"},{"line_number":300,"context_line":"    segment_plugin \u003d segment_plugin_cls()"},{"line_number":301,"context_line":"    segments \u003d segment_plugin.get_segments("},{"line_number":302,"context_line":"        admin_ctx, filters\u003d{\u0027network_id\u0027: [network_id]})"},{"line_number":303,"context_line":"    for segment in segments:"}],"source_content_type":"text/x-python","patch_set":18,"id":"9ad45d7e_a0806bf8","line":300,"updated":"2016-08-10 12:07:51.000000000","message":"I don\u0027t think we want to construct the plugin every time this is called. Use manager.neutronmanager.get_service_plugins() and get the segment plugin instance out of that.","commit_id":"0751ee0abca5a4b943a54e2e64cb8de1c9cce8fa"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"b75c545510ec219453f821a9ccfa2a06678a84f3","unresolved":false,"context_lines":[{"line_number":297,"context_line":"    admin_ctx \u003d context.elevated()"},{"line_number":298,"context_line":"    segment_plugin_cls \u003d manager.NeutronManager.load_class_for_provider("},{"line_number":299,"context_line":"        \u0027neutron.service_plugins\u0027, \u0027segments\u0027)"},{"line_number":300,"context_line":"    segment_plugin \u003d segment_plugin_cls()"},{"line_number":301,"context_line":"    segments \u003d segment_plugin.get_segments("},{"line_number":302,"context_line":"        admin_ctx, filters\u003d{\u0027network_id\u0027: [network_id]})"},{"line_number":303,"context_line":"    for segment in segments:"}],"source_content_type":"text/x-python","patch_set":18,"id":"9ad45d7e_06b20b06","line":300,"in_reply_to":"9ad45d7e_a0806bf8","updated":"2016-08-11 02:45:27.000000000","message":"get_service_plugins will only return segment plugin if we enable the segment as service plugin. But it is default not enabled. And this function should be called for all network deletion, with/without segment service plugin enabled. So, I have to load the instance every time here.","commit_id":"0751ee0abca5a4b943a54e2e64cb8de1c9cce8fa"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"1e961c8371435ab19244243f6d44d6b4fe4b9ba6","unresolved":false,"context_lines":[{"line_number":304,"context_line":"    admin_ctx \u003d context.elevated()"},{"line_number":305,"context_line":"    segment_plugin_cls \u003d manager.NeutronManager.load_class_for_provider("},{"line_number":306,"context_line":"        \u0027neutron.service_plugins\u0027, \u0027segments\u0027)"},{"line_number":307,"context_line":"    segment_plugin \u003d segment_plugin_cls()"},{"line_number":308,"context_line":"    segments \u003d segment_plugin.get_segments("},{"line_number":309,"context_line":"        admin_ctx, filters\u003d{\u0027network_id\u0027: [network_id]})"},{"line_number":310,"context_line":"    for segment in segments:"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_7683dc56","line":307,"updated":"2016-08-27 18:32:53.000000000","message":"Every time you construct this class, it calls the __init__ function which will call register_dict_extend_funcs. That means every time this is done, you increase the number of times that dict extend function is called by 1.\n\nIf you refuse to enable the service plugin by default even though we are loading it every time, store the plugin ref in a module-level variable so you aren\u0027t constructing it every time.","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ed7e06c8b39f94fb247ba60700f68252137353b0","unresolved":false,"context_lines":[{"line_number":304,"context_line":"    admin_ctx \u003d context.elevated()"},{"line_number":305,"context_line":"    segment_plugin_cls \u003d manager.NeutronManager.load_class_for_provider("},{"line_number":306,"context_line":"        \u0027neutron.service_plugins\u0027, \u0027segments\u0027)"},{"line_number":307,"context_line":"    segment_plugin \u003d segment_plugin_cls()"},{"line_number":308,"context_line":"    segments \u003d segment_plugin.get_segments("},{"line_number":309,"context_line":"        admin_ctx, filters\u003d{\u0027network_id\u0027: [network_id]})"},{"line_number":310,"context_line":"    for segment in segments:"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_4fa484a5","line":307,"updated":"2016-08-27 06:12:51.000000000","message":"Sorry, I didn\u0027t mean to ask the same question I asked on a previous patchset. We shouldn\u0027t be constructing this on every call. If it\u0027s something that should be run all of the time, it should be a default service plugin.","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"71afc3d5284f1aca5d1c8146b9aa1b50cfbb608d","unresolved":false,"context_lines":[{"line_number":304,"context_line":"    admin_ctx \u003d context.elevated()"},{"line_number":305,"context_line":"    segment_plugin_cls \u003d manager.NeutronManager.load_class_for_provider("},{"line_number":306,"context_line":"        \u0027neutron.service_plugins\u0027, \u0027segments\u0027)"},{"line_number":307,"context_line":"    segment_plugin \u003d segment_plugin_cls()"},{"line_number":308,"context_line":"    segments \u003d segment_plugin.get_segments("},{"line_number":309,"context_line":"        admin_ctx, filters\u003d{\u0027network_id\u0027: [network_id]})"},{"line_number":310,"context_line":"    for segment in segments:"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_8abbcfe4","line":307,"updated":"2016-08-28 04:27:17.000000000","message":"Sounds good","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e14089d51bfa63745698be1af138c6687a441ad4","unresolved":false,"context_lines":[{"line_number":304,"context_line":"    admin_ctx \u003d context.elevated()"},{"line_number":305,"context_line":"    segment_plugin_cls \u003d manager.NeutronManager.load_class_for_provider("},{"line_number":306,"context_line":"        \u0027neutron.service_plugins\u0027, \u0027segments\u0027)"},{"line_number":307,"context_line":"    segment_plugin \u003d segment_plugin_cls()"},{"line_number":308,"context_line":"    segments \u003d segment_plugin.get_segments("},{"line_number":309,"context_line":"        admin_ctx, filters\u003d{\u0027network_id\u0027: [network_id]})"},{"line_number":310,"context_line":"    for segment in segments:"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_8f776c39","line":307,"updated":"2016-08-27 06:10:13.000000000","message":"Why are we constructing a new plugin on every call? We should be looking through the loaded service plugins in the Neutron manager for the segment plugin and then use that. The segments plugin should be an auto loaded service plugin like the auto allocated topology plugin, the timestamp plugin, etc.","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"86622d054a6ed63c9569354eca86be344eed5ab3","unresolved":false,"context_lines":[{"line_number":304,"context_line":"    admin_ctx \u003d context.elevated()"},{"line_number":305,"context_line":"    segment_plugin_cls \u003d manager.NeutronManager.load_class_for_provider("},{"line_number":306,"context_line":"        \u0027neutron.service_plugins\u0027, \u0027segments\u0027)"},{"line_number":307,"context_line":"    segment_plugin \u003d segment_plugin_cls()"},{"line_number":308,"context_line":"    segments \u003d segment_plugin.get_segments("},{"line_number":309,"context_line":"        admin_ctx, filters\u003d{\u0027network_id\u0027: [network_id]})"},{"line_number":310,"context_line":"    for segment in segments:"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_a5028f09","line":307,"in_reply_to":"fa7ab95a_4fa484a5","updated":"2016-08-27 11:06:49.000000000","message":"Do you think I should make segment as default plugin in this patch or in a following patch?\n\nFrom my point of view, it is small code change, but big impact. I would prefer to add it in following patch, after discussing in the routed network meeting.","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"4addbc941833a91b3128c9be3764aebe928e949b","unresolved":false,"context_lines":[{"line_number":304,"context_line":"    admin_ctx \u003d context.elevated()"},{"line_number":305,"context_line":"    segment_plugin_cls \u003d manager.NeutronManager.load_class_for_provider("},{"line_number":306,"context_line":"        \u0027neutron.service_plugins\u0027, \u0027segments\u0027)"},{"line_number":307,"context_line":"    segment_plugin \u003d segment_plugin_cls()"},{"line_number":308,"context_line":"    segments \u003d segment_plugin.get_segments("},{"line_number":309,"context_line":"        admin_ctx, filters\u003d{\u0027network_id\u0027: [network_id]})"},{"line_number":310,"context_line":"    for segment in segments:"}],"source_content_type":"text/x-python","patch_set":25,"id":"fa7ab95a_bc454571","line":307,"in_reply_to":"fa7ab95a_7683dc56","updated":"2016-08-28 00:31:11.000000000","message":"Thanks Kevin for pointing out the issue, I will add a module-level variable first and bring it to the routed network meeting for discussion. I will follow up the discussion result.","commit_id":"624cfd66285c50ccd06bc2bc22d5644283e1b29c"}],"neutron/tests/unit/extensions/test_segment.py":[{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"e38d9ed47a20ba8883df1864ab2610970379b722","unresolved":false,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        def _mock_reserve_segmentation_id(rtype, event, trigger,"},{"line_number":181,"context_line":"                                          context, segment):"},{"line_number":182,"context_line":"            if not segment.get(\u0027segmentation_id\u0027, None):"},{"line_number":183,"context_line":"                segment[\u0027segmentation_id\u0027] \u003d 200"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        with self.network() as network:"}],"source_content_type":"text/x-python","patch_set":22,"id":"3ac371cc_65f25d08","line":182,"range":{"start_line":182,"start_character":48,"end_line":182,"end_character":55},"updated":"2016-08-19 10:28:49.000000000","message":"None is the default return for .get(), so this is redundant.","commit_id":"8b4b206c2d3bb8679a7b83e26b66561b5c17c308"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"95905a7b096aee55950f42f1f7300c12d2dbf37e","unresolved":false,"context_lines":[{"line_number":179,"context_line":""},{"line_number":180,"context_line":"        def _mock_reserve_segmentation_id(rtype, event, trigger,"},{"line_number":181,"context_line":"                                          context, segment):"},{"line_number":182,"context_line":"            if not segment.get(\u0027segmentation_id\u0027, None):"},{"line_number":183,"context_line":"                segment[\u0027segmentation_id\u0027] \u003d 200"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        with self.network() as network:"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ac06dbe_08c5758d","line":182,"range":{"start_line":182,"start_character":48,"end_line":182,"end_character":55},"in_reply_to":"3ac371cc_65f25d08","updated":"2016-08-20 06:12:34.000000000","message":"Done","commit_id":"8b4b206c2d3bb8679a7b83e26b66561b5c17c308"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"e38d9ed47a20ba8883df1864ab2610970379b722","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        with self.network() as network:"},{"line_number":186,"context_line":"            network \u003d network[\u0027network\u0027]"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"        registry.subscribe(_mock_reserve_segmentation_id, resources.SEGMENT,"},{"line_number":189,"context_line":"                           events.PRECOMMIT_CREATE)"},{"line_number":190,"context_line":"        expected_segment \u003d {\u0027network_id\u0027: network[\u0027id\u0027],"},{"line_number":191,"context_line":"                            \u0027physical_network\u0027: \u0027phys_net\u0027,"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ac06dbe_4674b4fc","line":188,"updated":"2016-08-19 10:28:49.000000000","message":"Why are we doing this instead of mocking the return value from reserve_network_segment?","commit_id":"8b4b206c2d3bb8679a7b83e26b66561b5c17c308"},{"author":{"_account_id":11159,"name":"Hong Hui Xiao","email":"honghui_xiao@yeah.net","username":"HongHuiXiao"},"change_message_id":"95905a7b096aee55950f42f1f7300c12d2dbf37e","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        with self.network() as network:"},{"line_number":186,"context_line":"            network \u003d network[\u0027network\u0027]"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"        registry.subscribe(_mock_reserve_segmentation_id, resources.SEGMENT,"},{"line_number":189,"context_line":"                           events.PRECOMMIT_CREATE)"},{"line_number":190,"context_line":"        expected_segment \u003d {\u0027network_id\u0027: network[\u0027id\u0027],"},{"line_number":191,"context_line":"                            \u0027physical_network\u0027: \u0027phys_net\u0027,"}],"source_content_type":"text/x-python","patch_set":22,"id":"1ac06dbe_8805e5c3","line":188,"in_reply_to":"1ac06dbe_4674b4fc","updated":"2016-08-20 06:12:34.000000000","message":"This is because the ml2 plugin is not initialized here, and thus the reserve_network_segment will not be called. The subscribe here will simulate the behavior of ml2 assigning segment id to segment.","commit_id":"8b4b206c2d3bb8679a7b83e26b66561b5c17c308"}]}