)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"3647125e500840ef22c661c1b5aa678036261c13","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Danger, danger, Will Robinson, do not use!"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"So the goal here is to have two API calls - one for the admin"},{"line_number":12,"context_line":"to create a \"blob\" containing the needed information to create"},{"line_number":13,"context_line":"resources; and a second for the tenant/nova to use to actually"},{"line_number":14,"context_line":"create the resources."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"da6ed579_1fc9ad78","line":12,"updated":"2016-01-13 06:26:47.000000000","message":"not sure if we need this blob API. To implement use case [4] as outlined in [2,3], I believe leveraging subnet pools will suffice. Once the single API call to provision network/subnet is implemented, then most of the heavy lifting is in [3], which needs to mirror the nova-network counterpart implementation where, if requested_networks is omitted will verify if the tenant has networks associated, and if not will provision one (using the new API).\n\nAs part of the spec implementation, we\u0027d still need to document use cases 1-3, but this is beyond the scope of this patch.\n\n[1] http://specs.openstack.org/openstack/neutron-specs/specs/mitaka/get-me-a-network.html\n[2] https://etherpad.openstack.org/p/YVR-neutron-get-me-a-network\n[3] https://github.com/openstack/nova/blob/master/nova/network/neutronv2/api.py#L504","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"812509393a06b4f101cb55bf3c8e711e675a2ad9","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Danger, danger, Will Robinson, do not use!"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"So the goal here is to have two API calls - one for the admin"},{"line_number":12,"context_line":"to create a \"blob\" containing the needed information to create"},{"line_number":13,"context_line":"resources; and a second for the tenant/nova to use to actually"},{"line_number":14,"context_line":"create the resources."},{"line_number":15,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"da6ed579_bb611fab","line":12,"in_reply_to":"da6ed579_1fc9ad78","updated":"2016-01-15 23:16:12.000000000","message":"The word blob makes me think of something unstructured.  The data is clearly structured by the DB and API changes you make in the patch.  I wouldn\u0027t use the word blob.","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"}],"etc/policy.json":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"edd79be30f86016fcb74f7a4b6b3728410faf099","unresolved":false,"context_lines":[{"line_number":205,"context_line":"    \"delete_flavor_service_profile\": \"rule:admin_only\","},{"line_number":206,"context_line":"    \"get_flavor_service_profile\": \"rule:regular_user\","},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    \"auto_allocate_network\": \"\","},{"line_number":209,"context_line":"    \"create_network_auto_allocation_policy\": \"rule:admin_only\","},{"line_number":210,"context_line":"    \"update_network_auto_allocation_policy\": \"rule:admin_only\","},{"line_number":211,"context_line":"    \"get_network_auto_allocation_policy\": \"rule:admin_only\","}],"source_content_type":"application/json","patch_set":7,"id":"da6ed579_2bf038c9","line":208,"updated":"2016-01-16 01:19:09.000000000","message":"I think auto_allocate_network may be misleading because we\u0027re allocating much more than that. Consider \u0027allocate_net_topology\u0027, as of today that would simply be basic net+router, but in the future we may consider extending this with topology flavors?","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"b47e2f24880c540de5c63928246cbc98fae7bc4d","unresolved":false,"context_lines":[{"line_number":206,"context_line":"    \"get_flavor_service_profile\": \"rule:regular_user\","},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    \"create_allocated_net_topology\": \"\","},{"line_number":209,"context_line":"    \"update_allocated_net_topology\": \"rule:admin_only\","},{"line_number":210,"context_line":"    \"get_allocated_net_topology\": \"rule:admin_only\","},{"line_number":211,"context_line":"    \"get_allocated_net_topologies\": \"rule:admin_only\","},{"line_number":212,"context_line":"    \"delete_allocated_net_topology\": \"rule:admin_only\""}],"source_content_type":"application/json","patch_set":8,"id":"9a68dd71_ff2ea78a","line":209,"updated":"2016-01-24 20:58:20.000000000","message":"I am having a hard time understanding any use-case for update that doesn\u0027t make things very complicated.","commit_id":"61f7c5c1ed140ee9722d02bd36a8f9fcb8fa77f9"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"b47e2f24880c540de5c63928246cbc98fae7bc4d","unresolved":false,"context_lines":[{"line_number":209,"context_line":"    \"update_allocated_net_topology\": \"rule:admin_only\","},{"line_number":210,"context_line":"    \"get_allocated_net_topology\": \"rule:admin_only\","},{"line_number":211,"context_line":"    \"get_allocated_net_topologies\": \"rule:admin_only\","},{"line_number":212,"context_line":"    \"delete_allocated_net_topology\": \"rule:admin_only\""},{"line_number":213,"context_line":"}"}],"source_content_type":"application/json","patch_set":8,"id":"9a68dd71_ff454742","line":212,"updated":"2016-01-24 20:58:20.000000000","message":"It seems to me that none of these operations need to be admin-only?","commit_id":"61f7c5c1ed140ee9722d02bd36a8f9fcb8fa77f9"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"bf28941501d1b4a9434bfefc26693327e0f2074f","unresolved":false,"context_lines":[{"line_number":204,"context_line":"    \"create_flavor_service_profile\": \"rule:admin_only\","},{"line_number":205,"context_line":"    \"delete_flavor_service_profile\": \"rule:admin_only\","},{"line_number":206,"context_line":"    \"get_flavor_service_profile\": \"rule:regular_user\","},{"line_number":207,"context_line":"    \"get_auto_allocated_topology\": \"rule:admin_or_owner\""},{"line_number":208,"context_line":"}"}],"source_content_type":"application/json","patch_set":17,"id":"5a5ae5dd_bf48e788","line":207,"updated":"2016-02-03 02:58:17.000000000","message":"Nevermind. I didn\u0027t realize the entire topology had a tenant_id field. Without a tenant_id field an \u0027owner\u0027 didn\u0027t really make sense.","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"2ba54a22c6c4ec92ea66a54b08643bc8e1e0844d","unresolved":false,"context_lines":[{"line_number":204,"context_line":"    \"create_flavor_service_profile\": \"rule:admin_only\","},{"line_number":205,"context_line":"    \"delete_flavor_service_profile\": \"rule:admin_only\","},{"line_number":206,"context_line":"    \"get_flavor_service_profile\": \"rule:regular_user\","},{"line_number":207,"context_line":"    \"get_auto_allocated_topology\": \"rule:admin_or_owner\""},{"line_number":208,"context_line":"}"}],"source_content_type":"application/json","patch_set":17,"id":"5a5ae5dd_194efbc1","line":207,"updated":"2016-02-03 01:39:02.000000000","message":"owner of what?","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"476e2c37583b0089fa46717a63e9f734d1b54b27","unresolved":false,"context_lines":[{"line_number":204,"context_line":"    \"create_flavor_service_profile\": \"rule:admin_only\","},{"line_number":205,"context_line":"    \"delete_flavor_service_profile\": \"rule:admin_only\","},{"line_number":206,"context_line":"    \"get_flavor_service_profile\": \"rule:regular_user\","},{"line_number":207,"context_line":"    \"get_auto_allocated_topology\": \"rule:admin_or_owner\""},{"line_number":208,"context_line":"}"}],"source_content_type":"application/json","patch_set":17,"id":"5a5ae5dd_94a4f208","line":207,"in_reply_to":"5a5ae5dd_194efbc1","updated":"2016-02-03 02:13:44.000000000","message":"Should I flip to regular_user?","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"ed76d141514dd22079d75fb54aa30b70c21ec982","unresolved":false,"context_lines":[{"line_number":204,"context_line":"    \"create_flavor_service_profile\": \"rule:admin_only\","},{"line_number":205,"context_line":"    \"delete_flavor_service_profile\": \"rule:admin_only\","},{"line_number":206,"context_line":"    \"get_flavor_service_profile\": \"rule:regular_user\","},{"line_number":207,"context_line":"    \"get_auto_allocated_topology\": \"rule:admin_or_owner\""},{"line_number":208,"context_line":"}"}],"source_content_type":"application/json","patch_set":19,"id":"5a5ae5dd_a103c5b7","line":207,"updated":"2016-02-03 19:53:55.000000000","message":"I thought you were going to add a rule for allowing only admins to set is_default? I\u0027m not sure that the fact that is an external network setting enables us to piggyback on the policy for external networks","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":204,"context_line":"    \"create_flavor_service_profile\": \"rule:admin_only\","},{"line_number":205,"context_line":"    \"delete_flavor_service_profile\": \"rule:admin_only\","},{"line_number":206,"context_line":"    \"get_flavor_service_profile\": \"rule:regular_user\","},{"line_number":207,"context_line":"    \"get_auto_allocated_topology\": \"rule:admin_or_owner\""},{"line_number":208,"context_line":"}"}],"source_content_type":"application/json","patch_set":19,"id":"5a5ae5dd_d131eaa3","line":207,"in_reply_to":"5a5ae5dd_a103c5b7","updated":"2016-02-03 20:47:03.000000000","message":"let me look into that. Good point.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"}],"neutron/db/auto_allocated_topology_db.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"8ca3b43c0392609d8b00f1769732f210bea8e343","unresolved":false,"context_lines":[{"line_number":18,"context_line":"class AutoAllocatedTopologyMixin(object):"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    @classmethod"},{"line_number":21,"context_line":"    def get_auto_allocated_topologies(cls, context, filters\u003dNone):"},{"line_number":22,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a68dd71_64987a6b","line":21,"updated":"2016-01-27 00:51:14.000000000","message":"this is broken.","commit_id":"0546ddcec502b97c6390db6aa3f6da6274168838"}],"neutron/db/external_net_db.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"8ca3b43c0392609d8b00f1769732f210bea8e343","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    network_id \u003d sa.Column(sa.String(36),"},{"line_number":37,"context_line":"                           sa.ForeignKey(\u0027networks.id\u0027, ondelete\u003d\"CASCADE\"),"},{"line_number":38,"context_line":"                           primary_key\u003dTrue)"},{"line_number":39,"context_line":"    is_default \u003d sa.Column(sa.Boolean(), nullable\u003dTrue)"},{"line_number":40,"context_line":"    # Add a relationship to the Network model in order to instruct"},{"line_number":41,"context_line":"    # SQLAlchemy to eagerly load this association"},{"line_number":42,"context_line":"    network \u003d orm.relationship("}],"source_content_type":"text/x-python","patch_set":9,"id":"9a68dd71_84b48ed1","line":39,"updated":"2016-01-27 00:51:14.000000000","message":"this will have to be exposed through the new Extension. TBD","commit_id":"0546ddcec502b97c6390db6aa3f6da6274168838"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"2ba54a22c6c4ec92ea66a54b08643bc8e1e0844d","unresolved":false,"context_lines":[{"line_number":123,"context_line":"                raise e.errors[0].error"},{"line_number":124,"context_line":"            context.session.add(ExternalNetwork(network_id\u003dnet_data[\u0027id\u0027]))"},{"line_number":125,"context_line":"            registry.notify("},{"line_number":126,"context_line":"                resources.EXTERNAL_NETWORK, events.AFTER_CREATE,"},{"line_number":127,"context_line":"                self, context\u003dcontext,"},{"line_number":128,"context_line":"                request\u003dreq_data, network\u003dnet_data)"},{"line_number":129,"context_line":"        net_data[external_net.EXTERNAL] \u003d external"}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_79512f62","line":126,"updated":"2016-02-03 01:39:02.000000000","message":"I think we need a new event for this phase of a transaction. It hasn\u0027t been committed to the DB yet so maybe AFTER_CREATE_PRECOMMIT.","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"bf28941501d1b4a9434bfefc26693327e0f2074f","unresolved":false,"context_lines":[{"line_number":123,"context_line":"                raise e.errors[0].error"},{"line_number":124,"context_line":"            context.session.add(ExternalNetwork(network_id\u003dnet_data[\u0027id\u0027]))"},{"line_number":125,"context_line":"            registry.notify("},{"line_number":126,"context_line":"                resources.EXTERNAL_NETWORK, events.AFTER_CREATE,"},{"line_number":127,"context_line":"                self, context\u003dcontext,"},{"line_number":128,"context_line":"                request\u003dreq_data, network\u003dnet_data)"},{"line_number":129,"context_line":"        net_data[external_net.EXTERNAL] \u003d external"}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_9f43ab6c","line":126,"updated":"2016-02-03 02:58:17.000000000","message":"It\u0027s fine for this use case, but it just means that someone won\u0027t be able to subscribe to this event and do a DB lookup in another session to get the latest state of the network. Not something worth worrying about yet, but maybe leave a TODO inline?","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"476e2c37583b0089fa46717a63e9f734d1b54b27","unresolved":false,"context_lines":[{"line_number":123,"context_line":"                raise e.errors[0].error"},{"line_number":124,"context_line":"            context.session.add(ExternalNetwork(network_id\u003dnet_data[\u0027id\u0027]))"},{"line_number":125,"context_line":"            registry.notify("},{"line_number":126,"context_line":"                resources.EXTERNAL_NETWORK, events.AFTER_CREATE,"},{"line_number":127,"context_line":"                self, context\u003dcontext,"},{"line_number":128,"context_line":"                request\u003dreq_data, network\u003dnet_data)"},{"line_number":129,"context_line":"        net_data[external_net.EXTERNAL] \u003d external"}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_b4f7aef4","line":126,"in_reply_to":"5a5ae5dd_79512f62","updated":"2016-02-03 02:13:44.000000000","message":"the object is in the session. In reality when change [1], we could switch to using the _COMMIT ones, but this looks like it\u0027s working fine.\n\n[1] https://review.openstack.org/#/c/252755/","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"23f67656f0be3cfafedb7ea2d696d2199ab88e70","unresolved":false,"context_lines":[{"line_number":123,"context_line":"                raise e.errors[0].error"},{"line_number":124,"context_line":"            context.session.add(ExternalNetwork(network_id\u003dnet_data[\u0027id\u0027]))"},{"line_number":125,"context_line":"            registry.notify("},{"line_number":126,"context_line":"                resources.EXTERNAL_NETWORK, events.AFTER_CREATE,"},{"line_number":127,"context_line":"                self, context\u003dcontext,"},{"line_number":128,"context_line":"                request\u003dreq_data, network\u003dnet_data)"},{"line_number":129,"context_line":"        net_data[external_net.EXTERNAL] \u003d external"}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_1a1d8966","line":126,"in_reply_to":"5a5ae5dd_9f43ab6c","updated":"2016-02-03 03:06:10.000000000","message":"Done","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"805ab8b325fb0fd1f722f2bfb52b9d30f989ca07","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                           sa.ForeignKey(\u0027networks.id\u0027, ondelete\u003d\"CASCADE\"),"},{"line_number":42,"context_line":"                           primary_key\u003dTrue)"},{"line_number":43,"context_line":"    # introduced by auto-allocated-topology extension"},{"line_number":44,"context_line":"    is_default \u003d sa.Column(sa.Boolean(), nullable\u003dTrue)"},{"line_number":45,"context_line":"    # Add a relationship to the Network model in order to instruct"},{"line_number":46,"context_line":"    # SQLAlchemy to eagerly load this association"},{"line_number":47,"context_line":"    network \u003d orm.relationship("}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_1db7993a","line":44,"updated":"2016-02-03 19:17:18.000000000","message":"Would unique\u003dTrue be appropriate in this case?  My understanding of unique constraints was that it would allow any number of null values but only one True value.  But, I\u0027m not totally confident, and it might be dependent on the DB backend.  I don\u0027t think this is a big deal anyway.\n\nI see that a similar field on SubnetPool [1] doesn\u0027t have a unique constraint.  But, it has nullable\u003dFalse which would prevent that anyway.\n\n [1] https://github.com/openstack/neutron/blob/b1999202b834d475d7472537449006bd60331710/neutron/db/models_v2.py#L244","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                           sa.ForeignKey(\u0027networks.id\u0027, ondelete\u003d\"CASCADE\"),"},{"line_number":42,"context_line":"                           primary_key\u003dTrue)"},{"line_number":43,"context_line":"    # introduced by auto-allocated-topology extension"},{"line_number":44,"context_line":"    is_default \u003d sa.Column(sa.Boolean(), nullable\u003dTrue)"},{"line_number":45,"context_line":"    # Add a relationship to the Network model in order to instruct"},{"line_number":46,"context_line":"    # SQLAlchemy to eagerly load this association"},{"line_number":47,"context_line":"    network \u003d orm.relationship("}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_91b5b2ce","line":44,"in_reply_to":"5a5ae5dd_1db7993a","updated":"2016-02-03 20:47:03.000000000","message":"if I did add a unique constraint here, I could only allow for NULL values for the is-default column (and thus preventing the use of FALSE altogether). Then the code to handle the use case where the user does an update to set is-default back to false would look clunky. The whole thing feels like a hackish way of implementing a business rule that should really be enforced via a DB trigger.\n\nHence I went this route.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1814b8a6276f697d2d06a4505b9e22b4d6b0a626","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                           sa.ForeignKey(\u0027networks.id\u0027, ondelete\u003d\"CASCADE\"),"},{"line_number":42,"context_line":"                           primary_key\u003dTrue)"},{"line_number":43,"context_line":"    # introduced by auto-allocated-topology extension"},{"line_number":44,"context_line":"    is_default \u003d sa.Column(sa.Boolean(), nullable\u003dTrue)"},{"line_number":45,"context_line":"    # Add a relationship to the Network model in order to instruct"},{"line_number":46,"context_line":"    # SQLAlchemy to eagerly load this association"},{"line_number":47,"context_line":"    network \u003d orm.relationship("}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_ec06891f","line":44,"in_reply_to":"5a5ae5dd_91b5b2ce","updated":"2016-02-04 20:42:20.000000000","message":"Sure.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ddda805debfe9140beef88ec0e197cedf39b2a72","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                resources.EXTERNAL_NETWORK, events.AFTER_CREATE,"},{"line_number":129,"context_line":"                self, context\u003dcontext,"},{"line_number":130,"context_line":"                request\u003dreq_data, network\u003dnet_data)"},{"line_number":131,"context_line":"        net_data[external_net.EXTERNAL] \u003d external"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"    def _process_l3_update(self, context, net_data, req_data):"},{"line_number":134,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a5ae5dd_a4245760","line":131,"updated":"2016-02-09 01:48:48.000000000","message":"I don\u0027t think it makes a difference for the current use case, but I wonder if this assignment into net_data about the external status should be before the payload is sent into the callback framework. I suppose the fact that the resource is \u0027external_network\u0027 should be obvious enough...","commit_id":"be7f12b6623a4fcf41a0e717655fee762aabacf4"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"d4b0d2663b61f9f121c2bf6930940e288e4e50d6","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                resources.EXTERNAL_NETWORK, events.AFTER_CREATE,"},{"line_number":129,"context_line":"                self, context\u003dcontext,"},{"line_number":130,"context_line":"                request\u003dreq_data, network\u003dnet_data)"},{"line_number":131,"context_line":"        net_data[external_net.EXTERNAL] \u003d external"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"    def _process_l3_update(self, context, net_data, req_data):"},{"line_number":134,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a5ae5dd_f8e75044","line":131,"updated":"2016-02-09 03:14:32.000000000","message":"right, or just move this line to L116","commit_id":"be7f12b6623a4fcf41a0e717655fee762aabacf4"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"29ff6366199d4d5cbb950fb962d508f925ecb546","unresolved":false,"context_lines":[{"line_number":128,"context_line":"                resources.EXTERNAL_NETWORK, events.AFTER_CREATE,"},{"line_number":129,"context_line":"                self, context\u003dcontext,"},{"line_number":130,"context_line":"                request\u003dreq_data, network\u003dnet_data)"},{"line_number":131,"context_line":"        net_data[external_net.EXTERNAL] \u003d external"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"    def _process_l3_update(self, context, net_data, req_data):"},{"line_number":134,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a5ae5dd_845a7b4f","line":131,"in_reply_to":"5a5ae5dd_a4245760","updated":"2016-02-09 02:02:27.000000000","message":"Good point. I could move the AFTER_CREATE notification after this is set, but in reality the callback implementation would not change.","commit_id":"be7f12b6623a4fcf41a0e717655fee762aabacf4"}],"neutron/db/l3_db.py":[{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"bf28941501d1b4a9434bfefc26693327e0f2074f","unresolved":false,"context_lines":[{"line_number":87,"context_line":"    name \u003d sa.Column(sa.String(attributes.NAME_MAX_LEN))"},{"line_number":88,"context_line":"    status \u003d sa.Column(sa.String(16))"},{"line_number":89,"context_line":"    admin_state_up \u003d sa.Column(sa.Boolean)"},{"line_number":90,"context_line":"    auto_allocated \u003d sa.Column(sa.Boolean, nullable\u003dTrue)"},{"line_number":91,"context_line":"    gw_port_id \u003d sa.Column(sa.String(36), sa.ForeignKey(\u0027ports.id\u0027))"},{"line_number":92,"context_line":"    gw_port \u003d orm.relationship(models_v2.Port, lazy\u003d\u0027joined\u0027)"},{"line_number":93,"context_line":"    attached_ports \u003d orm.relationship("}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_7f5b4f55","line":90,"updated":"2016-02-03 02:58:17.000000000","message":"Another table with a relationship to standardattr is fine. What I meant in that devref is that new columns shouldn\u0027t be added to the standardattr table unless they apply to everything.","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ab2d52590ce3fe7dc81329b58c2925f4dea28503","unresolved":false,"context_lines":[{"line_number":87,"context_line":"    name \u003d sa.Column(sa.String(attributes.NAME_MAX_LEN))"},{"line_number":88,"context_line":"    status \u003d sa.Column(sa.String(16))"},{"line_number":89,"context_line":"    admin_state_up \u003d sa.Column(sa.Boolean)"},{"line_number":90,"context_line":"    auto_allocated \u003d sa.Column(sa.Boolean, nullable\u003dTrue)"},{"line_number":91,"context_line":"    gw_port_id \u003d sa.Column(sa.String(36), sa.ForeignKey(\u0027ports.id\u0027))"},{"line_number":92,"context_line":"    gw_port \u003d orm.relationship(models_v2.Port, lazy\u003d\u0027joined\u0027)"},{"line_number":93,"context_line":"    attached_ports \u003d orm.relationship("}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_33042a14","line":90,"updated":"2016-02-03 08:18:31.000000000","message":"I\u0027m fine with other tables because they implicitly only have entries for the types of objects that they affect.\nWhat I\u0027m against is putting a bunch of columns in the main table that will be mainly made up of NULL values because they don\u0027t make sense for all objects.","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"2ba54a22c6c4ec92ea66a54b08643bc8e1e0844d","unresolved":false,"context_lines":[{"line_number":87,"context_line":"    name \u003d sa.Column(sa.String(attributes.NAME_MAX_LEN))"},{"line_number":88,"context_line":"    status \u003d sa.Column(sa.String(16))"},{"line_number":89,"context_line":"    admin_state_up \u003d sa.Column(sa.Boolean)"},{"line_number":90,"context_line":"    auto_allocated \u003d sa.Column(sa.Boolean, nullable\u003dTrue)"},{"line_number":91,"context_line":"    gw_port_id \u003d sa.Column(sa.String(36), sa.ForeignKey(\u0027ports.id\u0027))"},{"line_number":92,"context_line":"    gw_port \u003d orm.relationship(models_v2.Port, lazy\u003d\u0027joined\u0027)"},{"line_number":93,"context_line":"    attached_ports \u003d orm.relationship("}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_99d0abcb","line":90,"updated":"2016-02-03 01:39:02.000000000","message":"how about creating a new table related to the standardattr table called auto_allocations?\n\ncolumns are standard_attr_id, object_type, tenant_id\n\nThen you can put a uniqueconstraint on object_type and tenant_id and that will solve the auto allocation race.","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"23f67656f0be3cfafedb7ea2d696d2199ab88e70","unresolved":false,"context_lines":[{"line_number":87,"context_line":"    name \u003d sa.Column(sa.String(attributes.NAME_MAX_LEN))"},{"line_number":88,"context_line":"    status \u003d sa.Column(sa.String(16))"},{"line_number":89,"context_line":"    admin_state_up \u003d sa.Column(sa.Boolean)"},{"line_number":90,"context_line":"    auto_allocated \u003d sa.Column(sa.Boolean, nullable\u003dTrue)"},{"line_number":91,"context_line":"    gw_port_id \u003d sa.Column(sa.String(36), sa.ForeignKey(\u0027ports.id\u0027))"},{"line_number":92,"context_line":"    gw_port \u003d orm.relationship(models_v2.Port, lazy\u003d\u0027joined\u0027)"},{"line_number":93,"context_line":"    attached_ports \u003d orm.relationship("}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_da4cd178","line":90,"in_reply_to":"5a5ae5dd_7f5b4f55","updated":"2016-02-03 03:06:10.000000000","message":"um, another table is necessary for implementing 1-M relationships. I\u0027d be very strict to how this table is referenced, to be honest. So the same rule should apply to either 1-1 and 1-m relationships.","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"476e2c37583b0089fa46717a63e9f734d1b54b27","unresolved":false,"context_lines":[{"line_number":87,"context_line":"    name \u003d sa.Column(sa.String(attributes.NAME_MAX_LEN))"},{"line_number":88,"context_line":"    status \u003d sa.Column(sa.String(16))"},{"line_number":89,"context_line":"    admin_state_up \u003d sa.Column(sa.Boolean)"},{"line_number":90,"context_line":"    auto_allocated \u003d sa.Column(sa.Boolean, nullable\u003dTrue)"},{"line_number":91,"context_line":"    gw_port_id \u003d sa.Column(sa.String(36), sa.ForeignKey(\u0027ports.id\u0027))"},{"line_number":92,"context_line":"    gw_port \u003d orm.relationship(models_v2.Port, lazy\u003d\u0027joined\u0027)"},{"line_number":93,"context_line":"    attached_ports \u003d orm.relationship("}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_74c13680","line":90,"in_reply_to":"5a5ae5dd_99d0abcb","updated":"2016-02-03 02:13:44.000000000","message":"I thought about it, but as per your devref, then we should make sure the attribute applies to *all* core resources, and I don\u0027t think this applies here.","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"}],"neutron/db/migration/alembic_migrations/versions/mitaka/expand/19f26505c74f_auto_allocated_topology.py":[{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"97081157a2360256227b1ad3caa03e1e2311d746","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def upgrade():"},{"line_number":34,"context_line":"    for table in (\u0027networks\u0027, \u0027subnets\u0027, \u0027routers\u0027):"},{"line_number":35,"context_line":"        op.add_column(table,"},{"line_number":36,"context_line":"                  sa.Column(\u0027auto_allocated\u0027, sa.Boolean(), nullable\u003dTrue))"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7a5de9d1_5acd4585","line":34,"updated":"2016-02-02 10:20:21.000000000","message":"we don\u0027t have to care about contract?","commit_id":"8c83520cdc9caa71a388605207f8f5c14cdc5bd4"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"812aea48f30eba76c004462545b17e94cab50c59","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"def upgrade():"},{"line_number":34,"context_line":"    for table in (\u0027networks\u0027, \u0027subnets\u0027, \u0027routers\u0027):"},{"line_number":35,"context_line":"        op.add_column(table,"},{"line_number":36,"context_line":"                  sa.Column(\u0027auto_allocated\u0027, sa.Boolean(), nullable\u003dTrue))"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7a5de9d1_ce33a561","line":34,"in_reply_to":"7a5de9d1_5acd4585","updated":"2016-02-02 15:38:58.000000000","message":"No, all these changes are additions (expansion) only.","commit_id":"8c83520cdc9caa71a388605207f8f5c14cdc5bd4"}],"neutron/db/migration/alembic_migrations/versions/mitaka/expand/19f26505c74f_network_auto_allocation.py":[{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"ec83833871284a026b18057ff7af08711136769b","unresolved":false,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    op.create_table("},{"line_number":35,"context_line":"        \u0027network_auto_allocation\u0027,"},{"line_number":36,"context_line":"        sa.Column(\u0027tenant_id\u0027, sa.String(length\u003d255), nullable\u003dTrue),"},{"line_number":37,"context_line":"        sa.Column(\u0027id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":38,"context_line":"        sa.Column(\u0027network_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":39,"context_line":"        sa.ForeignKeyConstraint([\u0027network_id\u0027],"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba8a016a_a7575525","line":36,"range":{"start_line":36,"start_character":19,"end_line":36,"end_character":28},"updated":"2015-11-21 10:14:31.000000000","message":"nit: you can just add index\u003dTrue here, so you won\u0027t need to create index separatly.","commit_id":"6071b0074b80e22827c05134454806d2df7f57a8"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"3647125e500840ef22c661c1b5aa678036261c13","unresolved":false,"context_lines":[{"line_number":29,"context_line":"import sqlalchemy as sa"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    op.create_table("},{"line_number":35,"context_line":"        \u0027network_auto_allocation_policy\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"da6ed579_1f4c4d6f","line":32,"range":{"start_line":32,"start_character":4,"end_line":32,"end_character":11},"updated":"2016-01-13 06:26:47.000000000","message":"If you are going after use case 4 of [1], which is in line with the nova-network use case of auto-assigning a network to a tenant [2,3,4,5,6] when omitting a requested network on the boot command, why are we pursuing these data models that duplicate some of the information that we can leverage from subnet pools?\n\nI think that in the simplest implementation of use case 4 we\u0027d only need an API (call it network_associate) that based on the preprovisioned (default) subnet pool, creates a network and a subnet out of the subnet pool and returns it to Nova. At this point, I wouldn\u0027t bother with a router, but it\u0027s a minor enhancement.\n\nThen the nova/network API layer will have to be tweaked to mimic the behavior of nova-network as outlined in code lines [2,6].\n\n\n[1] http://specs.openstack.org/openstack/neutron-specs/specs/mitaka/get-me-a-network.html\n[2] https://github.com/openstack/nova/blob/master/nova/network/api.py#L281\n[3] https://github.com/openstack/nova/blob/master/nova/network/manager.py#L2091\n[4] https://github.com/openstack/nova/blob/master/nova/objects/network.py#L241\n[5] https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L4464\n[6] https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L2841","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"edd79be30f86016fcb74f7a4b6b3728410faf099","unresolved":false,"context_lines":[{"line_number":29,"context_line":"import sqlalchemy as sa"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    op.create_table("},{"line_number":35,"context_line":"        \u0027network_auto_allocation_policy\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"da6ed579_ebdb0020","line":32,"in_reply_to":"da6ed579_15535c3a","updated":"2016-01-16 01:19:09.000000000","message":"Carl, thanks for elaborating further. I realize I wasn\u0027t clear as I should have.\n\nI am advocating to use subnet pools for two reasons:\n\n1) To fulfill the nova-network parity we can leverage subnet-pools as nova-network doesn\u0027t support overlapping IPs.\n\n2) Using subnet pools admins are not required extra config steps to make auto-allocation possible","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"812509393a06b4f101cb55bf3c8e711e675a2ad9","unresolved":false,"context_lines":[{"line_number":29,"context_line":"import sqlalchemy as sa"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    op.create_table("},{"line_number":35,"context_line":"        \u0027network_auto_allocation_policy\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"da6ed579_15535c3a","line":32,"range":{"start_line":32,"start_character":4,"end_line":32,"end_character":11},"in_reply_to":"da6ed579_1f4c4d6f","updated":"2016-01-15 23:16:12.000000000","message":"If I understand Armando correctly.  I think he is arguing for two changes here.\n\nFirst, there is no need to require the operator to specify names for the network, router, and subnet.  Wouldn\u0027t it be better to just generate something generic?\n\nSecond, he is suggesting the use of subnet pools to mimic the subnet allocation behavior of nova network (which I\u0027m not very familiar with).  So, instead of each tenant getting the same cidr, a unique one would be allocated from the subnet pool.\n\nI think there are pros and cons to this.  It might be okay for each tenant to have the same cidr since we use NAT to get outside anyway.  To use subnetpools, we would require the operator to allocate some subnetpool large enough for all of the tenants to get an allocation.  And, if the subnets are private anyway, ...\n\nIf I were an operator in this situation, I might just create a subnet pool from 10.0.0.0/8 so that I don\u0027t have to worry about it getting full or anything.\n\nWhat are you thoughts, Armando?","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"812509393a06b4f101cb55bf3c8e711e675a2ad9","unresolved":false,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    op.create_table("},{"line_number":35,"context_line":"        \u0027network_auto_allocation_policy\u0027,"},{"line_number":36,"context_line":"        sa.Column(\u0027name\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":37,"context_line":"        sa.Column(\u0027network_name\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":38,"context_line":"        sa.Column(\u0027subnet_name\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":39,"context_line":"        sa.Column(\u0027subnet_cidr\u0027, sa.String(length\u003d64), nullable\u003dFalse),"}],"source_content_type":"text/x-python","patch_set":7,"id":"da6ed579_f261b912","line":36,"range":{"start_line":36,"start_character":19,"end_line":36,"end_character":23},"updated":"2016-01-15 23:16:12.000000000","message":"If there is only one, why does it need a name?","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"812509393a06b4f101cb55bf3c8e711e675a2ad9","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        sa.Column(\u0027name\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":37,"context_line":"        sa.Column(\u0027network_name\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":38,"context_line":"        sa.Column(\u0027subnet_name\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":39,"context_line":"        sa.Column(\u0027subnet_cidr\u0027, sa.String(length\u003d64), nullable\u003dFalse),"},{"line_number":40,"context_line":"        sa.Column(\u0027router_name\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":41,"context_line":"        sa.Column(\u0027ext_net_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":42,"context_line":"        sa.Column(\u0027shared\u0027, sa.Boolean(), nullable\u003dFalse),"}],"source_content_type":"text/x-python","patch_set":7,"id":"da6ed579_f2d1b930","line":39,"range":{"start_line":39,"start_character":19,"end_line":39,"end_character":30},"updated":"2016-01-15 23:16:12.000000000","message":"A subnetpool_id could replace this subnet_cidr here but see my comments on L32","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"812509393a06b4f101cb55bf3c8e711e675a2ad9","unresolved":false,"context_lines":[{"line_number":38,"context_line":"        sa.Column(\u0027subnet_name\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":39,"context_line":"        sa.Column(\u0027subnet_cidr\u0027, sa.String(length\u003d64), nullable\u003dFalse),"},{"line_number":40,"context_line":"        sa.Column(\u0027router_name\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":41,"context_line":"        sa.Column(\u0027ext_net_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":42,"context_line":"        sa.Column(\u0027shared\u0027, sa.Boolean(), nullable\u003dFalse),"},{"line_number":43,"context_line":"        sa.ForeignKeyConstraint([\u0027ext_net_id\u0027],"},{"line_number":44,"context_line":"                                [\u0027networks.id\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"da6ed579_52c0c52a","line":41,"range":{"start_line":41,"start_character":19,"end_line":41,"end_character":29},"updated":"2016-01-15 23:16:12.000000000","message":"This seems to be the most important piece of data in this table.  Basically, \"what network can I use to get to the outside world.\"","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"812509393a06b4f101cb55bf3c8e711e675a2ad9","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        sa.Column(\u0027subnet_cidr\u0027, sa.String(length\u003d64), nullable\u003dFalse),"},{"line_number":40,"context_line":"        sa.Column(\u0027router_name\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":41,"context_line":"        sa.Column(\u0027ext_net_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":42,"context_line":"        sa.Column(\u0027shared\u0027, sa.Boolean(), nullable\u003dFalse),"},{"line_number":43,"context_line":"        sa.ForeignKeyConstraint([\u0027ext_net_id\u0027],"},{"line_number":44,"context_line":"                                [\u0027networks.id\u0027],"},{"line_number":45,"context_line":"                                ondelete\u003d\u0027CASCADE\u0027),"}],"source_content_type":"text/x-python","patch_set":7,"id":"da6ed579_967e7268","line":42,"range":{"start_line":42,"start_character":19,"end_line":42,"end_character":25},"updated":"2016-01-15 23:16:12.000000000","message":"What is the use case for shared?","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"edd79be30f86016fcb74f7a4b6b3728410faf099","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        sa.ForeignKeyConstraint([\u0027ext_net_id\u0027],"},{"line_number":44,"context_line":"                                [\u0027networks.id\u0027],"},{"line_number":45,"context_line":"                                ondelete\u003d\u0027CASCADE\u0027),"},{"line_number":46,"context_line":"        sa.PrimaryKeyConstraint(\u0027id\u0027)"},{"line_number":47,"context_line":"    )"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    op.create_table("}],"source_content_type":"text/x-python","patch_set":7,"id":"da6ed579_abcf4828","line":46,"updated":"2016-01-16 01:19:09.000000000","message":"I think this migration can simply turned into adding a \u0027is_default\u0027 field to the externalnetwork model, the same way the model subnetpool does. This way we can simply figure out which external network to use in order to get VM\u0027s \u0027out\u0027 in the wild.\n\nThen, we don\u0027t need to be fancy about the network label; nova-network uses a label suffixed by an index. We could do the same with a label that is config driven. Tenant can always rename their resources later on.\n\nI too don\u0027t understand the shared field.\n\n[1] https://github.com/openstack/nova/blob/master/nova/network/manager.py#L1380","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"812509393a06b4f101cb55bf3c8e711e675a2ad9","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    )"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    op.create_table("},{"line_number":50,"context_line":"        \u0027network_auto_allocation\u0027,"},{"line_number":51,"context_line":"        sa.Column(\u0027network_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":52,"context_line":"        sa.ForeignKeyConstraint([\u0027network_id\u0027],"},{"line_number":53,"context_line":"                                [\u0027networks.id\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"da6ed579_96f512ee","line":50,"range":{"start_line":50,"start_character":9,"end_line":50,"end_character":32},"updated":"2016-01-15 23:16:12.000000000","message":"Is this just marking a network as the auto-generated one?  What does it matter if it is auto-generated?  I\u0027m not sure I understand the need for this.","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"edd79be30f86016fcb74f7a4b6b3728410faf099","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    )"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    op.create_table("},{"line_number":50,"context_line":"        \u0027network_auto_allocation\u0027,"},{"line_number":51,"context_line":"        sa.Column(\u0027network_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":52,"context_line":"        sa.ForeignKeyConstraint([\u0027network_id\u0027],"},{"line_number":53,"context_line":"                                [\u0027networks.id\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"da6ed579_cb471c76","line":50,"in_reply_to":"da6ed579_96f512ee","updated":"2016-01-16 01:19:09.000000000","message":"I think this is important to ensure that the allocate operation is idempotent. If I look at the nova-network implementation of the auto-allocation logic [1], they simply scan the networks table for a network assigned to the tenant; if there are none, it assigns one and returns it; if there are no left (in our case the subnet pool is exhausted), it errs; if there is one or more (forget the force\u003dTrue case for a sec), it returns the first of the resultset. \n\nNow this is where things get interesting, because we gotta be sure we return always the *same* network to subsequent allocate calls. And that\u0027s where this table comes in. I believe it would help us ensure that we always return the same network. If we didn\u0027t do that we can\u0027t be idempotent and we might be at danger of returning different networks every time the API call is made and the tenant creates networks under our feet.\n\nYou with me? I believe we could simply add an \u0027auto_allocated\u0027 bool to network, but this model is fine too.\n\nThat said, it would be good to hear from the author as I don\u0027t see any use of this in the code.\n\n[1] https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L2863","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"812509393a06b4f101cb55bf3c8e711e675a2ad9","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        sa.ForeignKeyConstraint([\u0027network_id\u0027],"},{"line_number":53,"context_line":"                                [\u0027networks.id\u0027],"},{"line_number":54,"context_line":"                                ondelete\u003d\u0027CASCADE\u0027),"},{"line_number":55,"context_line":"        sa.PrimaryKeyConstraint(\u0027network_id\u0027)"},{"line_number":56,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":7,"id":"da6ed579_d20fd586","line":55,"range":{"start_line":55,"start_character":33,"end_line":55,"end_character":43},"updated":"2016-01-15 23:16:12.000000000","message":"Shouldn\u0027t this table be mapping the tenant_id to a network_id or something like that?","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"}],"neutron/db/models_v2.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"8ca3b43c0392609d8b00f1769732f210bea8e343","unresolved":false,"context_lines":[{"line_number":190,"context_line":"                                        backref\u003d\u0027subnet\u0027,"},{"line_number":191,"context_line":"                                        lazy\u003d\"joined\","},{"line_number":192,"context_line":"                                        cascade\u003d\u0027delete\u0027)"},{"line_number":193,"context_line":"    auto_allocated \u003d sa.Column(sa.Boolean, nullable\u003dTrue)"},{"line_number":194,"context_line":"    enable_dhcp \u003d sa.Column(sa.Boolean())"},{"line_number":195,"context_line":"    dns_nameservers \u003d orm.relationship(DNSNameServer,"},{"line_number":196,"context_line":"                                       backref\u003d\u0027subnet\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a68dd71_64e63aea","line":193,"updated":"2016-01-27 00:51:14.000000000","message":"these can be keep as mere data model additions (no need to be exposed via the API)","commit_id":"0546ddcec502b97c6390db6aa3f6da6274168838"}],"neutron/db/naa_db.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"3647125e500840ef22c661c1b5aa678036261c13","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        return self._fields(res, fields)"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    @log_helpers.log_method_call"},{"line_number":70,"context_line":"    def auto_allocate_network(self, context, network):"},{"line_number":71,"context_line":"        \"\"\"Auto-allocate a network\"\"\""},{"line_number":72,"context_line":"        # Network specified, nothing for us to do here"},{"line_number":73,"context_line":"        if network:"}],"source_content_type":"text/x-python","patch_set":7,"id":"da6ed579_1fbf6dfc","line":70,"updated":"2016-01-13 06:26:47.000000000","message":"this is the API method that I implied in my precedent comment, but I\u0027d rather leverage subnet pools, it would lead to the least amount of effort on your part. The introduction of this table can be avoided IMO.","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"3647125e500840ef22c661c1b5aa678036261c13","unresolved":false,"context_lines":[{"line_number":122,"context_line":"        return network"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def _get_naap(self, context):"},{"line_number":125,"context_line":"        # there is only one policy"},{"line_number":126,"context_line":"        query \u003d self._model_query(context, NaaBasePolicy)"},{"line_number":127,"context_line":"        return query.one()"},{"line_number":128,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"da6ed579_bfa8a1dc","line":125,"range":{"start_line":125,"start_character":28,"end_line":125,"end_character":34},"updated":"2016-01-13 06:26:47.000000000","message":"Not sure if I understand why there\u0027d be only one. Besides, how is this table populated?","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"812509393a06b4f101cb55bf3c8e711e675a2ad9","unresolved":false,"context_lines":[{"line_number":122,"context_line":"        return network"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def _get_naap(self, context):"},{"line_number":125,"context_line":"        # there is only one policy"},{"line_number":126,"context_line":"        query \u003d self._model_query(context, NaaBasePolicy)"},{"line_number":127,"context_line":"        return query.one()"},{"line_number":128,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"da6ed579_1be2ab2f","line":125,"range":{"start_line":125,"start_character":28,"end_line":125,"end_character":34},"in_reply_to":"da6ed579_bfa8a1dc","updated":"2016-01-15 23:16:12.000000000","message":"Shouldn\u0027t this be \"_take_a_nap\" ?  ;)","commit_id":"c9a38f8254f3665a3fbc2638e67c177595749a87"}],"neutron/extensions/auto_allocated_topology.py":[{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"ed76d141514dd22079d75fb54aa30b70c21ec982","unresolved":false,"context_lines":[{"line_number":20,"context_line":"from neutron.services.auto_allocate import plugin"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"RESOURCE_NAME \u003d \"auto_allocated_topology\""},{"line_number":23,"context_line":"COLLECTION_NAME \u003d \"auto_allocated_topologies\""},{"line_number":24,"context_line":"IS_DEFAULT \u003d \"is_default\""},{"line_number":25,"context_line":"EXT_ALIAS \u003d RESOURCE_NAME.replace(\u0027_\u0027, \u0027-\u0027)"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_e1af7de1","line":23,"updated":"2016-02-03 19:53:55.000000000","message":"Aren\u0027t you having issues with collection names?\nthe API framework isn\u0027t good with english grammar, and might need an explicit plural registration like here: http://git.openstack.org/cgit/openstack/neutron/tree/neutron/extensions/qos.py#n126","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":20,"context_line":"from neutron.services.auto_allocate import plugin"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"RESOURCE_NAME \u003d \"auto_allocated_topology\""},{"line_number":23,"context_line":"COLLECTION_NAME \u003d \"auto_allocated_topologies\""},{"line_number":24,"context_line":"IS_DEFAULT \u003d \"is_default\""},{"line_number":25,"context_line":"EXT_ALIAS \u003d RESOURCE_NAME.replace(\u0027_\u0027, \u0027-\u0027)"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_31f95e81","line":23,"in_reply_to":"5a5ae5dd_e1af7de1","updated":"2016-02-03 20:47:03.000000000","message":"what sorts of issues? Let me poke at this a bit more.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"ed76d141514dd22079d75fb54aa30b70c21ec982","unresolved":false,"context_lines":[{"line_number":33,"context_line":"                      \u0027validate\u0027: {\u0027type:uuid\u0027: None},"},{"line_number":34,"context_line":"                      \u0027is_visible\u0027: True},"},{"line_number":35,"context_line":"    },"},{"line_number":36,"context_line":"    \u0027networks\u0027: {IS_DEFAULT: {\u0027allow_post\u0027: True,"},{"line_number":37,"context_line":"                              \u0027allow_put\u0027: True,"},{"line_number":38,"context_line":"                              \u0027default\u0027: False,"},{"line_number":39,"context_line":"                              \u0027is_visible\u0027: True,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_410e9135","line":36,"updated":"2016-02-03 19:53:55.000000000","message":"I spoke a bit with Henri about this, and it seems that the aim is to be able to identify a network, typically an external network, as \"default\".\n\nI hope the API framework is able to understand this... looking at API tests it seems so, as I was worried this might end up overriding the networks resource.\n\nUsing a distinct dict might improve readability, but this is a very minor point.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":33,"context_line":"                      \u0027validate\u0027: {\u0027type:uuid\u0027: None},"},{"line_number":34,"context_line":"                      \u0027is_visible\u0027: True},"},{"line_number":35,"context_line":"    },"},{"line_number":36,"context_line":"    \u0027networks\u0027: {IS_DEFAULT: {\u0027allow_post\u0027: True,"},{"line_number":37,"context_line":"                              \u0027allow_put\u0027: True,"},{"line_number":38,"context_line":"                              \u0027default\u0027: False,"},{"line_number":39,"context_line":"                              \u0027is_visible\u0027: True,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_d1656a45","line":36,"in_reply_to":"5a5ae5dd_410e9135","updated":"2016-02-03 20:47:03.000000000","message":"you mean a separate RESOURCE_ATTRIBUTE_MAP? I must admit I got inspired by the router:external flag so as far as I can tell this works fine :)","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"}],"neutron/services/auto_allocate/db.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5941b4f3a5ad3925c6c057ab7558a2981a28e5c9","unresolved":false,"context_lines":[{"line_number":77,"context_line":"            filter_by(is_default\u003dsql.true()).first())"},{"line_number":78,"context_line":"        if not default_external_network:"},{"line_number":79,"context_line":"            LOG.error(_LE(\"Unable to find default external network \""},{"line_number":80,"context_line":"                      \"for deployment, please create/assign one to \""},{"line_number":81,"context_line":"                      \"allow auto-allocation to work correctly.\"))"},{"line_number":82,"context_line":"            raise exceptions.AutoAllocationFailure("},{"line_number":83,"context_line":"                reason\u003d_(\"Deployment may miss important pre-requisites, \""}],"source_content_type":"text/x-python","patch_set":15,"id":"7a5de9d1_8a16194a","line":80,"updated":"2016-02-02 03:31:27.000000000","message":"indentation here is dodgy","commit_id":"8c83520cdc9caa71a388605207f8f5c14cdc5bd4"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b1ee0c9355e22ac58a72b274d6279320d15d5a8d","unresolved":false,"context_lines":[{"line_number":77,"context_line":"            filter_by(is_default\u003dsql.true()).first())"},{"line_number":78,"context_line":"        if not default_external_network:"},{"line_number":79,"context_line":"            LOG.error(_LE(\"Unable to find default external network \""},{"line_number":80,"context_line":"                      \"for deployment, please create/assign one to \""},{"line_number":81,"context_line":"                      \"allow auto-allocation to work correctly.\"))"},{"line_number":82,"context_line":"            raise exceptions.AutoAllocationFailure("},{"line_number":83,"context_line":"                reason\u003d_(\"Deployment may miss important pre-requisites, \""}],"source_content_type":"text/x-python","patch_set":15,"id":"7a5de9d1_70994bc3","line":80,"in_reply_to":"7a5de9d1_8a16194a","updated":"2016-02-02 06:58:14.000000000","message":"Done","commit_id":"8c83520cdc9caa71a388605207f8f5c14cdc5bd4"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5941b4f3a5ad3925c6c057ab7558a2981a28e5c9","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        if len(networks) \u003e 1:"},{"line_number":99,"context_line":"            LOG.error(_LE(\"Multiple auto-allocated networks detected for \""},{"line_number":100,"context_line":"                          \"tenant %(tenant)s. Keep only resources associated \""},{"line_number":101,"context_line":"                          \"to auto_allocated network %(network)s\") %"},{"line_number":102,"context_line":"                      {\u0027tenant\u0027: tenant_id, \u0027network\u0027: networks[0][\u0027id\u0027]})"},{"line_number":103,"context_line":"            return networks[0]"},{"line_number":104,"context_line":"        elif networks:"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a5de9d1_0a29c989","line":101,"updated":"2016-02-02 03:31:27.000000000","message":"please use a , instead of the %","commit_id":"8c83520cdc9caa71a388605207f8f5c14cdc5bd4"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b1ee0c9355e22ac58a72b274d6279320d15d5a8d","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        if len(networks) \u003e 1:"},{"line_number":99,"context_line":"            LOG.error(_LE(\"Multiple auto-allocated networks detected for \""},{"line_number":100,"context_line":"                          \"tenant %(tenant)s. Keep only resources associated \""},{"line_number":101,"context_line":"                          \"to auto_allocated network %(network)s\") %"},{"line_number":102,"context_line":"                      {\u0027tenant\u0027: tenant_id, \u0027network\u0027: networks[0][\u0027id\u0027]})"},{"line_number":103,"context_line":"            return networks[0]"},{"line_number":104,"context_line":"        elif networks:"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a5de9d1_9094078b","line":101,"in_reply_to":"7a5de9d1_0a29c989","updated":"2016-02-02 06:58:14.000000000","message":"Done","commit_id":"8c83520cdc9caa71a388605207f8f5c14cdc5bd4"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"0f3894de74cfeab2bb31ffbb83415bf2ab90ad0c","unresolved":false,"context_lines":[{"line_number":56,"context_line":"    obj.update({\u0027is_default\u0027: is_default})"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"def _create_update_network_callback(resource, event, trigger, **kwargs):"},{"line_number":60,"context_line":"    request \u003d kwargs[\u0027request\u0027]"},{"line_number":61,"context_line":"    is_default \u003d request.get(\u0027is_default\u0027)"},{"line_number":62,"context_line":"    # Update only if is_default value is provided"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a5de9d1_90670025","line":59,"updated":"2016-02-02 18:45:10.000000000","message":"Duh, this should be named _update_extend_network_callback","commit_id":"46074b357066b719b639017811671b8b0de265bd"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d1916539797da9eec997d65f41f9dbc0f2760d2e","unresolved":false,"context_lines":[{"line_number":56,"context_line":"    obj.update({\u0027is_default\u0027: is_default})"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"def _create_update_network_callback(resource, event, trigger, **kwargs):"},{"line_number":60,"context_line":"    request \u003d kwargs[\u0027request\u0027]"},{"line_number":61,"context_line":"    is_default \u003d request.get(\u0027is_default\u0027)"},{"line_number":62,"context_line":"    # Update only if is_default value is provided"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a5de9d1_f00bd429","line":59,"in_reply_to":"7a5de9d1_90670025","updated":"2016-02-02 18:45:56.000000000","message":"will fix it","commit_id":"46074b357066b719b639017811671b8b0de265bd"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"2ba54a22c6c4ec92ea66a54b08643bc8e1e0844d","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    return net_res"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"def _extend_external_network_callback(resource, event, trigger, **kwargs):"},{"line_number":51,"context_line":"    \"\"\"Ensure the is_default db field matches the create/update request.\"\"\""},{"line_number":52,"context_line":"    context \u003d kwargs[\u0027context\u0027]"},{"line_number":53,"context_line":"    if event in (events.BEFORE_CREATE, events.BEFORE_UPDATE):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_394bb7b2","line":50,"updated":"2016-02-03 01:39:02.000000000","message":"Since the code assumes the presense of the kwargs it uses, why not make them part of the signature to avoid the manual assignments?\n\ndef _extend_external_network_callback(resource, event, trigger, context, request, network **kwargs):","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"476e2c37583b0089fa46717a63e9f734d1b54b27","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    return net_res"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"def _extend_external_network_callback(resource, event, trigger, **kwargs):"},{"line_number":51,"context_line":"    \"\"\"Ensure the is_default db field matches the create/update request.\"\"\""},{"line_number":52,"context_line":"    context \u003d kwargs[\u0027context\u0027]"},{"line_number":53,"context_line":"    if event in (events.BEFORE_CREATE, events.BEFORE_UPDATE):"}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_34009eaa","line":50,"in_reply_to":"5a5ae5dd_394bb7b2","updated":"2016-02-03 02:13:44.000000000","message":"Done","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"2ba54a22c6c4ec92ea66a54b08643bc8e1e0844d","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        # ensure there is only one default external network at any given time"},{"line_number":55,"context_line":"        obj \u003d (context.session.query(external_net_db.ExternalNetwork)."},{"line_number":56,"context_line":"            filter_by(is_default\u003dTrue)).first()"},{"line_number":57,"context_line":"        if obj:"},{"line_number":58,"context_line":"            network_id \u003d obj.network_id"},{"line_number":59,"context_line":"            raise exceptions.DefaultExternalNetworkExists(net_id\u003dnetwork_id)"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_d9c62316","line":57,"updated":"2016-02-03 01:39:02.000000000","message":"This method is racey with multiple neutron servers. Two can receive an empty query here but then both set two different defaults down below.\n\nI think you might just need a DefaultExternalNetwork table that you can put a unique constraint on that ensures there is only one record.","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"bf28941501d1b4a9434bfefc26693327e0f2074f","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        # ensure there is only one default external network at any given time"},{"line_number":55,"context_line":"        obj \u003d (context.session.query(external_net_db.ExternalNetwork)."},{"line_number":56,"context_line":"            filter_by(is_default\u003dTrue)).first()"},{"line_number":57,"context_line":"        if obj:"},{"line_number":58,"context_line":"            network_id \u003d obj.network_id"},{"line_number":59,"context_line":"            raise exceptions.DefaultExternalNetworkExists(net_id\u003dnetwork_id)"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_5f5e9346","line":57,"updated":"2016-02-03 02:58:17.000000000","message":"Yeah, I suspect a table to fix this isn\u0027t worth it.","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"23f67656f0be3cfafedb7ea2d696d2199ab88e70","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        # ensure there is only one default external network at any given time"},{"line_number":55,"context_line":"        obj \u003d (context.session.query(external_net_db.ExternalNetwork)."},{"line_number":56,"context_line":"            filter_by(is_default\u003dTrue)).first()"},{"line_number":57,"context_line":"        if obj:"},{"line_number":58,"context_line":"            network_id \u003d obj.network_id"},{"line_number":59,"context_line":"            raise exceptions.DefaultExternalNetworkExists(net_id\u003dnetwork_id)"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_5a41c138","line":57,"in_reply_to":"5a5ae5dd_5f5e9346","updated":"2016-02-03 03:06:10.000000000","message":"let\u0027s see if this is going to bite us in the future, I\u0027ll had a trace down below.","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"476e2c37583b0089fa46717a63e9f734d1b54b27","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        # ensure there is only one default external network at any given time"},{"line_number":55,"context_line":"        obj \u003d (context.session.query(external_net_db.ExternalNetwork)."},{"line_number":56,"context_line":"            filter_by(is_default\u003dTrue)).first()"},{"line_number":57,"context_line":"        if obj:"},{"line_number":58,"context_line":"            network_id \u003d obj.network_id"},{"line_number":59,"context_line":"            raise exceptions.DefaultExternalNetworkExists(net_id\u003dnetwork_id)"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_f42f66f4","line":57,"in_reply_to":"5a5ae5dd_d9c62316","updated":"2016-02-03 02:13:44.000000000","message":"I had considered this and this is my conclusion: To be racy, two requests for creating external networks must be issues really close. Assumed this really happen in real life (how many public networks does one admin really creates every day?), then code at line 87 always ensure that the same one gets picked, so it\u0027s not like we\u0027re nondeterministic. I can issue a LOG.error over there and warn the admin to clean up, or create a table that hold a single record. Let me look into it.","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"2ba54a22c6c4ec92ea66a54b08643bc8e1e0844d","unresolved":false,"context_lines":[{"line_number":143,"context_line":"                \u0027name\u0027: \u0027auto_allocated_subnet\u0027,"},{"line_number":144,"context_line":"                \u0027network_id\u0027: network[\u0027id\u0027],"},{"line_number":145,"context_line":"                \u0027tenant_id\u0027: tenant_id,"},{"line_number":146,"context_line":"                \u0027ip_version\u0027: 4"},{"line_number":147,"context_line":"            }"},{"line_number":148,"context_line":"            return p_utils.create_subnet("},{"line_number":149,"context_line":"                self.core_plugin, context, {\u0027subnet\u0027: subnet_args})"}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_f9cbdf1c","line":146,"updated":"2016-02-03 01:39:02.000000000","message":"config option to use IPv6 maybe?","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"476e2c37583b0089fa46717a63e9f734d1b54b27","unresolved":false,"context_lines":[{"line_number":143,"context_line":"                \u0027name\u0027: \u0027auto_allocated_subnet\u0027,"},{"line_number":144,"context_line":"                \u0027network_id\u0027: network[\u0027id\u0027],"},{"line_number":145,"context_line":"                \u0027tenant_id\u0027: tenant_id,"},{"line_number":146,"context_line":"                \u0027ip_version\u0027: 4"},{"line_number":147,"context_line":"            }"},{"line_number":148,"context_line":"            return p_utils.create_subnet("},{"line_number":149,"context_line":"                self.core_plugin, context, {\u0027subnet\u0027: subnet_args})"}],"source_content_type":"text/x-python","patch_set":17,"id":"5a5ae5dd_54267a17","line":146,"in_reply_to":"5a5ae5dd_f9cbdf1c","updated":"2016-02-03 02:13:44.000000000","message":"That\u0027s a good point. Not sure I like the config option too much though. I need to think about this one.","commit_id":"0ed684b02a59053e9d03d46e22345fd2c3305e03"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"9fae33ea9a6eadfa6a5179f89625fd833dfb9c52","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    if event in (events.BEFORE_CREATE, events.BEFORE_UPDATE) and is_default:"},{"line_number":56,"context_line":"        # ensure there is only one default external network at any given time"},{"line_number":57,"context_line":"        obj \u003d (context.session.query(external_net_db.ExternalNetwork)."},{"line_number":58,"context_line":"            filter_by(is_default\u003dTrue)).first()"},{"line_number":59,"context_line":"        if obj:"},{"line_number":60,"context_line":"            network_id \u003d obj.network_id"},{"line_number":61,"context_line":"            raise exceptions.DefaultExternalNetworkExists(net_id\u003dnetwork_id)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_710cb694","line":58,"updated":"2016-02-03 20:30:45.000000000","message":"oh well, I guess this does solve the issues noted about possible multiple default external networks...\n\nYou might consider using \"first\" in the relevant queries then.\nAlso, may I ask why did you opted against just putting the id of the default ext net in the conf file? I think the subnetpool feature took that approach","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    if event in (events.BEFORE_CREATE, events.BEFORE_UPDATE) and is_default:"},{"line_number":56,"context_line":"        # ensure there is only one default external network at any given time"},{"line_number":57,"context_line":"        obj \u003d (context.session.query(external_net_db.ExternalNetwork)."},{"line_number":58,"context_line":"            filter_by(is_default\u003dTrue)).first()"},{"line_number":59,"context_line":"        if obj:"},{"line_number":60,"context_line":"            network_id \u003d obj.network_id"},{"line_number":61,"context_line":"            raise exceptions.DefaultExternalNetworkExists(net_id\u003dnetwork_id)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_b1eeae89","line":58,"in_reply_to":"5a5ae5dd_710cb694","updated":"2016-02-03 20:47:03.000000000","message":"The reason why I chose all() vs first() is to alert the admin of a potential (albeit rare) race.\n\nThe option you recall was deprecated and now default subnet pools are stored in the DB.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"adcc943a7bf11fa3f85c96d8afb02b59570e363f","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    registry.subscribe(_extend_external_network_callback,"},{"line_number":76,"context_line":"        resources.EXTERNAL_NETWORK, events.AFTER_CREATE)"},{"line_number":77,"context_line":"    registry.subscribe(_extend_external_network_callback,"},{"line_number":78,"context_line":"        resources.EXTERNAL_NETWORK, events.BEFORE_UPDATE)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def _plugins_lookup(self):"},{"line_number":81,"context_line":"        \"\"\"Initialize plugins.\"\"\""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_1337aeb1","line":78,"updated":"2016-02-03 08:23:51.000000000","message":"is it okay that this subscription happens at module import time or would it be safer to wait for __init__ in case something imports this module that doesn\u0027t actually want to implement the extension?","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"c79329bb2c5265f6d7ac22c215574af4dea7bc68","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    registry.subscribe(_extend_external_network_callback,"},{"line_number":76,"context_line":"        resources.EXTERNAL_NETWORK, events.AFTER_CREATE)"},{"line_number":77,"context_line":"    registry.subscribe(_extend_external_network_callback,"},{"line_number":78,"context_line":"        resources.EXTERNAL_NETWORK, events.BEFORE_UPDATE)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def _plugins_lookup(self):"},{"line_number":81,"context_line":"        \"\"\"Initialize plugins.\"\"\""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_66c72313","line":78,"in_reply_to":"5a5ae5dd_1337aeb1","updated":"2016-02-03 16:49:01.000000000","message":"That\u0027s a good point, I can move it, but I can\u0027t imagine why one would import the db module of an extension and not use it. Everything in here is private stuff.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    registry.subscribe(_extend_external_network_callback,"},{"line_number":76,"context_line":"        resources.EXTERNAL_NETWORK, events.AFTER_CREATE)"},{"line_number":77,"context_line":"    registry.subscribe(_extend_external_network_callback,"},{"line_number":78,"context_line":"        resources.EXTERNAL_NETWORK, events.BEFORE_UPDATE)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def _plugins_lookup(self):"},{"line_number":81,"context_line":"        \"\"\"Initialize plugins.\"\"\""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_f1aae618","line":78,"in_reply_to":"5a5ae5dd_618f0d24","updated":"2016-02-03 20:47:03.000000000","message":"moving this under __init__ may work fine in this specific configuration but can lead to strange behaviors. This module is really not supposed to be imported except by the plugin that implements the mixin, so...","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"805ab8b325fb0fd1f722f2bfb52b9d30f989ca07","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    registry.subscribe(_extend_external_network_callback,"},{"line_number":76,"context_line":"        resources.EXTERNAL_NETWORK, events.AFTER_CREATE)"},{"line_number":77,"context_line":"    registry.subscribe(_extend_external_network_callback,"},{"line_number":78,"context_line":"        resources.EXTERNAL_NETWORK, events.BEFORE_UPDATE)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def _plugins_lookup(self):"},{"line_number":81,"context_line":"        \"\"\"Initialize plugins.\"\"\""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_dc6d8833","line":78,"in_reply_to":"5a5ae5dd_66c72313","updated":"2016-02-03 19:17:18.000000000","message":"I suppose that someone could import some other module which imports this one.  That would be pretty strange though.  So, I don\u0027t really care if it moves or not.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"ed76d141514dd22079d75fb54aa30b70c21ec982","unresolved":false,"context_lines":[{"line_number":75,"context_line":"    registry.subscribe(_extend_external_network_callback,"},{"line_number":76,"context_line":"        resources.EXTERNAL_NETWORK, events.AFTER_CREATE)"},{"line_number":77,"context_line":"    registry.subscribe(_extend_external_network_callback,"},{"line_number":78,"context_line":"        resources.EXTERNAL_NETWORK, events.BEFORE_UPDATE)"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    def _plugins_lookup(self):"},{"line_number":81,"context_line":"        \"\"\"Initialize plugins.\"\"\""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_618f0d24","line":78,"in_reply_to":"5a5ae5dd_66c72313","updated":"2016-02-03 19:53:55.000000000","message":"I think Armando\u0027s comment makes sense.\nI\u0027d add the lazy developer perspective: adding an __init__ to a mixin might have funny unexpected side-effects.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"805ab8b325fb0fd1f722f2bfb52b9d30f989ca07","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def _get_default_external_network(self, context):"},{"line_number":87,"context_line":"        \"\"\"Get the default external network for the deployment.\"\"\""},{"line_number":88,"context_line":"        admin_context \u003d context.elevated()"},{"line_number":89,"context_line":"        default_external_networks \u003d (admin_context.session.query("},{"line_number":90,"context_line":"            external_net_db.ExternalNetwork)."},{"line_number":91,"context_line":"            filter_by(is_default\u003dsql.true())."}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_d731e2a3","line":88,"range":{"start_line":88,"start_character":8,"end_line":88,"end_character":21},"updated":"2016-02-03 19:17:18.000000000","message":"I\u0027m not sure why admin context is needed here.  Can a regular tenant not perform the query for an external network?","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def _get_default_external_network(self, context):"},{"line_number":87,"context_line":"        \"\"\"Get the default external network for the deployment.\"\"\""},{"line_number":88,"context_line":"        admin_context \u003d context.elevated()"},{"line_number":89,"context_line":"        default_external_networks \u003d (admin_context.session.query("},{"line_number":90,"context_line":"            external_net_db.ExternalNetwork)."},{"line_number":91,"context_line":"            filter_by(is_default\u003dsql.true())."}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_917292e4","line":88,"range":{"start_line":88,"start_character":8,"end_line":88,"end_character":21},"in_reply_to":"5a5ae5dd_d731e2a3","updated":"2016-02-03 20:47:03.000000000","message":"you may indeed be right.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"9fae33ea9a6eadfa6a5179f89625fd833dfb9c52","unresolved":false,"context_lines":[{"line_number":91,"context_line":"            filter_by(is_default\u003dsql.true())."},{"line_number":92,"context_line":"            join(models_v2.Network)."},{"line_number":93,"context_line":"            join(model_base.StandardAttribute)."},{"line_number":94,"context_line":"            order_by(model_base.StandardAttribute.id).all())"},{"line_number":95,"context_line":"        if not default_external_networks:"},{"line_number":96,"context_line":"            LOG.error(_LE(\"Unable to find default external network \""},{"line_number":97,"context_line":"                          \"for deployment, please create/assign one to \""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_2697e291","line":94,"updated":"2016-02-03 20:30:45.000000000","message":"what about multiple default networks? I see below that you return the first result, but the whole point of being \"default\" is that there should be only one (and that constraint is rather difficult to implement in DDL)","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":91,"context_line":"            filter_by(is_default\u003dsql.true())."},{"line_number":92,"context_line":"            join(models_v2.Network)."},{"line_number":93,"context_line":"            join(model_base.StandardAttribute)."},{"line_number":94,"context_line":"            order_by(model_base.StandardAttribute.id).all())"},{"line_number":95,"context_line":"        if not default_external_networks:"},{"line_number":96,"context_line":"            LOG.error(_LE(\"Unable to find default external network \""},{"line_number":97,"context_line":"                          \"for deployment, please create/assign one to \""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_1156a229","line":94,"in_reply_to":"5a5ae5dd_2697e291","updated":"2016-02-03 20:47:03.000000000","message":"there should be one only, errors led by races are dealt with.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"805ab8b325fb0fd1f722f2bfb52b9d30f989ca07","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                          \"for deployment, please create/assign one to \""},{"line_number":98,"context_line":"                          \"allow auto-allocation to work correctly.\"))"},{"line_number":99,"context_line":"            raise exceptions.AutoAllocationFailure("},{"line_number":100,"context_line":"                reason\u003d_(\"Deployment may miss important pre-requisites, \""},{"line_number":101,"context_line":"                         \"like a default router:external network\"))"},{"line_number":102,"context_line":"        if len(default_external_networks) \u003e 1:"},{"line_number":103,"context_line":"            LOG.error(_LE(\"Multiple external default etworks detected. \""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_5a119751","line":100,"range":{"start_line":100,"start_character":37,"end_line":100,"end_character":40},"updated":"2016-02-03 19:17:18.000000000","message":"Are you sure *may* is the right word here?  I think you should use *might*.  ;)\n\nSeriously though, I don\u0027t think there is any probability here.  I think it would be better to say:\n\nDeployment is missing a default external network, an important pre-req...\"","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                          \"for deployment, please create/assign one to \""},{"line_number":98,"context_line":"                          \"allow auto-allocation to work correctly.\"))"},{"line_number":99,"context_line":"            raise exceptions.AutoAllocationFailure("},{"line_number":100,"context_line":"                reason\u003d_(\"Deployment may miss important pre-requisites, \""},{"line_number":101,"context_line":"                         \"like a default router:external network\"))"},{"line_number":102,"context_line":"        if len(default_external_networks) \u003e 1:"},{"line_number":103,"context_line":"            LOG.error(_LE(\"Multiple external default etworks detected. \""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_713b56ec","line":100,"range":{"start_line":100,"start_character":37,"end_line":100,"end_character":40},"in_reply_to":"5a5ae5dd_5a119751","updated":"2016-02-03 20:47:03.000000000","message":"Done","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"805ab8b325fb0fd1f722f2bfb52b9d30f989ca07","unresolved":false,"context_lines":[{"line_number":100,"context_line":"                reason\u003d_(\"Deployment may miss important pre-requisites, \""},{"line_number":101,"context_line":"                         \"like a default router:external network\"))"},{"line_number":102,"context_line":"        if len(default_external_networks) \u003e 1:"},{"line_number":103,"context_line":"            LOG.error(_LE(\"Multiple external default etworks detected. \""},{"line_number":104,"context_line":"                          \"Network %s is true \u0027default\u0027.\"),"},{"line_number":105,"context_line":"                      default_external_networks[0][\u0027network_id\u0027])"},{"line_number":106,"context_line":"        return default_external_networks[0]"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_da2b4781","line":103,"range":{"start_line":103,"start_character":53,"end_line":103,"end_character":60},"updated":"2016-02-03 19:17:18.000000000","message":"*networks","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":100,"context_line":"                reason\u003d_(\"Deployment may miss important pre-requisites, \""},{"line_number":101,"context_line":"                         \"like a default router:external network\"))"},{"line_number":102,"context_line":"        if len(default_external_networks) \u003e 1:"},{"line_number":103,"context_line":"            LOG.error(_LE(\"Multiple external default etworks detected. \""},{"line_number":104,"context_line":"                          \"Network %s is true \u0027default\u0027.\"),"},{"line_number":105,"context_line":"                      default_external_networks[0][\u0027network_id\u0027])"},{"line_number":106,"context_line":"        return default_external_networks[0]"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_112d42a7","line":103,"range":{"start_line":103,"start_character":53,"end_line":103,"end_character":60},"in_reply_to":"5a5ae5dd_da2b4781","updated":"2016-02-03 20:47:03.000000000","message":"Done","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"805ab8b325fb0fd1f722f2bfb52b9d30f989ca07","unresolved":false,"context_lines":[{"line_number":103,"context_line":"            LOG.error(_LE(\"Multiple external default etworks detected. \""},{"line_number":104,"context_line":"                          \"Network %s is true \u0027default\u0027.\"),"},{"line_number":105,"context_line":"                      default_external_networks[0][\u0027network_id\u0027])"},{"line_number":106,"context_line":"        return default_external_networks[0]"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def _get_auto_allocated_network(self, context, tenant_id):"},{"line_number":109,"context_line":"        \"\"\"Get the auto allocated network for the tenant.\"\"\""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_5adbd77c","line":106,"range":{"start_line":106,"start_character":15,"end_line":106,"end_character":43},"updated":"2016-02-03 19:17:18.000000000","message":"I\u0027m just pointing this out.  I don\u0027t think I care.  In general, the operator should mark one and only one as the default.  I wonder if a unique constraint would be appropriate in this case.\n\nOrder by is by id.  I think the true default could be perceived as somewhat arbitrary.  It\u0027ll be consistent as long as no new networks are marked as default but when a new one is marked as a default, the new one could take over as the \"true\" default.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":103,"context_line":"            LOG.error(_LE(\"Multiple external default etworks detected. \""},{"line_number":104,"context_line":"                          \"Network %s is true \u0027default\u0027.\"),"},{"line_number":105,"context_line":"                      default_external_networks[0][\u0027network_id\u0027])"},{"line_number":106,"context_line":"        return default_external_networks[0]"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def _get_auto_allocated_network(self, context, tenant_id):"},{"line_number":109,"context_line":"        \"\"\"Get the auto allocated network for the tenant.\"\"\""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_f106661f","line":106,"range":{"start_line":106,"start_character":15,"end_line":106,"end_character":43},"in_reply_to":"5a5ae5dd_5adbd77c","updated":"2016-02-03 20:47:03.000000000","message":"Indeed the operator should mark the external network as default one and only one. This code tries to deal with people who love shooting themselves in the foot. The order by guarantees that, should the data be inconsistent, the outcome of the request isn\u0027t","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"9fae33ea9a6eadfa6a5179f89625fd833dfb9c52","unresolved":false,"context_lines":[{"line_number":111,"context_line":"            filter_by(auto_allocated\u003dsql.true())."},{"line_number":112,"context_line":"            filter_by(tenant_id\u003dtenant_id)."},{"line_number":113,"context_line":"            join(model_base.StandardAttribute)."},{"line_number":114,"context_line":"            order_by(model_base.StandardAttribute.id).all())"},{"line_number":115,"context_line":"        # NOTE(armax): if there are concurrent requests, this can lead to"},{"line_number":116,"context_line":"        # multiple resources being provisioned automatically to the tenant."},{"line_number":117,"context_line":"        # Instead of preventing this from occurring in the first place"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_66415a0b","line":114,"updated":"2016-02-03 20:30:45.000000000","message":"We had an habit of commenting that the .all() call was superfluous. Is that still valid or did something change in sqlalchemy?","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":111,"context_line":"            filter_by(auto_allocated\u003dsql.true())."},{"line_number":112,"context_line":"            filter_by(tenant_id\u003dtenant_id)."},{"line_number":113,"context_line":"            join(model_base.StandardAttribute)."},{"line_number":114,"context_line":"            order_by(model_base.StandardAttribute.id).all())"},{"line_number":115,"context_line":"        # NOTE(armax): if there are concurrent requests, this can lead to"},{"line_number":116,"context_line":"        # multiple resources being provisioned automatically to the tenant."},{"line_number":117,"context_line":"        # Instead of preventing this from occurring in the first place"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_cc04ed16","line":114,"in_reply_to":"5a5ae5dd_66415a0b","updated":"2016-02-03 20:47:03.000000000","message":"I can get rid of it.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"9fae33ea9a6eadfa6a5179f89625fd833dfb9c52","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        # NOTE(armax): if there are concurrent requests, this can lead to"},{"line_number":116,"context_line":"        # multiple resources being provisioned automatically to the tenant."},{"line_number":117,"context_line":"        # Instead of preventing this from occurring in the first place"},{"line_number":118,"context_line":"        # (through pessimistic locking), detect the error and alert"},{"line_number":119,"context_line":"        # the admin of cleanup required."},{"line_number":120,"context_line":"        if len(networks) \u003e 1:"},{"line_number":121,"context_line":"            LOG.error(_LE(\"Multiple auto-allocated networks detected for \""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_c60a8ed2","line":118,"updated":"2016-02-03 20:30:45.000000000","message":"have you tried implementing a compare-and-swap algorithm for this? Is it possible at all?","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        # NOTE(armax): if there are concurrent requests, this can lead to"},{"line_number":116,"context_line":"        # multiple resources being provisioned automatically to the tenant."},{"line_number":117,"context_line":"        # Instead of preventing this from occurring in the first place"},{"line_number":118,"context_line":"        # (through pessimistic locking), detect the error and alert"},{"line_number":119,"context_line":"        # the admin of cleanup required."},{"line_number":120,"context_line":"        if len(networks) \u003e 1:"},{"line_number":121,"context_line":"            LOG.error(_LE(\"Multiple auto-allocated networks detected for \""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_0c1be573","line":118,"in_reply_to":"5a5ae5dd_c60a8ed2","updated":"2016-02-03 20:47:03.000000000","message":"I considered it but the problem is that I have nothing to swap because during the creation phase I have no network to swap with.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"805ab8b325fb0fd1f722f2bfb52b9d30f989ca07","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        # the admin of cleanup required."},{"line_number":120,"context_line":"        if len(networks) \u003e 1:"},{"line_number":121,"context_line":"            LOG.error(_LE(\"Multiple auto-allocated networks detected for \""},{"line_number":122,"context_line":"                          \"tenant %(tenant)s. Keep only resources associated \""},{"line_number":123,"context_line":"                          \"to auto_allocated network %(network)s\"),"},{"line_number":124,"context_line":"                      {\u0027tenant\u0027: tenant_id, \u0027network\u0027: networks[0][\u0027id\u0027]})"},{"line_number":125,"context_line":"            return networks[0]"},{"line_number":126,"context_line":"        elif networks:"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_9a870f0f","line":123,"range":{"start_line":122,"start_character":46,"end_line":123,"end_character":64},"updated":"2016-02-03 19:17:18.000000000","message":"I can see this being easy to mis-read by the operator.  I wonder if instead of mentioning the ones that the operator should keep, mention explicitly the ones that should be removed (i.e. the ids in networks[1:]) and ask the operator to remove resources associated with those networks.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        # the admin of cleanup required."},{"line_number":120,"context_line":"        if len(networks) \u003e 1:"},{"line_number":121,"context_line":"            LOG.error(_LE(\"Multiple auto-allocated networks detected for \""},{"line_number":122,"context_line":"                          \"tenant %(tenant)s. Keep only resources associated \""},{"line_number":123,"context_line":"                          \"to auto_allocated network %(network)s\"),"},{"line_number":124,"context_line":"                      {\u0027tenant\u0027: tenant_id, \u0027network\u0027: networks[0][\u0027id\u0027]})"},{"line_number":125,"context_line":"            return networks[0]"},{"line_number":126,"context_line":"        elif networks:"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_6c20599c","line":123,"range":{"start_line":122,"start_character":46,"end_line":123,"end_character":64},"in_reply_to":"5a5ae5dd_9a870f0f","updated":"2016-02-03 20:47:03.000000000","message":"Done","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"805ab8b325fb0fd1f722f2bfb52b9d30f989ca07","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                          \"tenant %(tenant)s. Keep only resources associated \""},{"line_number":123,"context_line":"                          \"to auto_allocated network %(network)s\"),"},{"line_number":124,"context_line":"                      {\u0027tenant\u0027: tenant_id, \u0027network\u0027: networks[0][\u0027id\u0027]})"},{"line_number":125,"context_line":"            return networks[0]"},{"line_number":126,"context_line":"        elif networks:"},{"line_number":127,"context_line":"            return networks[0]"},{"line_number":128,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_5d93d15b","line":125,"range":{"start_line":125,"start_character":19,"end_line":125,"end_character":30},"updated":"2016-02-03 19:17:18.000000000","message":"In this case -- as opposed to L106 -- we\u0027ll only get multiple networks if a race occurs.  When that race occurs, both processes will reread the DB after closing their transactions (L217) and they\u0027ll both read the same record.  So, I\u0027m fine with this.  Am I right in assuming that the chances of hitting this are pretty slim?  And, the penalty is not too severe (just some extra resources to cleanup)","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                          \"tenant %(tenant)s. Keep only resources associated \""},{"line_number":123,"context_line":"                          \"to auto_allocated network %(network)s\"),"},{"line_number":124,"context_line":"                      {\u0027tenant\u0027: tenant_id, \u0027network\u0027: networks[0][\u0027id\u0027]})"},{"line_number":125,"context_line":"            return networks[0]"},{"line_number":126,"context_line":"        elif networks:"},{"line_number":127,"context_line":"            return networks[0]"},{"line_number":128,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_cca06df7","line":125,"range":{"start_line":125,"start_character":19,"end_line":125,"end_character":30},"in_reply_to":"5a5ae5dd_5d93d15b","updated":"2016-02-03 20:47:03.000000000","message":"My educated guess tells me that chances should be slim, should the race do occur the penalty is a one-off cost.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"805ab8b325fb0fd1f722f2bfb52b9d30f989ca07","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                      {\u0027tenant\u0027: tenant_id, \u0027network\u0027: networks[0][\u0027id\u0027]})"},{"line_number":125,"context_line":"            return networks[0]"},{"line_number":126,"context_line":"        elif networks:"},{"line_number":127,"context_line":"            return networks[0]"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def _prep(self, context, tenant_id, request):"},{"line_number":130,"context_line":"        \"\"\"Add tenant id for admin requests.\"\"\""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_7d41ed0a","line":127,"updated":"2016-02-03 19:17:18.000000000","message":"nit:  This looks a tiny bit better to me but it isn\u0027t a big deal:\n\n        if len(networks) \u003e 1:\n            LOG.error(_LE(\"Multiple auto-allocated networks detected for \"\n                          \"tenant %(tenant)s. Keep only resources associated \"\n                          \"to auto_allocated network %(network)s\"),\n                      {\u0027tenant\u0027: tenant_id, \u0027network\u0027: networks[0][\u0027id\u0027]})\n\n        if networks:\n            return networks[0]","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                      {\u0027tenant\u0027: tenant_id, \u0027network\u0027: networks[0][\u0027id\u0027]})"},{"line_number":125,"context_line":"            return networks[0]"},{"line_number":126,"context_line":"        elif networks:"},{"line_number":127,"context_line":"            return networks[0]"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    def _prep(self, context, tenant_id, request):"},{"line_number":130,"context_line":"        \"\"\"Add tenant id for admin requests.\"\"\""}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_6ca5b902","line":127,"in_reply_to":"5a5ae5dd_7d41ed0a","updated":"2016-02-03 20:47:03.000000000","message":"Done","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"805ab8b325fb0fd1f722f2bfb52b9d30f989ca07","unresolved":false,"context_lines":[{"line_number":144,"context_line":"                self.core_plugin, context,"},{"line_number":145,"context_line":"                {\u0027network\u0027: self._prep(context, tenant_id, network_args)})"},{"line_number":146,"context_line":"            # TODO(armax): determine how to handle dualstack or ipv6 only"},{"line_number":147,"context_line":"            # deployments, as ip_version is a required parameter when"},{"line_number":148,"context_line":"            # relying on subnetpools."},{"line_number":149,"context_line":"            subnet_args \u003d {"},{"line_number":150,"context_line":"                \u0027name\u0027: \u0027auto_allocated_subnet\u0027,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_e087725f","line":147,"range":{"start_line":147,"start_character":30,"end_line":147,"end_character":64},"updated":"2016-02-03 19:17:18.000000000","message":"An ip_version is required for any subnet creation.  It is just that normally the ip_version can be inferred from the cidr or the subnetpool_id, neither of which is specified here.  Since neither is specified, this create relies on a default subnet pool being marked (similar to your default external network)\n\nHere\u0027s an idea off the top of my head:\n\nTry creating subnets for both versions.  If there is no default subnetpool defined for one of them, then an exception will be thrown [1].  We could catch and ignore the exception as long as at least one of the subnet creates works.\n\nThe exception thrown in [1] is pretty generic.  It should probably throw a more specific exception so that it can be caught here without accidentally catching any other BadRequest exceptions and without having to look at the message (yuck!)\n\n [1] https://github.com/openstack/neutron/blob/b1999202b8/neutron/db/db_base_plugin_v2.py#L663","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":144,"context_line":"                self.core_plugin, context,"},{"line_number":145,"context_line":"                {\u0027network\u0027: self._prep(context, tenant_id, network_args)})"},{"line_number":146,"context_line":"            # TODO(armax): determine how to handle dualstack or ipv6 only"},{"line_number":147,"context_line":"            # deployments, as ip_version is a required parameter when"},{"line_number":148,"context_line":"            # relying on subnetpools."},{"line_number":149,"context_line":"            subnet_args \u003d {"},{"line_number":150,"context_line":"                \u0027name\u0027: \u0027auto_allocated_subnet\u0027,"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_6c7cd972","line":147,"range":{"start_line":147,"start_character":30,"end_line":147,"end_character":64},"in_reply_to":"5a5ae5dd_e087725f","updated":"2016-02-03 20:47:03.000000000","message":"I am gonna have to digest this some more. But thanks this is really helpful","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"805ab8b325fb0fd1f722f2bfb52b9d30f989ca07","unresolved":false,"context_lines":[{"line_number":145,"context_line":"                {\u0027network\u0027: self._prep(context, tenant_id, network_args)})"},{"line_number":146,"context_line":"            # TODO(armax): determine how to handle dualstack or ipv6 only"},{"line_number":147,"context_line":"            # deployments, as ip_version is a required parameter when"},{"line_number":148,"context_line":"            # relying on subnetpools."},{"line_number":149,"context_line":"            subnet_args \u003d {"},{"line_number":150,"context_line":"                \u0027name\u0027: \u0027auto_allocated_subnet\u0027,"},{"line_number":151,"context_line":"                \u0027network_id\u0027: network[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_00916e99","line":148,"range":{"start_line":148,"start_character":14,"end_line":148,"end_character":36},"updated":"2016-02-03 19:17:18.000000000","message":"Maybe \"relying on a default subnetpool\" to be just a little more specific.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":145,"context_line":"                {\u0027network\u0027: self._prep(context, tenant_id, network_args)})"},{"line_number":146,"context_line":"            # TODO(armax): determine how to handle dualstack or ipv6 only"},{"line_number":147,"context_line":"            # deployments, as ip_version is a required parameter when"},{"line_number":148,"context_line":"            # relying on subnetpools."},{"line_number":149,"context_line":"            subnet_args \u003d {"},{"line_number":150,"context_line":"                \u0027name\u0027: \u0027auto_allocated_subnet\u0027,"},{"line_number":151,"context_line":"                \u0027network_id\u0027: network[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_8caa75d3","line":148,"range":{"start_line":148,"start_character":14,"end_line":148,"end_character":36},"in_reply_to":"5a5ae5dd_00916e99","updated":"2016-02-03 20:47:03.000000000","message":"Done","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"805ab8b325fb0fd1f722f2bfb52b9d30f989ca07","unresolved":false,"context_lines":[{"line_number":158,"context_line":"            LOG.exception(_LE(\"Unable to auto allocate topology \""},{"line_number":159,"context_line":"                              \"for tenant %s\"), tenant_id)"},{"line_number":160,"context_line":"            raise exceptions.AutoAllocationFailure("},{"line_number":161,"context_line":"                reason\u003d_(\"Deployment may miss important pre-requisites, \""},{"line_number":162,"context_line":"                         \"like shared/default subnetpool setup\"))"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def _provision_external_connectivity("}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_40c5268c","line":161,"range":{"start_line":161,"start_character":37,"end_line":161,"end_character":40},"updated":"2016-02-03 19:17:18.000000000","message":"Similar comment as on L100","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":158,"context_line":"            LOG.exception(_LE(\"Unable to auto allocate topology \""},{"line_number":159,"context_line":"                              \"for tenant %s\"), tenant_id)"},{"line_number":160,"context_line":"            raise exceptions.AutoAllocationFailure("},{"line_number":161,"context_line":"                reason\u003d_(\"Deployment may miss important pre-requisites, \""},{"line_number":162,"context_line":"                         \"like shared/default subnetpool setup\"))"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def _provision_external_connectivity("}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_2cb02143","line":161,"range":{"start_line":161,"start_character":37,"end_line":161,"end_character":40},"in_reply_to":"5a5ae5dd_40c5268c","updated":"2016-02-03 20:47:03.000000000","message":"Done","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"9fae33ea9a6eadfa6a5179f89625fd833dfb9c52","unresolved":false,"context_lines":[{"line_number":208,"context_line":"                context)"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"            self._plugins_lookup()"},{"line_number":211,"context_line":"            subnet \u003d self._provision_tenant_private_network(context, tenant_id)"},{"line_number":212,"context_line":"            router \u003d self._provision_external_connectivity("},{"line_number":213,"context_line":"                context, default_external_network, subnet[\u0027id\u0027], tenant_id)"},{"line_number":214,"context_line":"            self._update("}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_06c66667","line":211,"updated":"2016-02-03 20:30:45.000000000","message":"Going back to the concurrency issue you correctly identified, the approach implemented here implies not only that more than one auto_allocated topology can exist for a tenant, but also that the additional topology might actually be used by an instance.\n\nIf that is intentional and expected, never mind.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":208,"context_line":"                context)"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"            self._plugins_lookup()"},{"line_number":211,"context_line":"            subnet \u003d self._provision_tenant_private_network(context, tenant_id)"},{"line_number":212,"context_line":"            router \u003d self._provision_external_connectivity("},{"line_number":213,"context_line":"                context, default_external_network, subnet[\u0027id\u0027], tenant_id)"},{"line_number":214,"context_line":"            self._update("}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_4cdb1d7d","line":211,"in_reply_to":"5a5ae5dd_06c66667","updated":"2016-02-03 20:47:03.000000000","message":"Not sure I understand what you\u0027re implying here. The chance of a race will lead to two topologies","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"805ab8b325fb0fd1f722f2bfb52b9d30f989ca07","unresolved":false,"context_lines":[{"line_number":211,"context_line":"            subnet \u003d self._provision_tenant_private_network(context, tenant_id)"},{"line_number":212,"context_line":"            router \u003d self._provision_external_connectivity("},{"line_number":213,"context_line":"                context, default_external_network, subnet[\u0027id\u0027], tenant_id)"},{"line_number":214,"context_line":"            self._update("},{"line_number":215,"context_line":"                context, subnet[\u0027network_id\u0027],"},{"line_number":216,"context_line":"                subnet[\u0027id\u0027], router[\u0027id\u0027], tenant_id)"},{"line_number":217,"context_line":"        network \u003d self._get_auto_allocated_network(context, tenant_id)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_80dd5e44","line":214,"range":{"start_line":214,"start_character":17,"end_line":214,"end_character":24},"updated":"2016-02-03 19:17:18.000000000","message":"I\u0027m curious, was it not possible to provision these with auto_allocated \u003d True in the first place?  I guess not since you\u0027re calling in to the plugin to do the provisioning.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dc6ba4ca1b3bf2b3a4a20c79c92bc0812f5150e3","unresolved":false,"context_lines":[{"line_number":211,"context_line":"            subnet \u003d self._provision_tenant_private_network(context, tenant_id)"},{"line_number":212,"context_line":"            router \u003d self._provision_external_connectivity("},{"line_number":213,"context_line":"                context, default_external_network, subnet[\u0027id\u0027], tenant_id)"},{"line_number":214,"context_line":"            self._update("},{"line_number":215,"context_line":"                context, subnet[\u0027network_id\u0027],"},{"line_number":216,"context_line":"                subnet[\u0027id\u0027], router[\u0027id\u0027], tenant_id)"},{"line_number":217,"context_line":"        network \u003d self._get_auto_allocated_network(context, tenant_id)"}],"source_content_type":"text/x-python","patch_set":19,"id":"5a5ae5dd_4cc23d87","line":214,"range":{"start_line":214,"start_character":17,"end_line":214,"end_character":24},"in_reply_to":"5a5ae5dd_80dd5e44","updated":"2016-02-03 20:47:03.000000000","message":"that\u0027s right.","commit_id":"1bee8924e3d6efabca6897a2edbfb55234c272ff"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d86e5504625e8be0b59cfd81d257e7036d974718","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        The topology will be provisioned upon return, if network is missing."},{"line_number":91,"context_line":"        \"\"\""},{"line_number":92,"context_line":"        tenant_id \u003d self._validate(context, tenant_id)"},{"line_number":93,"context_line":"        try:"},{"line_number":94,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":95,"context_line":"                # Check for an existent topology"},{"line_number":96,"context_line":"                network_id \u003d self._get_auto_allocated_network("}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_c2e6beb3","line":93,"range":{"start_line":93,"start_character":8,"end_line":93,"end_character":11},"updated":"2016-02-04 18:55:44.000000000","message":"Isn\u0027t L111 really the only line that should be under this try/catch?  Wait, but it has to go outside of the with statement to catch the duplicate entry, right?  Bummer.  Oh well.","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"f7a0b7d9b5acd469e9f657934cfa477cae32138f","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        The topology will be provisioned upon return, if network is missing."},{"line_number":91,"context_line":"        \"\"\""},{"line_number":92,"context_line":"        tenant_id \u003d self._validate(context, tenant_id)"},{"line_number":93,"context_line":"        try:"},{"line_number":94,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":95,"context_line":"                # Check for an existent topology"},{"line_number":96,"context_line":"                network_id \u003d self._get_auto_allocated_network("}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_d419987a","line":93,"range":{"start_line":93,"start_character":8,"end_line":93,"end_character":11},"in_reply_to":"5a5ae5dd_6e2d7145","updated":"2016-02-04 20:41:21.000000000","message":"not quite. It\u0027ll be clear in the next PS","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"db05f90da63515fbde69efba32203115cb245835","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        The topology will be provisioned upon return, if network is missing."},{"line_number":91,"context_line":"        \"\"\""},{"line_number":92,"context_line":"        tenant_id \u003d self._validate(context, tenant_id)"},{"line_number":93,"context_line":"        try:"},{"line_number":94,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":95,"context_line":"                # Check for an existent topology"},{"line_number":96,"context_line":"                network_id \u003d self._get_auto_allocated_network("}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_c559e85b","line":93,"range":{"start_line":93,"start_character":8,"end_line":93,"end_character":11},"in_reply_to":"5a5ae5dd_c2e6beb3","updated":"2016-02-04 19:24:30.000000000","message":"yes, this gotta wrap the entire transaction.","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"e7fed3636314cb4fa8d15b13862ea8b2f849afe1","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        The topology will be provisioned upon return, if network is missing."},{"line_number":91,"context_line":"        \"\"\""},{"line_number":92,"context_line":"        tenant_id \u003d self._validate(context, tenant_id)"},{"line_number":93,"context_line":"        try:"},{"line_number":94,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":95,"context_line":"                # Check for an existent topology"},{"line_number":96,"context_line":"                network_id \u003d self._get_auto_allocated_network("}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_6e2d7145","line":93,"range":{"start_line":93,"start_character":8,"end_line":93,"end_character":11},"in_reply_to":"5a5ae5dd_c559e85b","updated":"2016-02-04 20:05:05.000000000","message":"You could do like this:\n\n- https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/plugin.py#L1042","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d86e5504625e8be0b59cfd81d257e7036d974718","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                network_id \u003d subnets[0][\u0027network_id\u0027]"},{"line_number":109,"context_line":"                router \u003d self._provision_external_connectivity("},{"line_number":110,"context_line":"                    context, default_external_network, subnets, tenant_id)"},{"line_number":111,"context_line":"                self._save(context, network_id, router[\u0027id\u0027], tenant_id)"},{"line_number":112,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":113,"context_line":"            # oops, we hit a race!"},{"line_number":114,"context_line":"            with excutils.save_and_reraise_exception(reraise\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_25923c58","line":111,"range":{"start_line":111,"start_character":21,"end_line":111,"end_character":26},"updated":"2016-02-04 18:55:44.000000000","message":"If this raises a duplicate entry exception, would that roll back the entire transaction making it unnecessary to cleanup?  Maybe that is wishful thinking.","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"db05f90da63515fbde69efba32203115cb245835","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                network_id \u003d subnets[0][\u0027network_id\u0027]"},{"line_number":109,"context_line":"                router \u003d self._provision_external_connectivity("},{"line_number":110,"context_line":"                    context, default_external_network, subnets, tenant_id)"},{"line_number":111,"context_line":"                self._save(context, network_id, router[\u0027id\u0027], tenant_id)"},{"line_number":112,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":113,"context_line":"            # oops, we hit a race!"},{"line_number":114,"context_line":"            with excutils.save_and_reraise_exception(reraise\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_8805457b","line":111,"range":{"start_line":111,"start_character":21,"end_line":111,"end_character":26},"in_reply_to":"5a5ae5dd_25923c58","updated":"2016-02-04 19:24:30.000000000","message":"This would only roll back the DB to its state before the API call. Any backend changes that may have resulted by these calls will linger, but that\u0027s the nature of how busted our plugin hierarchy has been designed.","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"570e1f93817492ac57f4fc7924ad376cd071b909","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                network_id \u003d subnets[0][\u0027network_id\u0027]"},{"line_number":109,"context_line":"                router \u003d self._provision_external_connectivity("},{"line_number":110,"context_line":"                    context, default_external_network, subnets, tenant_id)"},{"line_number":111,"context_line":"                self._save(context, network_id, router[\u0027id\u0027], tenant_id)"},{"line_number":112,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":113,"context_line":"            # oops, we hit a race!"},{"line_number":114,"context_line":"            with excutils.save_and_reraise_exception(reraise\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_ee6961bf","line":111,"range":{"start_line":111,"start_character":21,"end_line":111,"end_character":26},"in_reply_to":"5a5ae5dd_8805457b","updated":"2016-02-04 19:55:16.000000000","message":"Bummer","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d86e5504625e8be0b59cfd81d257e7036d974718","unresolved":false,"context_lines":[{"line_number":111,"context_line":"                self._save(context, network_id, router[\u0027id\u0027], tenant_id)"},{"line_number":112,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":113,"context_line":"            # oops, we hit a race!"},{"line_number":114,"context_line":"            with excutils.save_and_reraise_exception(reraise\u003dFalse):"},{"line_number":115,"context_line":"                network_id \u003d self._get_auto_allocated_network("},{"line_number":116,"context_line":"                    context, tenant_id)"},{"line_number":117,"context_line":"                LOG.error(_LE(\"Multiple auto-allocated networks detected for \""}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_c2fd5e3b","line":114,"range":{"start_line":114,"start_character":26,"end_line":114,"end_character":66},"updated":"2016-02-04 18:55:44.000000000","message":"Why?  Couldn\u0027t you just remove this?","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"570e1f93817492ac57f4fc7924ad376cd071b909","unresolved":false,"context_lines":[{"line_number":111,"context_line":"                self._save(context, network_id, router[\u0027id\u0027], tenant_id)"},{"line_number":112,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":113,"context_line":"            # oops, we hit a race!"},{"line_number":114,"context_line":"            with excutils.save_and_reraise_exception(reraise\u003dFalse):"},{"line_number":115,"context_line":"                network_id \u003d self._get_auto_allocated_network("},{"line_number":116,"context_line":"                    context, tenant_id)"},{"line_number":117,"context_line":"                LOG.error(_LE(\"Multiple auto-allocated networks detected for \""}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_4e5bd52e","line":114,"range":{"start_line":114,"start_character":26,"end_line":114,"end_character":66},"in_reply_to":"5a5ae5dd_a8992121","updated":"2016-02-04 19:55:16.000000000","message":"By this, I just meant that I don\u0027t think you need to save_and_reraise_exception because you\u0027re not reraising under any condition.  I think you can just remove this line of code and indent the following lines 4 spaces less.","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"db05f90da63515fbde69efba32203115cb245835","unresolved":false,"context_lines":[{"line_number":111,"context_line":"                self._save(context, network_id, router[\u0027id\u0027], tenant_id)"},{"line_number":112,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":113,"context_line":"            # oops, we hit a race!"},{"line_number":114,"context_line":"            with excutils.save_and_reraise_exception(reraise\u003dFalse):"},{"line_number":115,"context_line":"                network_id \u003d self._get_auto_allocated_network("},{"line_number":116,"context_line":"                    context, tenant_id)"},{"line_number":117,"context_line":"                LOG.error(_LE(\"Multiple auto-allocated networks detected for \""}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_a8992121","line":114,"range":{"start_line":114,"start_character":26,"end_line":114,"end_character":66},"in_reply_to":"5a5ae5dd_c2fd5e3b","updated":"2016-02-04 19:24:30.000000000","message":"I could delete what\u0027s been generated as part of the duplicated topology, but that would open up to an explosion of things that could go wrong, and I\u0027d rather defer that to the admin in the chance this could happen. Let me think about this a little more, if I don\u0027t end up doing it now, I will still add a TODO and take care of it when I don\u0027t have much pressure.","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d86e5504625e8be0b59cfd81d257e7036d974718","unresolved":false,"context_lines":[{"line_number":114,"context_line":"            with excutils.save_and_reraise_exception(reraise\u003dFalse):"},{"line_number":115,"context_line":"                network_id \u003d self._get_auto_allocated_network("},{"line_number":116,"context_line":"                    context, tenant_id)"},{"line_number":117,"context_line":"                LOG.error(_LE(\"Multiple auto-allocated networks detected for \""},{"line_number":118,"context_line":"                              \"tenant %(tenant)s. Keep only resources \""},{"line_number":119,"context_line":"                              \"associated to auto allocated network \""},{"line_number":120,"context_line":"                              \"%(network)s\"),"},{"line_number":121,"context_line":"                          {\u0027tenant\u0027: tenant_id, \u0027network\u0027: network_id})"},{"line_number":122,"context_line":"        return self._response(network_id, tenant_id, fields\u003dfields)"},{"line_number":123,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_026ff62b","line":120,"range":{"start_line":117,"start_character":31,"end_line":120,"end_character":45},"updated":"2016-02-04 18:55:44.000000000","message":"So, back on my comment in PS19.  You have all of the ids of the resources which should be removed.  Above on L96 - 109, you have network_id, subnets, and router.  I think giving these to the operator would result in a slightly better experience for everyone.\n\nThis message makes my head spin with scenarios where the operator accidentally deletes the ones that he should\u0027ve kept or deletes other unrelated tenant resources that weren\u0027t auto-provisioned.  I still think telling the operator the exact ids of the ones that *should* be deleted would avoid some potential pitfalls.\n\nIf you hit the race multiple times, then you\u0027ll hit this log statement once for each of them.  That\u0027s good.  The operator will see one line for each set that should be cleaned up.\n\nActually, given that you have this data already, automatic cleanup isn\u0027t that much of a stretch.  But, that could be done in a follow-on or something.  I wouldn\u0027t insist on it.","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"db05f90da63515fbde69efba32203115cb245835","unresolved":false,"context_lines":[{"line_number":114,"context_line":"            with excutils.save_and_reraise_exception(reraise\u003dFalse):"},{"line_number":115,"context_line":"                network_id \u003d self._get_auto_allocated_network("},{"line_number":116,"context_line":"                    context, tenant_id)"},{"line_number":117,"context_line":"                LOG.error(_LE(\"Multiple auto-allocated networks detected for \""},{"line_number":118,"context_line":"                              \"tenant %(tenant)s. Keep only resources \""},{"line_number":119,"context_line":"                              \"associated to auto allocated network \""},{"line_number":120,"context_line":"                              \"%(network)s\"),"},{"line_number":121,"context_line":"                          {\u0027tenant\u0027: tenant_id, \u0027network\u0027: network_id})"},{"line_number":122,"context_line":"        return self._response(network_id, tenant_id, fields\u003dfields)"},{"line_number":123,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_884b0587","line":120,"range":{"start_line":117,"start_character":31,"end_line":120,"end_character":45},"in_reply_to":"5a5ae5dd_026ff62b","updated":"2016-02-04 19:24:30.000000000","message":"Ok, let look into it.","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"0f23ad7d059694d39d76e06aed2a684f1ec38486","unresolved":false,"context_lines":[{"line_number":114,"context_line":"            with excutils.save_and_reraise_exception(reraise\u003dFalse):"},{"line_number":115,"context_line":"                network_id \u003d self._get_auto_allocated_network("},{"line_number":116,"context_line":"                    context, tenant_id)"},{"line_number":117,"context_line":"                LOG.error(_LE(\"Multiple auto-allocated networks detected for \""},{"line_number":118,"context_line":"                              \"tenant %(tenant)s. Keep only resources \""},{"line_number":119,"context_line":"                              \"associated to auto allocated network \""},{"line_number":120,"context_line":"                              \"%(network)s\"),"},{"line_number":121,"context_line":"                          {\u0027tenant\u0027: tenant_id, \u0027network\u0027: network_id})"},{"line_number":122,"context_line":"        return self._response(network_id, tenant_id, fields\u003dfields)"},{"line_number":123,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_ceed85f0","line":120,"range":{"start_line":117,"start_character":31,"end_line":120,"end_character":45},"in_reply_to":"5a5ae5dd_884b0587","updated":"2016-02-04 20:02:18.000000000","message":"Don\u0027t do cleanup!  I didn\u0027t mean to get hung up on cleanup.  Mostly, I just thinking giving the operator the ids that he should delete would make me happy.","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d86e5504625e8be0b59cfd81d257e7036d974718","unresolved":false,"context_lines":[{"line_number":207,"context_line":"            network \u003d p_utils.create_network("},{"line_number":208,"context_line":"                self.core_plugin, context, {\u0027network\u0027: network_args})"},{"line_number":209,"context_line":"            subnets \u003d []"},{"line_number":210,"context_line":"            for ip_version in self._get_supported_versions(context):"},{"line_number":211,"context_line":"                subnet_args \u003d {"},{"line_number":212,"context_line":"                    \u0027name\u0027: \u0027auto_allocated_subnet_v%s\u0027 % ip_version,"},{"line_number":213,"context_line":"                    \u0027network_id\u0027: network[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_657274af","line":210,"range":{"start_line":210,"start_character":12,"end_line":210,"end_character":58},"updated":"2016-02-04 18:55:44.000000000","message":"I think I like this.  Seems like the right thing to do.","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"db05f90da63515fbde69efba32203115cb245835","unresolved":false,"context_lines":[{"line_number":207,"context_line":"            network \u003d p_utils.create_network("},{"line_number":208,"context_line":"                self.core_plugin, context, {\u0027network\u0027: network_args})"},{"line_number":209,"context_line":"            subnets \u003d []"},{"line_number":210,"context_line":"            for ip_version in self._get_supported_versions(context):"},{"line_number":211,"context_line":"                subnet_args \u003d {"},{"line_number":212,"context_line":"                    \u0027name\u0027: \u0027auto_allocated_subnet_v%s\u0027 % ip_version,"},{"line_number":213,"context_line":"                    \u0027network_id\u0027: network[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_285bf155","line":210,"range":{"start_line":210,"start_character":12,"end_line":210,"end_character":58},"in_reply_to":"5a5ae5dd_657274af","updated":"2016-02-04 19:24:30.000000000","message":"sweet","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7fe07be846e47dd4a4e3c3365f5dd4e3e53cccc3","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        resources.EXTERNAL_NETWORK, events.BEFORE_UPDATE)"},{"line_number":81,"context_line":"    # NOTE(armax): there are no BEFORE_CREATE/BEFORE_UPDATE callbacks"},{"line_number":82,"context_line":"    # that will stop actions on networks, subnets and routers, thus"},{"line_number":83,"context_line":"    # there are no safeguards to tenants who modify resources that"},{"line_number":84,"context_line":"    # belong to an auto-allocated topology."},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def get_auto_allocated_topology(self, context, tenant_id, fields\u003dNone):"}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_bf9ccb0e","line":83,"updated":"2016-02-05 16:34:14.000000000","message":"at this point I am going to take care of HenryG\u0027s concern here and wipe out the entire topology is the user is deleting either the subnet or the router.","commit_id":"03386460fb46fb9970c4c705931717865a499497"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"1eba80c7a820e643975cd639cc07befbab29592f","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        The topology will be provisioned upon return, if network is missing."},{"line_number":90,"context_line":"        \"\"\""},{"line_number":91,"context_line":"        tenant_id \u003d self._validate(context, tenant_id)"},{"line_number":92,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":93,"context_line":"            # Check for an existent topology"},{"line_number":94,"context_line":"            network_id \u003d self._get_auto_allocated_network("},{"line_number":95,"context_line":"                context, tenant_id)"}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_6de000fb","line":92,"updated":"2016-02-05 08:56:43.000000000","message":"no transactions before calling the sacred core! I would rather this leave dangling resources like you do on L120 than violate the post_commit semantics of ML2","commit_id":"03386460fb46fb9970c4c705931717865a499497"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7fe07be846e47dd4a4e3c3365f5dd4e3e53cccc3","unresolved":false,"context_lines":[{"line_number":89,"context_line":"        The topology will be provisioned upon return, if network is missing."},{"line_number":90,"context_line":"        \"\"\""},{"line_number":91,"context_line":"        tenant_id \u003d self._validate(context, tenant_id)"},{"line_number":92,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":93,"context_line":"            # Check for an existent topology"},{"line_number":94,"context_line":"            network_id \u003d self._get_auto_allocated_network("},{"line_number":95,"context_line":"                context, tenant_id)"}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_3fb77b3b","line":92,"in_reply_to":"5a5ae5dd_6de000fb","updated":"2016-02-05 16:34:14.000000000","message":"I had mixed feeling about this one too, you convinced me.","commit_id":"03386460fb46fb9970c4c705931717865a499497"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"c6439f50ded4b8921099e740610b53b890126dd0","unresolved":false,"context_lines":[{"line_number":113,"context_line":"            # more bullet proof cleanup."},{"line_number":114,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":115,"context_line":"                self._save(context, network_id, router[\u0027id\u0027], tenant_id)"},{"line_number":116,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":117,"context_line":"            # TODO(armax): if we get here, more resources have been provisioned"},{"line_number":118,"context_line":"            # than necessary. Consider adding a cleanup handler, but for now"},{"line_number":119,"context_line":"            # punt it to the user."}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_a8cee62b","line":116,"updated":"2016-02-05 09:25:44.000000000","message":"Problem we discussed online: 2 VMs boot concurrently and create two networks. Next VM that boots without specifying a network will get the \u0027too many networks\u0027 exception from Nova.\n\nI propose that we leverage the is_default API attribute you\u0027ve added for this. Right now it\u0027s only being used to determine which external network is used, but the attribute is on every network. So move that column to the networks table and then the logic here would go:\n\ntry:\n    with context.session.being(subtransactions\u003dTrue):\n        self._save(...)\n    self.core_plugin.update_network(context, network_id, {\u0027network\u0027: {\u0027is_default\u0027: True}})\nexcept db_exc.DBDuplicateEntry:\n    # log error, trigger some cleanup\n    return self._get_auto_allo...\nreturn self._response(...\n\n\nThis ensures that the auto allocated network is visible from the networks that Nova retrieves. Then if Nova detects multiple networks for a tenant, it first iterates through looking for one with is_default\u003dTrue. If it finds one, it uses that; else, fail with the too many networks exception.","commit_id":"03386460fb46fb9970c4c705931717865a499497"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dec537bc30f5cc5731464ae42484fa216d6ab27f","unresolved":false,"context_lines":[{"line_number":113,"context_line":"            # more bullet proof cleanup."},{"line_number":114,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":115,"context_line":"                self._save(context, network_id, router[\u0027id\u0027], tenant_id)"},{"line_number":116,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":117,"context_line":"            # TODO(armax): if we get here, more resources have been provisioned"},{"line_number":118,"context_line":"            # than necessary. Consider adding a cleanup handler, but for now"},{"line_number":119,"context_line":"            # punt it to the user."}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_93978847","line":116,"in_reply_to":"5a5ae5dd_0d8a951b","updated":"2016-02-05 17:51:24.000000000","message":"Yup, I\u0027d that that we\u0027re aware, and will keep an eye on it, but I wouldn\u0027t sweat it just yet.","commit_id":"03386460fb46fb9970c4c705931717865a499497"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7fe07be846e47dd4a4e3c3365f5dd4e3e53cccc3","unresolved":false,"context_lines":[{"line_number":113,"context_line":"            # more bullet proof cleanup."},{"line_number":114,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":115,"context_line":"                self._save(context, network_id, router[\u0027id\u0027], tenant_id)"},{"line_number":116,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":117,"context_line":"            # TODO(armax): if we get here, more resources have been provisioned"},{"line_number":118,"context_line":"            # than necessary. Consider adding a cleanup handler, but for now"},{"line_number":119,"context_line":"            # punt it to the user."}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_bf328b5c","line":116,"in_reply_to":"5a5ae5dd_a8cee62b","updated":"2016-02-05 16:34:14.000000000","message":"You may have omitted a tiny particular in your discussion: this is an hypothetical failure mode, because until we know for sure how Nova uses this API, we can\u0027t really tell, and mine was a simple educated guess.\n\nThat said, I am now convinced that addressing the TODO is worth now more than later. As for your suggestion on leveraging is_default, I\u0027d rather see how Nova is going to use this, before going down this more intrusive path.","commit_id":"03386460fb46fb9970c4c705931717865a499497"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"61fa92140d6a164060495b5cf6927bd86eb98787","unresolved":false,"context_lines":[{"line_number":113,"context_line":"            # more bullet proof cleanup."},{"line_number":114,"context_line":"            with context.session.begin(subtransactions\u003dTrue):"},{"line_number":115,"context_line":"                self._save(context, network_id, router[\u0027id\u0027], tenant_id)"},{"line_number":116,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":117,"context_line":"            # TODO(armax): if we get here, more resources have been provisioned"},{"line_number":118,"context_line":"            # than necessary. Consider adding a cleanup handler, but for now"},{"line_number":119,"context_line":"            # punt it to the user."}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_0d8a951b","line":116,"in_reply_to":"5a5ae5dd_bf328b5c","updated":"2016-02-05 17:04:08.000000000","message":"Well it\u0027s definitely something we need to confirm. If they do just do a net-list from each compute node first, it will be a race condition waiting to happen once many VMs are launched successively.","commit_id":"03386460fb46fb9970c4c705931717865a499497"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"c6439f50ded4b8921099e740610b53b890126dd0","unresolved":false,"context_lines":[{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def _plugins_lookup(self):"},{"line_number":185,"context_line":"        \"\"\"Initialize plugins.\"\"\""},{"line_number":186,"context_line":"        self.core_plugin \u003d manager.NeutronManager.get_plugin()"},{"line_number":187,"context_line":"        self.l3_plugin \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":188,"context_line":"                 constants.L3_ROUTER_NAT)"},{"line_number":189,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_48cf3229","line":186,"updated":"2016-02-05 09:25:44.000000000","message":"You can avoid this function if you just make each of these properties.\n\n@property\ndef core_plugin(self):\n    if not getattr(self, \u0027_core_plugin\u0027, None):\n        self._core_plugin \u003d manager.NeutronManager.get_plugin()\n    return self._core_plugin","commit_id":"03386460fb46fb9970c4c705931717865a499497"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7fe07be846e47dd4a4e3c3365f5dd4e3e53cccc3","unresolved":false,"context_lines":[{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def _plugins_lookup(self):"},{"line_number":185,"context_line":"        \"\"\"Initialize plugins.\"\"\""},{"line_number":186,"context_line":"        self.core_plugin \u003d manager.NeutronManager.get_plugin()"},{"line_number":187,"context_line":"        self.l3_plugin \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":188,"context_line":"                 constants.L3_ROUTER_NAT)"},{"line_number":189,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_7f6f332b","line":186,"in_reply_to":"5a5ae5dd_48cf3229","updated":"2016-02-05 16:34:14.000000000","message":"Your suggestion is more lines of code per plugin for no apparent gain. You timed it yourself. This isn\u0027t a big deal.","commit_id":"03386460fb46fb9970c4c705931717865a499497"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"61fa92140d6a164060495b5cf6927bd86eb98787","unresolved":false,"context_lines":[{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def _plugins_lookup(self):"},{"line_number":185,"context_line":"        \"\"\"Initialize plugins.\"\"\""},{"line_number":186,"context_line":"        self.core_plugin \u003d manager.NeutronManager.get_plugin()"},{"line_number":187,"context_line":"        self.l3_plugin \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":188,"context_line":"                 constants.L3_ROUTER_NAT)"},{"line_number":189,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_eddff937","line":186,"in_reply_to":"5a5ae5dd_7f6f332b","updated":"2016-02-05 17:04:08.000000000","message":"It\u0027s not a performance or LOC thing. It\u0027s just having to call _plugins_lookup in every method that might use the plugins is a little wonky.","commit_id":"03386460fb46fb9970c4c705931717865a499497"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"dec537bc30f5cc5731464ae42484fa216d6ab27f","unresolved":false,"context_lines":[{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def _plugins_lookup(self):"},{"line_number":185,"context_line":"        \"\"\"Initialize plugins.\"\"\""},{"line_number":186,"context_line":"        self.core_plugin \u003d manager.NeutronManager.get_plugin()"},{"line_number":187,"context_line":"        self.l3_plugin \u003d manager.NeutronManager.get_service_plugins().get("},{"line_number":188,"context_line":"                 constants.L3_ROUTER_NAT)"},{"line_number":189,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_937ea817","line":186,"in_reply_to":"5a5ae5dd_eddff937","updated":"2016-02-05 17:51:24.000000000","message":"I find hard to believe a user of this plugin, but I can address your concern.","commit_id":"03386460fb46fb9970c4c705931717865a499497"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"c6439f50ded4b8921099e740610b53b890126dd0","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            s[\u0027ip_version\u0027] for s in default_subnet_pools if s"},{"line_number":198,"context_line":"        ]"},{"line_number":199,"context_line":"        if not ip_versions:"},{"line_number":200,"context_line":"            raise ValueError(_(\"No default pools available\"))"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"        return ip_versions"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_68d46ebc","line":200,"updated":"2016-02-05 09:25:44.000000000","message":"Maybe a new exception type?","commit_id":"03386460fb46fb9970c4c705931717865a499497"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7fe07be846e47dd4a4e3c3365f5dd4e3e53cccc3","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            s[\u0027ip_version\u0027] for s in default_subnet_pools if s"},{"line_number":198,"context_line":"        ]"},{"line_number":199,"context_line":"        if not ip_versions:"},{"line_number":200,"context_line":"            raise ValueError(_(\"No default pools available\"))"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"        return ip_versions"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_7f56535d","line":200,"in_reply_to":"5a5ae5dd_68d46ebc","updated":"2016-02-05 16:34:14.000000000","message":"Seems superfluous but ok","commit_id":"03386460fb46fb9970c4c705931717865a499497"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"c6439f50ded4b8921099e740610b53b890126dd0","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            LOG.exception(_LE(\"Unable to auto allocate topology \""},{"line_number":228,"context_line":"                              \"for tenant %s\"), tenant_id)"},{"line_number":229,"context_line":"            raise exceptions.AutoAllocationFailure("},{"line_number":230,"context_line":"                reason\u003d_(\"Deployment miss important pre-requisites, \""},{"line_number":231,"context_line":"                         \"like shared/default subnetpool setup\"))"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def _provision_external_connectivity("}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_08de1add","line":230,"updated":"2016-02-05 09:25:44.000000000","message":"missing* prerequisites*","commit_id":"03386460fb46fb9970c4c705931717865a499497"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7fe07be846e47dd4a4e3c3365f5dd4e3e53cccc3","unresolved":false,"context_lines":[{"line_number":227,"context_line":"            LOG.exception(_LE(\"Unable to auto allocate topology \""},{"line_number":228,"context_line":"                              \"for tenant %s\"), tenant_id)"},{"line_number":229,"context_line":"            raise exceptions.AutoAllocationFailure("},{"line_number":230,"context_line":"                reason\u003d_(\"Deployment miss important pre-requisites, \""},{"line_number":231,"context_line":"                         \"like shared/default subnetpool setup\"))"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def _provision_external_connectivity("}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_ffc80324","line":230,"in_reply_to":"5a5ae5dd_08de1add","updated":"2016-02-05 16:34:14.000000000","message":"My humble apologies","commit_id":"03386460fb46fb9970c4c705931717865a499497"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"b184d758cc004d28f10b410677c4cf11c11a61e8","unresolved":false,"context_lines":[{"line_number":37,"context_line":"from neutron.plugins.common import utils as p_utils"},{"line_number":38,"context_line":"from neutron.services.auto_allocate import exceptions"},{"line_number":39,"context_line":"from neutron.services.auto_allocate import models"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":43,"context_line":"IS_DEFAULT \u003d \u0027is_default\u0027"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_4ff11794","line":40,"updated":"2016-02-08 17:20:20.000000000","message":"1 too many new lines","commit_id":"66b9a7a84a0ac274c924824a6aa3cf7989408244"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"cbf06d046c917491d7f7fad3b0330fb533fbd8c0","unresolved":false,"context_lines":[{"line_number":37,"context_line":"from neutron.plugins.common import utils as p_utils"},{"line_number":38,"context_line":"from neutron.services.auto_allocate import exceptions"},{"line_number":39,"context_line":"from neutron.services.auto_allocate import models"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":43,"context_line":"IS_DEFAULT \u003d \u0027is_default\u0027"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_18a4c7b7","line":40,"in_reply_to":"5a5ae5dd_4ff11794","updated":"2016-02-08 18:04:34.000000000","message":"Done","commit_id":"66b9a7a84a0ac274c924824a6aa3cf7989408244"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"b184d758cc004d28f10b410677c4cf11c11a61e8","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":43,"context_line":"IS_DEFAULT \u003d \u0027is_default\u0027"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"def _extend_external_network_default(self, net_res, net_db):"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_ef8483d3","line":43,"updated":"2016-02-08 17:20:20.000000000","message":"nit but this should be defined in the extension","commit_id":"66b9a7a84a0ac274c924824a6aa3cf7989408244"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"cbf06d046c917491d7f7fad3b0330fb533fbd8c0","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":43,"context_line":"IS_DEFAULT \u003d \u0027is_default\u0027"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"def _extend_external_network_default(self, net_res, net_db):"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_f87d2b6d","line":43,"in_reply_to":"5a5ae5dd_ef8483d3","updated":"2016-02-08 18:04:34.000000000","message":"It causes a circular dependency if I do","commit_id":"66b9a7a84a0ac274c924824a6aa3cf7989408244"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"fb79b112db35f26d6760b4935fdcba4494a025c3","unresolved":false,"context_lines":[{"line_number":60,"context_line":"            filter_by(is_default\u003dTrue)).first()"},{"line_number":61,"context_line":"        if obj:"},{"line_number":62,"context_line":"            network_id \u003d obj.network_id"},{"line_number":63,"context_line":"            raise exceptions.DefaultExternalNetworkExists(net_id\u003dnetwork_id)"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    # Reflect the status of the is_default on the create/update request"},{"line_number":66,"context_line":"    obj \u003d (context.session.query(external_net_db.ExternalNetwork)."}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_0e0b412b","line":63,"updated":"2016-02-08 07:30:25.000000000","message":"This exception should not be raised if the user is doing the update on the network that is already the default. That should be an idempotent operation.","commit_id":"66b9a7a84a0ac274c924824a6aa3cf7989408244"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"cbf06d046c917491d7f7fad3b0330fb533fbd8c0","unresolved":false,"context_lines":[{"line_number":60,"context_line":"            filter_by(is_default\u003dTrue)).first()"},{"line_number":61,"context_line":"        if obj:"},{"line_number":62,"context_line":"            network_id \u003d obj.network_id"},{"line_number":63,"context_line":"            raise exceptions.DefaultExternalNetworkExists(net_id\u003dnetwork_id)"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    # Reflect the status of the is_default on the create/update request"},{"line_number":66,"context_line":"    obj \u003d (context.session.query(external_net_db.ExternalNetwork)."}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_58e6bf44","line":63,"in_reply_to":"5a5ae5dd_0e0b412b","updated":"2016-02-08 18:04:34.000000000","message":"Done","commit_id":"66b9a7a84a0ac274c924824a6aa3cf7989408244"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"b184d758cc004d28f10b410677c4cf11c11a61e8","unresolved":false,"context_lines":[{"line_number":127,"context_line":""},{"line_number":128,"context_line":"    def _validate(self, context, tenant_id):"},{"line_number":129,"context_line":"        \"\"\"Validate and return the tenant to be associated to the topology.\"\"\""},{"line_number":130,"context_line":"        if tenant_id \u003d\u003d \u0027None\u0027:"},{"line_number":131,"context_line":"            # NOTE(HenryG): the client might be sending us astray by"},{"line_number":132,"context_line":"            # passing a bogus tenant; this is really meant to be the"},{"line_number":133,"context_line":"            # tenant issuing the request, therefore let\u0027s get the it"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_2f9b7b19","line":130,"updated":"2016-02-08 17:20:20.000000000","message":"if the client is sending us a bogus tenant we should be returning an exception","commit_id":"66b9a7a84a0ac274c924824a6aa3cf7989408244"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"cbf06d046c917491d7f7fad3b0330fb533fbd8c0","unresolved":false,"context_lines":[{"line_number":127,"context_line":""},{"line_number":128,"context_line":"    def _validate(self, context, tenant_id):"},{"line_number":129,"context_line":"        \"\"\"Validate and return the tenant to be associated to the topology.\"\"\""},{"line_number":130,"context_line":"        if tenant_id \u003d\u003d \u0027None\u0027:"},{"line_number":131,"context_line":"            # NOTE(HenryG): the client might be sending us astray by"},{"line_number":132,"context_line":"            # passing a bogus tenant; this is really meant to be the"},{"line_number":133,"context_line":"            # tenant issuing the request, therefore let\u0027s get the it"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_383d038f","line":130,"in_reply_to":"5a5ae5dd_2f9b7b19","updated":"2016-02-08 18:04:34.000000000","message":"None is not actually a \u0027bogus\u0027 tenant. It\u0027s the tenant him/herself. The exception for bogus tenant id is raised below. I\u0027ll rephrase.","commit_id":"66b9a7a84a0ac274c924824a6aa3cf7989408244"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"3dee967026d952bd253d8c594d4de893a7c4fe21","unresolved":false,"context_lines":[{"line_number":175,"context_line":"                          \"for deployment, please create/assign one to \""},{"line_number":176,"context_line":"                          \"allow auto-allocation to work correctly.\"))"},{"line_number":177,"context_line":"            raise exceptions.AutoAllocationFailure("},{"line_number":178,"context_line":"                reason\u003d_(\"Deployment miss important pre-requisites, \""},{"line_number":179,"context_line":"                         \"like a default router:external network\"))"},{"line_number":180,"context_line":"        if len(default_external_networks) \u003e 1:"},{"line_number":181,"context_line":"            LOG.error(_LE(\"Multiple external default networks detected. \""},{"line_number":182,"context_line":"                          \"Network %s is true \u0027default\u0027.\"),"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_1bf28f06","line":179,"range":{"start_line":178,"start_character":26,"end_line":179,"end_character":65},"updated":"2016-02-08 14:38:26.000000000","message":"Using the CLI it looks like this:\n\n  $ neutron auto-allocated-topology-show \n  Deployment error: Deployment miss important pre-requisites, like a default router:external network.\n\nSo I think it would be better to just have \"No default router:external network\" for the reason.","commit_id":"66b9a7a84a0ac274c924824a6aa3cf7989408244"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"cbf06d046c917491d7f7fad3b0330fb533fbd8c0","unresolved":false,"context_lines":[{"line_number":175,"context_line":"                          \"for deployment, please create/assign one to \""},{"line_number":176,"context_line":"                          \"allow auto-allocation to work correctly.\"))"},{"line_number":177,"context_line":"            raise exceptions.AutoAllocationFailure("},{"line_number":178,"context_line":"                reason\u003d_(\"Deployment miss important pre-requisites, \""},{"line_number":179,"context_line":"                         \"like a default router:external network\"))"},{"line_number":180,"context_line":"        if len(default_external_networks) \u003e 1:"},{"line_number":181,"context_line":"            LOG.error(_LE(\"Multiple external default networks detected. \""},{"line_number":182,"context_line":"                          \"Network %s is true \u0027default\u0027.\"),"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_b849f332","line":179,"range":{"start_line":178,"start_character":26,"end_line":179,"end_character":65},"in_reply_to":"5a5ae5dd_1bf28f06","updated":"2016-02-08 18:04:34.000000000","message":"Done","commit_id":"66b9a7a84a0ac274c924824a6aa3cf7989408244"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"3dee967026d952bd253d8c594d4de893a7c4fe21","unresolved":false,"context_lines":[{"line_number":228,"context_line":"            if network:"},{"line_number":229,"context_line":"                self._cleanup(context, network[\u0027id\u0027])"},{"line_number":230,"context_line":"            raise exceptions.AutoAllocationFailure("},{"line_number":231,"context_line":"                reason\u003d_(\"Unable to provide tenant private network\"))"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def _provision_external_connectivity("},{"line_number":234,"context_line":"        self, context, default_external_network, subnets, tenant_id):"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_fb24530d","line":231,"range":{"start_line":231,"start_character":26,"end_line":231,"end_character":66},"updated":"2016-02-08 14:38:26.000000000","message":"IMO (after a lot of testing) this is not very friendly. In a follow-up improvement we can catch each missing resource and error case individually and give a more precise response.","commit_id":"66b9a7a84a0ac274c924824a6aa3cf7989408244"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"cbf06d046c917491d7f7fad3b0330fb533fbd8c0","unresolved":false,"context_lines":[{"line_number":228,"context_line":"            if network:"},{"line_number":229,"context_line":"                self._cleanup(context, network[\u0027id\u0027])"},{"line_number":230,"context_line":"            raise exceptions.AutoAllocationFailure("},{"line_number":231,"context_line":"                reason\u003d_(\"Unable to provide tenant private network\"))"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def _provision_external_connectivity("},{"line_number":234,"context_line":"        self, context, default_external_network, subnets, tenant_id):"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_9823d755","line":231,"range":{"start_line":231,"start_character":26,"end_line":231,"end_character":66},"in_reply_to":"5a5ae5dd_6f009302","updated":"2016-02-08 18:04:34.000000000","message":"I thought the method name was pretty clear. So the former","commit_id":"66b9a7a84a0ac274c924824a6aa3cf7989408244"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"b184d758cc004d28f10b410677c4cf11c11a61e8","unresolved":false,"context_lines":[{"line_number":228,"context_line":"            if network:"},{"line_number":229,"context_line":"                self._cleanup(context, network[\u0027id\u0027])"},{"line_number":230,"context_line":"            raise exceptions.AutoAllocationFailure("},{"line_number":231,"context_line":"                reason\u003d_(\"Unable to provide tenant private network\"))"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def _provision_external_connectivity("},{"line_number":234,"context_line":"        self, context, default_external_network, subnets, tenant_id):"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_6f009302","line":231,"range":{"start_line":231,"start_character":26,"end_line":231,"end_character":66},"in_reply_to":"5a5ae5dd_fb24530d","updated":"2016-02-08 17:20:20.000000000","message":"are we providing a private network or an external network?","commit_id":"66b9a7a84a0ac274c924824a6aa3cf7989408244"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"cbf06d046c917491d7f7fad3b0330fb533fbd8c0","unresolved":false,"context_lines":[{"line_number":228,"context_line":"            if network:"},{"line_number":229,"context_line":"                self._cleanup(context, network[\u0027id\u0027])"},{"line_number":230,"context_line":"            raise exceptions.AutoAllocationFailure("},{"line_number":231,"context_line":"                reason\u003d_(\"Unable to provide tenant private network\"))"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def _provision_external_connectivity("},{"line_number":234,"context_line":"        self, context, default_external_network, subnets, tenant_id):"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_182fe73f","line":231,"range":{"start_line":231,"start_character":26,"end_line":231,"end_character":66},"in_reply_to":"5a5ae5dd_fb24530d","updated":"2016-02-08 18:04:34.000000000","message":"there\u0027s really a few things that could go wrong here. I am not sure we should expose too much details to the user, if the deployment is not setup correctly.","commit_id":"66b9a7a84a0ac274c924824a6aa3cf7989408244"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"3082697c4931e81342bfe76e4f0af081f68f3a2d","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        if tenant_id \u003d\u003d \u0027None\u0027:"},{"line_number":130,"context_line":"            # NOTE(HenryG): the client might be sending us astray by"},{"line_number":131,"context_line":"            # passing no tenant; this is really meant to be the tenant"},{"line_number":132,"context_line":"            # issuing the request, therefore let\u0027s get the it from the"},{"line_number":133,"context_line":"            # context"},{"line_number":134,"context_line":"            tenant_id \u003d context.tenant_id"},{"line_number":135,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"5a5ae5dd_e40a621f","line":132,"range":{"start_line":132,"start_character":55,"end_line":132,"end_character":58},"updated":"2016-02-08 19:55:20.000000000","message":"nit:  remove \"the\" if you get another chance.","commit_id":"1ded953557e617c676ec7637d510f7098c0c566c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"61b0cd2672fba0d8571fdb6ab3c6bc0d753f650e","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        if tenant_id \u003d\u003d \u0027None\u0027:"},{"line_number":130,"context_line":"            # NOTE(HenryG): the client might be sending us astray by"},{"line_number":131,"context_line":"            # passing no tenant; this is really meant to be the tenant"},{"line_number":132,"context_line":"            # issuing the request, therefore let\u0027s get the it from the"},{"line_number":133,"context_line":"            # context"},{"line_number":134,"context_line":"            tenant_id \u003d context.tenant_id"},{"line_number":135,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"5a5ae5dd_6b915552","line":132,"range":{"start_line":132,"start_character":55,"end_line":132,"end_character":58},"in_reply_to":"5a5ae5dd_e40a621f","updated":"2016-02-08 23:17:11.000000000","message":"Done","commit_id":"1ded953557e617c676ec7637d510f7098c0c566c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"3082697c4931e81342bfe76e4f0af081f68f3a2d","unresolved":false,"context_lines":[{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    def _get_auto_allocated_network(self, context, tenant_id):"},{"line_number":146,"context_line":"        \"\"\"Get the auto allocated network for the tenant.\"\"\""},{"line_number":147,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":148,"context_line":"            network \u003d (context.session.query(models.AutoAllocatedTopology)."},{"line_number":149,"context_line":"                filter_by(tenant_id\u003dtenant_id).first())"},{"line_number":150,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"5a5ae5dd_a43f8ab2","line":147,"range":{"start_line":147,"start_character":8,"end_line":147,"end_character":57},"updated":"2016-02-08 19:55:20.000000000","message":"Not sure why this has to be in a transaction.  Just a single query, right?  Is there something about a transaction that I\u0027m forgetting?","commit_id":"1ded953557e617c676ec7637d510f7098c0c566c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"61b0cd2672fba0d8571fdb6ab3c6bc0d753f650e","unresolved":false,"context_lines":[{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    def _get_auto_allocated_network(self, context, tenant_id):"},{"line_number":146,"context_line":"        \"\"\"Get the auto allocated network for the tenant.\"\"\""},{"line_number":147,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":148,"context_line":"            network \u003d (context.session.query(models.AutoAllocatedTopology)."},{"line_number":149,"context_line":"                filter_by(tenant_id\u003dtenant_id).first())"},{"line_number":150,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"5a5ae5dd_eb2ea5f9","line":147,"range":{"start_line":147,"start_character":8,"end_line":147,"end_character":57},"in_reply_to":"5a5ae5dd_a43f8ab2","updated":"2016-02-08 23:17:11.000000000","message":"No specific reason, I mostly did it as a future-proofing exercise","commit_id":"1ded953557e617c676ec7637d510f7098c0c566c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"3082697c4931e81342bfe76e4f0af081f68f3a2d","unresolved":false,"context_lines":[{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    def _get_default_external_network(self, context):"},{"line_number":163,"context_line":"        \"\"\"Get the default external network for the deployment.\"\"\""},{"line_number":164,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":165,"context_line":"            default_external_networks \u003d (context.session.query("},{"line_number":166,"context_line":"                external_net_db.ExternalNetwork)."},{"line_number":167,"context_line":"                filter_by(is_default\u003dsql.true())."}],"source_content_type":"text/x-python","patch_set":24,"id":"5a5ae5dd_449ed663","line":164,"range":{"start_line":164,"start_character":8,"end_line":164,"end_character":56},"updated":"2016-02-08 19:55:20.000000000","message":"ditto","commit_id":"1ded953557e617c676ec7637d510f7098c0c566c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"61b0cd2672fba0d8571fdb6ab3c6bc0d753f650e","unresolved":false,"context_lines":[{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    def _get_default_external_network(self, context):"},{"line_number":163,"context_line":"        \"\"\"Get the default external network for the deployment.\"\"\""},{"line_number":164,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":165,"context_line":"            default_external_networks \u003d (context.session.query("},{"line_number":166,"context_line":"                external_net_db.ExternalNetwork)."},{"line_number":167,"context_line":"                filter_by(is_default\u003dsql.true())."}],"source_content_type":"text/x-python","patch_set":24,"id":"5a5ae5dd_0b48e1cb","line":164,"range":{"start_line":164,"start_character":8,"end_line":164,"end_character":56},"in_reply_to":"5a5ae5dd_449ed663","updated":"2016-02-08 23:17:11.000000000","message":"ditto","commit_id":"1ded953557e617c676ec7637d510f7098c0c566c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"3082697c4931e81342bfe76e4f0af081f68f3a2d","unresolved":false,"context_lines":[{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def _provision_tenant_private_network(self, context, tenant_id):"},{"line_number":200,"context_line":"        \"\"\"Create a tenant private network/subnets.\"\"\""},{"line_number":201,"context_line":"        network \u003d None"},{"line_number":202,"context_line":"        try:"},{"line_number":203,"context_line":"            network_args \u003d {"},{"line_number":204,"context_line":"                \u0027name\u0027: \u0027auto_allocated_network\u0027,"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a5ae5dd_a4f6aa8c","line":201,"range":{"start_line":201,"start_character":8,"end_line":201,"end_character":15},"updated":"2016-02-08 19:55:20.000000000","message":"nit:  Is this an indication that the try block is too broad?  This and the list of three exceptions on L222 all caught in the same except.  I guess if it were me, I\u0027d tighten up the try/except block to make it clearer where the various exceptions are expected to be raised.  In general, I think tighter try / except blocks makes for more precise error handling and messages to the user.","commit_id":"1ded953557e617c676ec7637d510f7098c0c566c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"61b0cd2672fba0d8571fdb6ab3c6bc0d753f650e","unresolved":false,"context_lines":[{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    def _provision_tenant_private_network(self, context, tenant_id):"},{"line_number":200,"context_line":"        \"\"\"Create a tenant private network/subnets.\"\"\""},{"line_number":201,"context_line":"        network \u003d None"},{"line_number":202,"context_line":"        try:"},{"line_number":203,"context_line":"            network_args \u003d {"},{"line_number":204,"context_line":"                \u0027name\u0027: \u0027auto_allocated_network\u0027,"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a5ae5dd_4b01f938","line":201,"range":{"start_line":201,"start_character":8,"end_line":201,"end_character":15},"in_reply_to":"5a5ae5dd_a4f6aa8c","updated":"2016-02-08 23:17:11.000000000","message":"I agree in principle, but in this case the error handling is the same (log and raise)...and hence I did it only once. I could split it but other duplication/conditionals would arise.","commit_id":"1ded953557e617c676ec7637d510f7098c0c566c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"3082697c4931e81342bfe76e4f0af081f68f3a2d","unresolved":false,"context_lines":[{"line_number":283,"context_line":"    def _cleanup(self, context, network_id\u003dNone, router_id\u003dNone, subnets\u003dNone):"},{"line_number":284,"context_line":"        \"\"\"Clean up auto allocated resources.\"\"\""},{"line_number":285,"context_line":"        if router_id:"},{"line_number":286,"context_line":"            subnets \u003d subnets or []"},{"line_number":287,"context_line":"            for subnet in subnets:"},{"line_number":288,"context_line":"                self.l3_plugin.remove_router_interface("},{"line_number":289,"context_line":"                    context, router_id, {\u0027subnet_id\u0027: subnet[\u0027id\u0027]})"},{"line_number":290,"context_line":"            self.l3_plugin.delete_router(context, router_id)"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a5ae5dd_384ade55","line":287,"range":{"start_line":286,"start_character":12,"end_line":287,"end_character":34},"updated":"2016-02-08 19:55:20.000000000","message":"nit: Rather than modifying the parameter:\n\n  for subnet in subnets or []:","commit_id":"1ded953557e617c676ec7637d510f7098c0c566c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"61b0cd2672fba0d8571fdb6ab3c6bc0d753f650e","unresolved":false,"context_lines":[{"line_number":283,"context_line":"    def _cleanup(self, context, network_id\u003dNone, router_id\u003dNone, subnets\u003dNone):"},{"line_number":284,"context_line":"        \"\"\"Clean up auto allocated resources.\"\"\""},{"line_number":285,"context_line":"        if router_id:"},{"line_number":286,"context_line":"            subnets \u003d subnets or []"},{"line_number":287,"context_line":"            for subnet in subnets:"},{"line_number":288,"context_line":"                self.l3_plugin.remove_router_interface("},{"line_number":289,"context_line":"                    context, router_id, {\u0027subnet_id\u0027: subnet[\u0027id\u0027]})"},{"line_number":290,"context_line":"            self.l3_plugin.delete_router(context, router_id)"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a5ae5dd_4b2499c5","line":287,"range":{"start_line":286,"start_character":12,"end_line":287,"end_character":34},"in_reply_to":"5a5ae5dd_384ade55","updated":"2016-02-08 23:17:11.000000000","message":"something stopped me from doing it...I am gonna have to remember...but if I don\u0027t than this makes sense :)","commit_id":"1ded953557e617c676ec7637d510f7098c0c566c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"3082697c4931e81342bfe76e4f0af081f68f3a2d","unresolved":false,"context_lines":[{"line_number":288,"context_line":"                self.l3_plugin.remove_router_interface("},{"line_number":289,"context_line":"                    context, router_id, {\u0027subnet_id\u0027: subnet[\u0027id\u0027]})"},{"line_number":290,"context_line":"            self.l3_plugin.delete_router(context, router_id)"},{"line_number":291,"context_line":"            if subnets and not network_id:"},{"line_number":292,"context_line":"                network_id \u003d subnets[0][\u0027network_id\u0027]"},{"line_number":293,"context_line":""},{"line_number":294,"context_line":"        if network_id:"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a5ae5dd_9838b2d4","line":291,"range":{"start_line":291,"start_character":12,"end_line":291,"end_character":41},"updated":"2016-02-08 19:55:20.000000000","message":"nit:  This seems like a special case that would be better handled by the caller before L252.","commit_id":"1ded953557e617c676ec7637d510f7098c0c566c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"61b0cd2672fba0d8571fdb6ab3c6bc0d753f650e","unresolved":false,"context_lines":[{"line_number":288,"context_line":"                self.l3_plugin.remove_router_interface("},{"line_number":289,"context_line":"                    context, router_id, {\u0027subnet_id\u0027: subnet[\u0027id\u0027]})"},{"line_number":290,"context_line":"            self.l3_plugin.delete_router(context, router_id)"},{"line_number":291,"context_line":"            if subnets and not network_id:"},{"line_number":292,"context_line":"                network_id \u003d subnets[0][\u0027network_id\u0027]"},{"line_number":293,"context_line":""},{"line_number":294,"context_line":"        if network_id:"}],"source_content_type":"text/x-python","patch_set":24,"id":"5a5ae5dd_ab70addd","line":291,"range":{"start_line":291,"start_character":12,"end_line":291,"end_character":41},"in_reply_to":"5a5ae5dd_9838b2d4","updated":"2016-02-08 23:17:11.000000000","message":"Done","commit_id":"1ded953557e617c676ec7637d510f7098c0c566c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ddda805debfe9140beef88ec0e197cedf39b2a72","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    return net_res"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"def _extend_external_network_callback("},{"line_number":53,"context_line":"    resource, event, trigger, context, request, network):"},{"line_number":54,"context_line":"    \"\"\"Ensure the is_default db field matches the create/update request.\"\"\""},{"line_number":55,"context_line":"    is_default \u003d request.get(IS_DEFAULT)"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a5ae5dd_c4295339","line":52,"updated":"2016-02-09 01:48:48.000000000","message":"this is going to be nitpicky, but can we name this more specifically? _set_external_network_default_value? The current name sounds like one of the dictionary extension functions. Not a blocker but it was just something that occured to me when I was figuring out what this did.","commit_id":"be7f12b6623a4fcf41a0e717655fee762aabacf4"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"29ff6366199d4d5cbb950fb962d508f925ecb546","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    return net_res"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"def _extend_external_network_callback("},{"line_number":53,"context_line":"    resource, event, trigger, context, request, network):"},{"line_number":54,"context_line":"    \"\"\"Ensure the is_default db field matches the create/update request.\"\"\""},{"line_number":55,"context_line":"    is_default \u003d request.get(IS_DEFAULT)"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a5ae5dd_2492a7f7","line":52,"in_reply_to":"5a5ae5dd_c4295339","updated":"2016-02-09 02:02:27.000000000","message":"Done","commit_id":"be7f12b6623a4fcf41a0e717655fee762aabacf4"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"ddda805debfe9140beef88ec0e197cedf39b2a72","unresolved":false,"context_lines":[{"line_number":132,"context_line":"            # issuing the request, therefore let\u0027s get it from the context"},{"line_number":133,"context_line":"            tenant_id \u003d context.tenant_id"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        if not uuidutils.is_uuid_like(tenant_id):"},{"line_number":136,"context_line":"            raise n_exc.BadRequest("},{"line_number":137,"context_line":"                resource\u003d\u0027tenant\u0027, msg\u003d_(\u0027not a valid tenant id\u0027))"},{"line_number":138,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"5a5ae5dd_641f3f0a","line":135,"updated":"2016-02-09 01:48:48.000000000","message":"I think this needs to go. We don\u0027t validate the format of tenant IDs anywhere else that I\u0027m aware of so technically Neutron could work with non-uuid tenant identifiers. Even if nobody does that, we should probably be consistent here.","commit_id":"be7f12b6623a4fcf41a0e717655fee762aabacf4"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"29ff6366199d4d5cbb950fb962d508f925ecb546","unresolved":false,"context_lines":[{"line_number":132,"context_line":"            # issuing the request, therefore let\u0027s get it from the context"},{"line_number":133,"context_line":"            tenant_id \u003d context.tenant_id"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        if not uuidutils.is_uuid_like(tenant_id):"},{"line_number":136,"context_line":"            raise n_exc.BadRequest("},{"line_number":137,"context_line":"                resource\u003d\u0027tenant\u0027, msg\u003d_(\u0027not a valid tenant id\u0027))"},{"line_number":138,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"5a5ae5dd_64889fc7","line":135,"in_reply_to":"5a5ae5dd_641f3f0a","updated":"2016-02-09 02:02:27.000000000","message":"Done","commit_id":"be7f12b6623a4fcf41a0e717655fee762aabacf4"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5055a740bc2ba1a69edd6424d9af689f00bca873","unresolved":false,"context_lines":[{"line_number":52,"context_line":"    resource, event, trigger, context, request, network):"},{"line_number":53,"context_line":"    \"\"\"Ensure the is_default db field matches the create/update request.\"\"\""},{"line_number":54,"context_line":"    is_default \u003d request.get(IS_DEFAULT)"},{"line_number":55,"context_line":"    if event in (events.BEFORE_CREATE, events.BEFORE_UPDATE) and is_default:"},{"line_number":56,"context_line":"        # ensure there is only one default external network at any given time"},{"line_number":57,"context_line":"        obj \u003d (context.session.query(external_net_db.ExternalNetwork)."},{"line_number":58,"context_line":"            filter_by(is_default\u003dTrue)).first()"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a5ae5dd_d3b7d7f3","line":55,"updated":"2016-02-09 03:55:25.000000000","message":"I thinks its preferable to do:\n\nif is_default and event in (events.BEFORE_CREATE, events.BEFORE_UPDATE):","commit_id":"955fa1c075520e3416e430f72f003d8ded0eeb8e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"e815e5ba88cc060c0ad7df27aa6fc965af1d11be","unresolved":false,"context_lines":[{"line_number":52,"context_line":"    resource, event, trigger, context, request, network):"},{"line_number":53,"context_line":"    \"\"\"Ensure the is_default db field matches the create/update request.\"\"\""},{"line_number":54,"context_line":"    is_default \u003d request.get(IS_DEFAULT)"},{"line_number":55,"context_line":"    if event in (events.BEFORE_CREATE, events.BEFORE_UPDATE) and is_default:"},{"line_number":56,"context_line":"        # ensure there is only one default external network at any given time"},{"line_number":57,"context_line":"        obj \u003d (context.session.query(external_net_db.ExternalNetwork)."},{"line_number":58,"context_line":"            filter_by(is_default\u003dTrue)).first()"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a5ae5dd_13b56ecf","line":55,"in_reply_to":"5a5ae5dd_d3b7d7f3","updated":"2016-02-09 04:14:44.000000000","message":"why? What if I like to read from right to left?","commit_id":"955fa1c075520e3416e430f72f003d8ded0eeb8e"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"7f3093a310a59612e76f5715bf9b830f2d3b07b9","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                \u0027admin_state_up\u0027: True,"},{"line_number":200,"context_line":"                \u0027tenant_id\u0027: tenant_id,"},{"line_number":201,"context_line":"                \u0027shared\u0027: False"},{"line_number":202,"context_line":"            }"},{"line_number":203,"context_line":"            network \u003d p_utils.create_network("},{"line_number":204,"context_line":"                self.core_plugin, context, {\u0027network\u0027: network_args})"},{"line_number":205,"context_line":"            subnets \u003d []"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a5ae5dd_7db57bce","line":202,"updated":"2016-02-09 12:38:39.000000000","message":"@Gary, the plugin utils method that is being called here will handle population of default values in the attribute map.\n\nhttps://github.com/openstack/neutron/blob/5b7fd5f0f1edb237a4f72baa34fe47ec59c5266f/neutron/plugins/common/utils.py#L118-L136","commit_id":"955fa1c075520e3416e430f72f003d8ded0eeb8e"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5055a740bc2ba1a69edd6424d9af689f00bca873","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                \u0027admin_state_up\u0027: True,"},{"line_number":200,"context_line":"                \u0027tenant_id\u0027: tenant_id,"},{"line_number":201,"context_line":"                \u0027shared\u0027: False"},{"line_number":202,"context_line":"            }"},{"line_number":203,"context_line":"            network \u003d p_utils.create_network("},{"line_number":204,"context_line":"                self.core_plugin, context, {\u0027network\u0027: network_args})"},{"line_number":205,"context_line":"            subnets \u003d []"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a5ae5dd_537d27b0","line":202,"updated":"2016-02-09 03:55:25.000000000","message":"i have a minor concern here and maybe am missing something (will not be the first nor the last time). if the network plugin has a number of extensions for the networks, how ill those values get their default settings? Could this lead to breakages?\nmaybe we should consider having a callback here that populates the default settings for the network object?","commit_id":"955fa1c075520e3416e430f72f003d8ded0eeb8e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"e815e5ba88cc060c0ad7df27aa6fc965af1d11be","unresolved":false,"context_lines":[{"line_number":199,"context_line":"                \u0027admin_state_up\u0027: True,"},{"line_number":200,"context_line":"                \u0027tenant_id\u0027: tenant_id,"},{"line_number":201,"context_line":"                \u0027shared\u0027: False"},{"line_number":202,"context_line":"            }"},{"line_number":203,"context_line":"            network \u003d p_utils.create_network("},{"line_number":204,"context_line":"                self.core_plugin, context, {\u0027network\u0027: network_args})"},{"line_number":205,"context_line":"            subnets \u003d []"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a5ae5dd_d3fcb674","line":202,"in_reply_to":"5a5ae5dd_537d27b0","updated":"2016-02-09 04:14:44.000000000","message":"These are the minimum set of args that *all* plugins must honor. If they do something bad, it\u0027s on them","commit_id":"955fa1c075520e3416e430f72f003d8ded0eeb8e"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"6779714759b9a3bb87fd9365e0e027f7835dbd96","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            network \u003d p_utils.create_network("},{"line_number":204,"context_line":"                self.core_plugin, context, {\u0027network\u0027: network_args})"},{"line_number":205,"context_line":"            subnets \u003d []"},{"line_number":206,"context_line":"            for ip_version in self._get_supported_versions(context):"},{"line_number":207,"context_line":"                subnet_args \u003d {"},{"line_number":208,"context_line":"                    \u0027name\u0027: \u0027auto_allocated_subnet_v%s\u0027 % ip_version,"},{"line_number":209,"context_line":"                    \u0027network_id\u0027: network[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a5ae5dd_53e68645","line":206,"updated":"2016-02-09 04:00:00.000000000","message":"Consider doing:\n\ndef _provision_tenant_private_network(self, context, tenant_id):\n    versions \u003d self._get_supported_versions(context):\n    then you can catch that specific exception\n    create network\n    create subnet\n        for ip_version in versions:\n...","commit_id":"955fa1c075520e3416e430f72f003d8ded0eeb8e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"e815e5ba88cc060c0ad7df27aa6fc965af1d11be","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            network \u003d p_utils.create_network("},{"line_number":204,"context_line":"                self.core_plugin, context, {\u0027network\u0027: network_args})"},{"line_number":205,"context_line":"            subnets \u003d []"},{"line_number":206,"context_line":"            for ip_version in self._get_supported_versions(context):"},{"line_number":207,"context_line":"                subnet_args \u003d {"},{"line_number":208,"context_line":"                    \u0027name\u0027: \u0027auto_allocated_subnet_v%s\u0027 % ip_version,"},{"line_number":209,"context_line":"                    \u0027network_id\u0027: network[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a5ae5dd_33642a48","line":206,"in_reply_to":"5a5ae5dd_53e68645","updated":"2016-02-09 04:14:44.000000000","message":"can do, but the gain is really marginal at this point.","commit_id":"955fa1c075520e3416e430f72f003d8ded0eeb8e"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5055a740bc2ba1a69edd6424d9af689f00bca873","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                    \u0027network_id\u0027: network[\u0027id\u0027],"},{"line_number":210,"context_line":"                    \u0027tenant_id\u0027: tenant_id,"},{"line_number":211,"context_line":"                    \u0027ip_version\u0027: ip_version,"},{"line_number":212,"context_line":"                }"},{"line_number":213,"context_line":"                subnets.append(p_utils.create_subnet("},{"line_number":214,"context_line":"                    self.core_plugin, context, {\u0027subnet\u0027: subnet_args}))"},{"line_number":215,"context_line":"            return subnets"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a5ae5dd_935abf4f","line":212,"updated":"2016-02-09 03:55:25.000000000","message":"same point here","commit_id":"955fa1c075520e3416e430f72f003d8ded0eeb8e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"e815e5ba88cc060c0ad7df27aa6fc965af1d11be","unresolved":false,"context_lines":[{"line_number":209,"context_line":"                    \u0027network_id\u0027: network[\u0027id\u0027],"},{"line_number":210,"context_line":"                    \u0027tenant_id\u0027: tenant_id,"},{"line_number":211,"context_line":"                    \u0027ip_version\u0027: ip_version,"},{"line_number":212,"context_line":"                }"},{"line_number":213,"context_line":"                subnets.append(p_utils.create_subnet("},{"line_number":214,"context_line":"                    self.core_plugin, context, {\u0027subnet\u0027: subnet_args}))"},{"line_number":215,"context_line":"            return subnets"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a5ae5dd_f338d23f","line":212,"in_reply_to":"5a5ae5dd_935abf4f","updated":"2016-02-09 04:14:44.000000000","message":"same as line 202","commit_id":"955fa1c075520e3416e430f72f003d8ded0eeb8e"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5055a740bc2ba1a69edd6424d9af689f00bca873","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                subnets.append(p_utils.create_subnet("},{"line_number":214,"context_line":"                    self.core_plugin, context, {\u0027subnet\u0027: subnet_args}))"},{"line_number":215,"context_line":"            return subnets"},{"line_number":216,"context_line":"        except (ValueError, n_exc.BadRequest, n_exc.NotFound):"},{"line_number":217,"context_line":"            LOG.error(_LE(\"Unable to auto allocate topology for tenant \""},{"line_number":218,"context_line":"                          \"%s due to missing requirements, e.g. default \""},{"line_number":219,"context_line":"                          \"or shared subnetpools\"), tenant_id)"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a5ae5dd_532be794","line":216,"updated":"2016-02-09 03:55:25.000000000","message":"this has been troubling me since my first review here. why not a try catch on each opertaion. combining the same block for the network and subnet creation is a bit complicated and requires setting variables, for example network \u003d None\nIt would also enable us to have better logging","commit_id":"955fa1c075520e3416e430f72f003d8ded0eeb8e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"e815e5ba88cc060c0ad7df27aa6fc965af1d11be","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                subnets.append(p_utils.create_subnet("},{"line_number":214,"context_line":"                    self.core_plugin, context, {\u0027subnet\u0027: subnet_args}))"},{"line_number":215,"context_line":"            return subnets"},{"line_number":216,"context_line":"        except (ValueError, n_exc.BadRequest, n_exc.NotFound):"},{"line_number":217,"context_line":"            LOG.error(_LE(\"Unable to auto allocate topology for tenant \""},{"line_number":218,"context_line":"                          \"%s due to missing requirements, e.g. default \""},{"line_number":219,"context_line":"                          \"or shared subnetpools\"), tenant_id)"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a5ae5dd_33350af7","line":216,"in_reply_to":"5a5ae5dd_532be794","updated":"2016-02-09 04:14:44.000000000","message":"As I explained on previous patchset, the same error handling is required (log and traceback - the traceback is your source of truth, as this is an admin-facing error. I could take your suggestion on line 202, but then I am gonna have to duplicate the log statements unless I bring in other conditionals.\n\nAll things considered what you suggested doesn\u0027t look that much cleaner; these errors are most likely driven by deployment error configurations, which you\u0027d expect to be ironed out before rolling this out to users.\n\nHence I chose this route, where the business logic is less \u0027noisy\u0027","commit_id":"955fa1c075520e3416e430f72f003d8ded0eeb8e"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"6779714759b9a3bb87fd9365e0e027f7835dbd96","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                subnets.append(p_utils.create_subnet("},{"line_number":214,"context_line":"                    self.core_plugin, context, {\u0027subnet\u0027: subnet_args}))"},{"line_number":215,"context_line":"            return subnets"},{"line_number":216,"context_line":"        except (ValueError, n_exc.BadRequest, n_exc.NotFound):"},{"line_number":217,"context_line":"            LOG.error(_LE(\"Unable to auto allocate topology for tenant \""},{"line_number":218,"context_line":"                          \"%s due to missing requirements, e.g. default \""},{"line_number":219,"context_line":"                          \"or shared subnetpools\"), tenant_id)"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a5ae5dd_93671eea","line":216,"in_reply_to":"5a5ae5dd_532be794","updated":"2016-02-09 04:00:00.000000000","message":"what raises ValueError?","commit_id":"955fa1c075520e3416e430f72f003d8ded0eeb8e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"e815e5ba88cc060c0ad7df27aa6fc965af1d11be","unresolved":false,"context_lines":[{"line_number":213,"context_line":"                subnets.append(p_utils.create_subnet("},{"line_number":214,"context_line":"                    self.core_plugin, context, {\u0027subnet\u0027: subnet_args}))"},{"line_number":215,"context_line":"            return subnets"},{"line_number":216,"context_line":"        except (ValueError, n_exc.BadRequest, n_exc.NotFound):"},{"line_number":217,"context_line":"            LOG.error(_LE(\"Unable to auto allocate topology for tenant \""},{"line_number":218,"context_line":"                          \"%s due to missing requirements, e.g. default \""},{"line_number":219,"context_line":"                          \"or shared subnetpools\"), tenant_id)"}],"source_content_type":"text/x-python","patch_set":26,"id":"5a5ae5dd_f30f7286","line":216,"in_reply_to":"5a5ae5dd_93671eea","updated":"2016-02-09 04:14:44.000000000","message":"The p_utils methods.","commit_id":"955fa1c075520e3416e430f72f003d8ded0eeb8e"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5055a740bc2ba1a69edd6424d9af689f00bca873","unresolved":false,"context_lines":[{"line_number":230,"context_line":"            l3.EXTERNAL_GW_INFO: default_external_network,"},{"line_number":231,"context_line":"            \u0027tenant_id\u0027: tenant_id,"},{"line_number":232,"context_line":"            \u0027admin_state_up\u0027: True"},{"line_number":233,"context_line":"        }"},{"line_number":234,"context_line":"        router \u003d None"},{"line_number":235,"context_line":"        try:"},{"line_number":236,"context_line":"            router \u003d self.l3_plugin.create_router("}],"source_content_type":"text/x-python","patch_set":26,"id":"5a5ae5dd_131c8f76","line":233,"updated":"2016-02-09 03:55:25.000000000","message":"same point here","commit_id":"955fa1c075520e3416e430f72f003d8ded0eeb8e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"e815e5ba88cc060c0ad7df27aa6fc965af1d11be","unresolved":false,"context_lines":[{"line_number":230,"context_line":"            l3.EXTERNAL_GW_INFO: default_external_network,"},{"line_number":231,"context_line":"            \u0027tenant_id\u0027: tenant_id,"},{"line_number":232,"context_line":"            \u0027admin_state_up\u0027: True"},{"line_number":233,"context_line":"        }"},{"line_number":234,"context_line":"        router \u003d None"},{"line_number":235,"context_line":"        try:"},{"line_number":236,"context_line":"            router \u003d self.l3_plugin.create_router("}],"source_content_type":"text/x-python","patch_set":26,"id":"5a5ae5dd_13300e07","line":233,"in_reply_to":"5a5ae5dd_131c8f76","updated":"2016-02-09 04:14:44.000000000","message":"same point as above.","commit_id":"955fa1c075520e3416e430f72f003d8ded0eeb8e"}],"neutron/services/auto_allocate/exceptions.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5941b4f3a5ad3925c6c057ab7558a2981a28e5c9","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from neutron.common import exceptions as n_exc"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class AutoAllocationFailure(n_exc.Conflict):"},{"line_number":22,"context_line":"    message \u003d _(\"Deployment error: %(reason)s.\")"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a5de9d1_ea58ed32","line":21,"updated":"2016-02-02 03:31:27.000000000","message":"why is this not just part of the neutron exceptions?","commit_id":"8c83520cdc9caa71a388605207f8f5c14cdc5bd4"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b1ee0c9355e22ac58a72b274d6279320d15d5a8d","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from neutron.common import exceptions as n_exc"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class AutoAllocationFailure(n_exc.Conflict):"},{"line_number":22,"context_line":"    message \u003d _(\"Deployment error: %(reason)s.\")"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a5de9d1_10efb71e","line":21,"in_reply_to":"7a5de9d1_ea58ed32","updated":"2016-02-02 06:58:14.000000000","message":"Because it is pertinent to this module not the whole thing","commit_id":"8c83520cdc9caa71a388605207f8f5c14cdc5bd4"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"b184d758cc004d28f10b410677c4cf11c11a61e8","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron.common import exceptions as n_exc"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from neutron._i18n import _"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"class AutoAllocationFailure(n_exc.Conflict):"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_523300f6","line":20,"updated":"2016-02-08 17:20:20.000000000","message":"this should be in the same section as line 18","commit_id":"66b9a7a84a0ac274c924824a6aa3cf7989408244"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"cbf06d046c917491d7f7fad3b0330fb533fbd8c0","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron.common import exceptions as n_exc"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from neutron._i18n import _"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"class AutoAllocationFailure(n_exc.Conflict):"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a5ae5dd_98eaf7d9","line":20,"in_reply_to":"5a5ae5dd_523300f6","updated":"2016-02-08 18:04:34.000000000","message":"what difference does it make?","commit_id":"66b9a7a84a0ac274c924824a6aa3cf7989408244"}],"neutron/services/auto_allocate/models.py":[{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"5e09854be9c9ee12770fa6004870aa67dc7c1ab3","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    router_id \u003d sa.Column(sa.String(36),"},{"line_number":32,"context_line":"                          sa.ForeignKey(\u0027routers.id\u0027,"},{"line_number":33,"context_line":"                                        ondelete\u003d\u0027CASCADE\u0027),"},{"line_number":34,"context_line":"                          nullable\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_939ca3c1","line":34,"updated":"2016-02-04 16:30:32.000000000","message":"Do we want to track the two subnets also, to facilitate cascading deletes?","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"0e76c601dc34728c529c535e96445ee362953629","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    router_id \u003d sa.Column(sa.String(36),"},{"line_number":32,"context_line":"                          sa.ForeignKey(\u0027routers.id\u0027,"},{"line_number":33,"context_line":"                                        ondelete\u003d\u0027CASCADE\u0027),"},{"line_number":34,"context_line":"                          nullable\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_95ab1475","line":34,"in_reply_to":"5a5ae5dd_4eaf5594","updated":"2016-02-05 01:14:48.000000000","message":"Actually this does *not* work. (My initial test was flawed.)\n\nNetworks do not have a FK relationship with subnets, so no network will be deleted when a subnet is deleted.\n\nSubnets have a FK relationship with networks, but network deletes are not cascaded to subnets:\n- https://github.com/openstack/neutron/blob/master/neutron/db/models_v2.py#L184","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"e7fed3636314cb4fa8d15b13862ea8b2f849afe1","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    router_id \u003d sa.Column(sa.String(36),"},{"line_number":32,"context_line":"                          sa.ForeignKey(\u0027routers.id\u0027,"},{"line_number":33,"context_line":"                                        ondelete\u003d\u0027CASCADE\u0027),"},{"line_number":34,"context_line":"                          nullable\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_4eaf5594","line":34,"in_reply_to":"5a5ae5dd_6286ea14","updated":"2016-02-04 20:05:05.000000000","message":"I should have realized that. I tested it and it works.","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"29d5a95a86aeb885bdf405341b1014bda378e58c","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    router_id \u003d sa.Column(sa.String(36),"},{"line_number":32,"context_line":"                          sa.ForeignKey(\u0027routers.id\u0027,"},{"line_number":33,"context_line":"                                        ondelete\u003d\u0027CASCADE\u0027),"},{"line_number":34,"context_line":"                          nullable\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_9fbb42e5","line":34,"in_reply_to":"5a5ae5dd_939ca3c1","updated":"2016-02-04 17:43:16.000000000","message":"no need I believe, the network does the work for us.","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"10372cae34bad7d66ec315a1c8f801058550ce60","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    router_id \u003d sa.Column(sa.String(36),"},{"line_number":32,"context_line":"                          sa.ForeignKey(\u0027routers.id\u0027,"},{"line_number":33,"context_line":"                                        ondelete\u003d\u0027CASCADE\u0027),"},{"line_number":34,"context_line":"                          nullable\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_b88e45ce","line":34,"in_reply_to":"5a5ae5dd_95ab1475","updated":"2016-02-05 02:01:11.000000000","message":"Henry, you\u0027re looking at this from the wrong angle. Whenever you delete a network, subnets will be deleted via business logic.","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"698d7184dca912ccac4d8fecc7fb414764408bf6","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    router_id \u003d sa.Column(sa.String(36),"},{"line_number":32,"context_line":"                          sa.ForeignKey(\u0027routers.id\u0027,"},{"line_number":33,"context_line":"                                        ondelete\u003d\u0027CASCADE\u0027),"},{"line_number":34,"context_line":"                          nullable\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_6286ea14","line":34,"in_reply_to":"5a5ae5dd_9fbb42e5","updated":"2016-02-04 18:33:47.000000000","message":"I think the delete will cascade through the network to the subnets.","commit_id":"c10cf6db11ff4f36e781007fa83ef5245686e08a"}],"neutron/services/auto_allocate/plugin.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"8ca3b43c0392609d8b00f1769732f210bea8e343","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from neutron.services.auto_allocate import db"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class Plugin(db.AutoAllocatedTopologyMixin):"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    _instance \u003d None"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9a68dd71_c4084694","line":21,"updated":"2016-01-27 00:51:14.000000000","message":"the plugin is off by default right now. This change is innocuous","commit_id":"0546ddcec502b97c6390db6aa3f6da6274168838"},{"author":{"_account_id":14605,"name":"vikram.choudhary","email":"vikschw@gmail.com","username":"vikram.choudhary"},"change_message_id":"97081157a2360256227b1ad3caa03e1e2311d746","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from neutron.services.auto_allocate import db"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class Plugin(db.AutoAllocatedTopologyMixin):"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    _instance \u003d None"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7a5de9d1_7a47e11d","line":21,"updated":"2016-02-02 10:20:21.000000000","message":"Can we make this name more clear? Plugin sound too generic.","commit_id":"8c83520cdc9caa71a388605207f8f5c14cdc5bd4"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b9b6b900a4abdf6e5f0988997fe0efc8ca1383ff","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from neutron.services.auto_allocate import db"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class Plugin(db.AutoAllocatedTopologyMixin):"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    _instance \u003d None"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7a5de9d1_654ac757","line":21,"in_reply_to":"7a5de9d1_6ea63155","updated":"2016-02-02 17:05:37.000000000","message":"indeed. It\u0027s in a namespace. I\u0027d rather keep the name small to be honest. The way we wrote this it\u0027s rare we\u0027ll have name clashing.","commit_id":"8c83520cdc9caa71a388605207f8f5c14cdc5bd4"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"812aea48f30eba76c004462545b17e94cab50c59","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from neutron.services.auto_allocate import db"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class Plugin(db.AutoAllocatedTopologyMixin):"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    _instance \u003d None"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7a5de9d1_6ea63155","line":21,"in_reply_to":"7a5de9d1_7a47e11d","updated":"2016-02-02 15:38:58.000000000","message":"It\u0027s in services/auto_allocate/plugin.py so I think that makes is clear enough?","commit_id":"8c83520cdc9caa71a388605207f8f5c14cdc5bd4"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"16051cb6a4e0442ad88b14a77676a95fb4f747cd","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    def get_plugin_type(self):"},{"line_number":37,"context_line":"        return \"auto-allocated-topology\""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def get_auto_allocated_topology(self, context, tenant_id, fields\u003dNone):"},{"line_number":40,"context_line":"        return super(Plugin, self).get_auto_allocated_topology("},{"line_number":41,"context_line":"            context, tenant_id, fields\u003dfields)"}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_8833ea13","line":39,"updated":"2016-02-05 09:28:05.000000000","message":"this doesn\u0027t do anything","commit_id":"03386460fb46fb9970c4c705931717865a499497"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7fe07be846e47dd4a4e3c3365f5dd4e3e53cccc3","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    def get_plugin_type(self):"},{"line_number":37,"context_line":"        return \"auto-allocated-topology\""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def get_auto_allocated_topology(self, context, tenant_id, fields\u003dNone):"},{"line_number":40,"context_line":"        return super(Plugin, self).get_auto_allocated_topology("},{"line_number":41,"context_line":"            context, tenant_id, fields\u003dfields)"}],"source_content_type":"text/x-python","patch_set":22,"id":"5a5ae5dd_1fa2bffd","line":39,"in_reply_to":"5a5ae5dd_8833ea13","updated":"2016-02-05 16:34:14.000000000","message":"it doesn\u0027t, it was mainly for documentation purposes and in case something plugin specific had to go in. I\u0027ll drop it.","commit_id":"03386460fb46fb9970c4c705931717865a499497"}],"neutron/tests/api/base.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5941b4f3a5ad3925c6c057ab7558a2981a28e5c9","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        cls.admin_address_scopes \u003d []"},{"line_number":109,"context_line":"        cls.subnetpools \u003d []"},{"line_number":110,"context_line":"        cls.admin_subnetpools \u003d []"},{"line_number":111,"context_line":"        cls.ant \u003d []"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    @classmethod"},{"line_number":114,"context_line":"    def resource_cleanup(cls):"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a5de9d1_4ab8212a","line":111,"updated":"2016-02-02 03:31:27.000000000","message":"do we need pest control here? can we use a name that is more user friendly","commit_id":"8c83520cdc9caa71a388605207f8f5c14cdc5bd4"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b1ee0c9355e22ac58a72b274d6279320d15d5a8d","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        cls.admin_address_scopes \u003d []"},{"line_number":109,"context_line":"        cls.subnetpools \u003d []"},{"line_number":110,"context_line":"        cls.admin_subnetpools \u003d []"},{"line_number":111,"context_line":"        cls.ant \u003d []"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    @classmethod"},{"line_number":114,"context_line":"    def resource_cleanup(cls):"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a5de9d1_70e06b29","line":111,"in_reply_to":"7a5de9d1_4ab8212a","updated":"2016-02-02 06:58:14.000000000","message":"disregard these changes, they will need to be redone.","commit_id":"8c83520cdc9caa71a388605207f8f5c14cdc5bd4"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"812aea48f30eba76c004462545b17e94cab50c59","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        cls.admin_address_scopes \u003d []"},{"line_number":109,"context_line":"        cls.subnetpools \u003d []"},{"line_number":110,"context_line":"        cls.admin_subnetpools \u003d []"},{"line_number":111,"context_line":"        cls.ant \u003d []"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    @classmethod"},{"line_number":114,"context_line":"    def resource_cleanup(cls):"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a5de9d1_3b871a14","line":111,"in_reply_to":"7a5de9d1_70e06b29","updated":"2016-02-02 15:38:58.000000000","message":"I\u0027ll get rid of the insects when updating the tests.","commit_id":"8c83520cdc9caa71a388605207f8f5c14cdc5bd4"}],"neutron/tests/api/test_allocate_net_topology.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"8ca3b43c0392609d8b00f1769732f210bea8e343","unresolved":false,"context_lines":[{"line_number":24,"context_line":"CONF \u003d config.CONF"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"class TestAllocateNetTopology(base.BaseNetworkTest):"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    ext_net_id \u003d str(uuid.uuid4())"},{"line_number":30,"context_line":"    subnet_cidr \u003d \u002710.11.12.0/31\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a68dd71_e4da2a17","line":27,"updated":"2016-01-27 00:51:14.000000000","message":"this needs to be revised","commit_id":"0546ddcec502b97c6390db6aa3f6da6274168838"}],"neutron/tests/tempest/services/network/json/network_client.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"8ca3b43c0392609d8b00f1769732f210bea8e343","unresolved":false,"context_lines":[{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    # Allocate Net Topology methods"},{"line_number":204,"context_line":"    def create_ant(self, post_data):"},{"line_number":205,"context_line":"        body \u003d self.serialize_list(post_data, \"allocated-net-topologies\")"},{"line_number":206,"context_line":"        uri \u003d self.get_uri(\"allocated-net-topologies\")"},{"line_number":207,"context_line":"        resp, body \u003d self.post(uri, body)"},{"line_number":208,"context_line":"        body \u003d {\u0027allocated-net-topologies\u0027: self.deserialize_list(body)}"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a68dd71_04d89e1c","line":205,"range":{"start_line":205,"start_character":35,"end_line":205,"end_character":44},"updated":"2016-01-27 00:51:14.000000000","message":"ditto","commit_id":"0546ddcec502b97c6390db6aa3f6da6274168838"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"5941b4f3a5ad3925c6c057ab7558a2981a28e5c9","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        return service_client.ResponseBody(resp, body)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    # Allocate Net Topology methods"},{"line_number":203,"context_line":"    def create_ant(self, post_data):"},{"line_number":204,"context_line":"        body \u003d self.serialize_list(post_data, \"allocated-net-topologies\")"},{"line_number":205,"context_line":"        uri \u003d self.get_uri(\"allocated-net-topologies\")"},{"line_number":206,"context_line":"        resp, body \u003d self.post(uri, body)"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a5de9d1_6a8abd65","line":203,"updated":"2016-02-02 03:31:27.000000000","message":"sorry but i do not like the ant. can we use a better name here if possible?","commit_id":"8c83520cdc9caa71a388605207f8f5c14cdc5bd4"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b1ee0c9355e22ac58a72b274d6279320d15d5a8d","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        return service_client.ResponseBody(resp, body)"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    # Allocate Net Topology methods"},{"line_number":203,"context_line":"    def create_ant(self, post_data):"},{"line_number":204,"context_line":"        body \u003d self.serialize_list(post_data, \"allocated-net-topologies\")"},{"line_number":205,"context_line":"        uri \u003d self.get_uri(\"allocated-net-topologies\")"},{"line_number":206,"context_line":"        resp, body \u003d self.post(uri, body)"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a5de9d1_90c2c783","line":203,"in_reply_to":"7a5de9d1_6a8abd65","updated":"2016-02-02 06:58:14.000000000","message":"don\u0027t worry, this needs cleaning.","commit_id":"8c83520cdc9caa71a388605207f8f5c14cdc5bd4"}]}