)]}'
{"neutron/services/trunk/exceptions.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"f20d3c7c4ee7bf3e4108273ea27e2e6dccbaeefb","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                \"eligible for use as a parent port.\")"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"class PortInUseAsTrunk(n_exc.InUse):"},{"line_number":45,"context_line":"    message \u003d _(\"Port %(port_id)s is currently in use as a Trunk.\")"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"dada55a8_467fde65","line":44,"updated":"2016-07-20 21:29:18.000000000","message":"I am not 100% convinced we need two separate exceptions, as I feel a message along the line \"port is in use by a trunk FOO should suffice\". Then the user can look up the trunk and see where the port stands. I don\u0027t feel strongly either way, but we do need to add the trunk UUID.","commit_id":"19ea6b916af7ec4ac5ae890eee0a751270eceac5"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"35cdc56955cc4322e2b894d568f4f46eaf16719a","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                \"eligible for use as a parent port.\")"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"class PortInUseAsTrunk(n_exc.InUse):"},{"line_number":45,"context_line":"    message \u003d _(\"Port %(port_id)s is currently in use as a Trunk.\")"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"dada55a8_ec7fd1a8","line":44,"in_reply_to":"dada55a8_467fde65","updated":"2016-07-20 21:47:49.000000000","message":"Sounds fair to me, one exception should be fine. Next PS will include the trunk UUID too.","commit_id":"19ea6b916af7ec4ac5ae890eee0a751270eceac5"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"603792b8769990bd897205b9ef40009f58526a30","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                \"eligible for use as a parent port.\")"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"class PortInUseByTrunk(n_exc.InUse):"},{"line_number":45,"context_line":"    message \u003d _(\"Port %(port_id)s is currently in use by trunk %(trunk_id)s.\")"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_6bea6bdc","line":44,"updated":"2016-07-21 21:00:22.000000000","message":"If I must nitpick I would say PortInUseByTrunkParent","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"4a42a1134f763c03b700b84e4dbb0589ba801e1a","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                \"eligible for use as a parent port.\")"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"class PortInUseByTrunk(n_exc.InUse):"},{"line_number":45,"context_line":"    message \u003d _(\"Port %(port_id)s is currently in use by trunk %(trunk_id)s.\")"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_9dc6ca47","line":44,"in_reply_to":"dada55a8_11ccc2b9","updated":"2016-07-22 21:17:28.000000000","message":"Done","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"9ab53e5dcdea6220e807d811ddfe3e681f7e8ba3","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                \"eligible for use as a parent port.\")"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"class PortInUseByTrunk(n_exc.InUse):"},{"line_number":45,"context_line":"    message \u003d _(\"Port %(port_id)s is currently in use by trunk %(trunk_id)s.\")"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_ae8f5173","line":44,"in_reply_to":"dada55a8_6bea6bdc","updated":"2016-07-21 21:21:08.000000000","message":"I would say *as* instead of *by*. To me that makes more sense: PortInUseAsTrunkParent. But do we need to be so specific when we have a different exception for the SubPort case?","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"bbddf212596e7fb7abf50fde282764942a0ef501","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                \"eligible for use as a parent port.\")"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"class PortInUseByTrunk(n_exc.InUse):"},{"line_number":45,"context_line":"    message \u003d _(\"Port %(port_id)s is currently in use by trunk %(trunk_id)s.\")"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_11ccc2b9","line":44,"in_reply_to":"dada55a8_ae8f5173","updated":"2016-07-21 21:26:52.000000000","message":"I am not a native speaker so I am not in the best position to say. I leave the choice to you.","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"603792b8769990bd897205b9ef40009f58526a30","unresolved":false,"context_lines":[{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"class PortInUseByTrunk(n_exc.InUse):"},{"line_number":45,"context_line":"    message \u003d _(\"Port %(port_id)s is currently in use by trunk %(trunk_id)s.\")"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"class PortInUseBySubPort(n_exc.InUse):"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_abf0e3c9","line":45,"updated":"2016-07-21 21:00:22.000000000","message":"Port %(port_id)s is currently a parent port for trunk %(trunk_id)s","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"4a42a1134f763c03b700b84e4dbb0589ba801e1a","unresolved":false,"context_lines":[{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"class PortInUseByTrunk(n_exc.InUse):"},{"line_number":45,"context_line":"    message \u003d _(\"Port %(port_id)s is currently in use by trunk %(trunk_id)s.\")"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"class PortInUseBySubPort(n_exc.InUse):"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_fdc38636","line":45,"in_reply_to":"dada55a8_abf0e3c9","updated":"2016-07-22 21:17:28.000000000","message":"Done","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"603792b8769990bd897205b9ef40009f58526a30","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"class PortInUseBySubPort(n_exc.InUse):"},{"line_number":49,"context_line":"    message \u003d _(\"Port %(port_id)s is currently in use by a subport in \""},{"line_number":50,"context_line":"                \"trunk %(trunk_id)s.\")"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_8beb5fdb","line":49,"updated":"2016-07-21 21:00:22.000000000","message":"Port %(port_id)s is currently a subport for trunk %(trunk_id)s","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"4a42a1134f763c03b700b84e4dbb0589ba801e1a","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"class PortInUseBySubPort(n_exc.InUse):"},{"line_number":49,"context_line":"    message \u003d _(\"Port %(port_id)s is currently in use by a subport in \""},{"line_number":50,"context_line":"                \"trunk %(trunk_id)s.\")"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_3ddcfe17","line":49,"in_reply_to":"dada55a8_8beb5fdb","updated":"2016-07-22 21:17:28.000000000","message":"Done","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"}],"neutron/services/trunk/rules.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"588812ae1d84205c8db58655d75bf01c055c5d00","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        raise trunk_exc.TrunkDisabled(trunk_id\u003dtrunk.id)"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"def enforce_port_deletion_rules(resource, event, trigger, **kwargs):"},{"line_number":37,"context_line":"    context \u003d kwargs[\u0027context\u0027]"},{"line_number":38,"context_line":"    port_id \u003d kwargs[\u0027port_id\u0027]"},{"line_number":39,"context_line":"    trunk \u003d trunk_objects.Trunk.get_object(context, port_id\u003dport_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"dada55a8_7dd0adf6","line":36,"updated":"2016-07-20 23:00:54.000000000","message":"docstring please.","commit_id":"19ea6b916af7ec4ac5ae890eee0a751270eceac5"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"f20d3c7c4ee7bf3e4108273ea27e2e6dccbaeefb","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    port_id \u003d kwargs[\u0027port_id\u0027]"},{"line_number":39,"context_line":"    trunk \u003d trunk_objects.Trunk.get_object(context, port_id\u003dport_id)"},{"line_number":40,"context_line":"    if trunk:"},{"line_number":41,"context_line":"        raise trunk_exc.PortInUseAsTrunk(port_id\u003dport_id)"},{"line_number":42,"context_line":"    subport \u003d trunk_objects.SubPort.get_object(context, port_id\u003dport_id)"},{"line_number":43,"context_line":"    if subport:"},{"line_number":44,"context_line":"        raise trunk_exc.PortInUseAsSubPort(port_id\u003dport_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"dada55a8_ac9839c1","line":41,"updated":"2016-07-20 21:29:18.000000000","message":"We gotta be careful here, before whoever is issuing a notify may not catch the exception and the bad handling may lead to a 500 error, and that\u0027s not cool. So far we had the notify in [1] but that may only work with trunk working with ML2. Did you look more closely into this?\n\n[1] https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/plugin.py#L1451","commit_id":"19ea6b916af7ec4ac5ae890eee0a751270eceac5"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"705d5798bf1311fa9461f0c08ca16722e3a2e50d","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    port_id \u003d kwargs[\u0027port_id\u0027]"},{"line_number":39,"context_line":"    trunk \u003d trunk_objects.Trunk.get_object(context, port_id\u003dport_id)"},{"line_number":40,"context_line":"    if trunk:"},{"line_number":41,"context_line":"        raise trunk_exc.PortInUseAsTrunk(port_id\u003dport_id)"},{"line_number":42,"context_line":"    subport \u003d trunk_objects.SubPort.get_object(context, port_id\u003dport_id)"},{"line_number":43,"context_line":"    if subport:"},{"line_number":44,"context_line":"        raise trunk_exc.PortInUseAsSubPort(port_id\u003dport_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"dada55a8_777bae28","line":41,"in_reply_to":"dada55a8_17019205","updated":"2016-07-20 22:00:25.000000000","message":"A few points raised.\n\na) the BEFORE_DELETE event is emitted only by ML2: I am ok for now to limit this to be ML2 specific, that said, we may want to add a note for us to keep track of the \u0027limitation\u0027. The notification can be moved higher up in the plugin hierarchy but this is something that can be done later on.\n\nb) Change the cascade policy to restrict: historically we never used the restrict cascade policy as it may be too \u0027restrictive\u0027 and/or lead to unexpected failure modes if an extraneous model is inserted in the alembic chain. So it\u0027s better to apply the constraint via a business rule.\n\nI wonder if note [1] has outlived its purpose (or perhaps should be moved). With this patch and what we done so far we effectively knocked 3 out 4 rules so far. Bence\u0027s patch properly tweaked should complete the list.\n\n[1] https://github.com/openstack/neutron/blob/master/neutron/services/trunk/models.py#L75","commit_id":"19ea6b916af7ec4ac5ae890eee0a751270eceac5"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"e67a9e915a965033d38c133e14dcdd5a91eb39c5","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    port_id \u003d kwargs[\u0027port_id\u0027]"},{"line_number":39,"context_line":"    trunk \u003d trunk_objects.Trunk.get_object(context, port_id\u003dport_id)"},{"line_number":40,"context_line":"    if trunk:"},{"line_number":41,"context_line":"        raise trunk_exc.PortInUseAsTrunk(port_id\u003dport_id)"},{"line_number":42,"context_line":"    subport \u003d trunk_objects.SubPort.get_object(context, port_id\u003dport_id)"},{"line_number":43,"context_line":"    if subport:"},{"line_number":44,"context_line":"        raise trunk_exc.PortInUseAsSubPort(port_id\u003dport_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"dada55a8_18653f1d","line":41,"in_reply_to":"dada55a8_52c838a8","updated":"2016-07-20 23:51:16.000000000","message":"This is solved by [1]. [1] + this patch should take care of all the business rules we have in place. \n\nAs for the point you make I\u0027d add a note for now and cross that bridge later. As for the note, we can leave it there.\n\n[1] https://review.openstack.org/#/c/345101/","commit_id":"19ea6b916af7ec4ac5ae890eee0a751270eceac5"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"c8bf8d4386f4e6d3593dfdbd1efe89026a23bd16","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    port_id \u003d kwargs[\u0027port_id\u0027]"},{"line_number":39,"context_line":"    trunk \u003d trunk_objects.Trunk.get_object(context, port_id\u003dport_id)"},{"line_number":40,"context_line":"    if trunk:"},{"line_number":41,"context_line":"        raise trunk_exc.PortInUseAsTrunk(port_id\u003dport_id)"},{"line_number":42,"context_line":"    subport \u003d trunk_objects.SubPort.get_object(context, port_id\u003dport_id)"},{"line_number":43,"context_line":"    if subport:"},{"line_number":44,"context_line":"        raise trunk_exc.PortInUseAsSubPort(port_id\u003dport_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"dada55a8_52c838a8","line":41,"in_reply_to":"dada55a8_777bae28","updated":"2016-07-20 22:24:22.000000000","message":"Ok, thank you for pointing out the history in point B.\n\nFor point A, by moving the notification higher up in the plugin hierarchy, we would effectively be making all plugins emit this notification, rather than just there in the ml2 plugin?\n\n\u003e I wonder if note [1] has outlived its purpose\n\nWhat was the original purpose of that note? Just a checklist to make sure we implement those business rules? It seems like useful information in relation to point B. However,\n\n\u003e \"Deletion of a trunk automatically deletes all of its subports\"\n\nseems like it would be handled by the SubPort\u0027s trunk_id FK constraint with ondelete\u003d\"CASCADE\". However, that\u0027s currently throwing an error in my devstack but not in Ryan\u0027s for some reason when deleting a trunk with a subport. I imagine it\u0027s also the same error Bence encountered. Issuing the delete directly in the DB cascades the subport deletes properly, just not when it\u0027s done via the REST API.","commit_id":"19ea6b916af7ec4ac5ae890eee0a751270eceac5"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"35cdc56955cc4322e2b894d568f4f46eaf16719a","unresolved":false,"context_lines":[{"line_number":38,"context_line":"    port_id \u003d kwargs[\u0027port_id\u0027]"},{"line_number":39,"context_line":"    trunk \u003d trunk_objects.Trunk.get_object(context, port_id\u003dport_id)"},{"line_number":40,"context_line":"    if trunk:"},{"line_number":41,"context_line":"        raise trunk_exc.PortInUseAsTrunk(port_id\u003dport_id)"},{"line_number":42,"context_line":"    subport \u003d trunk_objects.SubPort.get_object(context, port_id\u003dport_id)"},{"line_number":43,"context_line":"    if subport:"},{"line_number":44,"context_line":"        raise trunk_exc.PortInUseAsSubPort(port_id\u003dport_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"dada55a8_17019205","line":41,"in_reply_to":"dada55a8_ac9839c1","updated":"2016-07-20 21:47:49.000000000","message":"I didn\u0027t look further than the ML2 plugin. Do we need to run through every non-ML2 plugin before we can be assured that this exception will be caught by all of them?\n\nAnd if some plugin doesn\u0027t catch it, does this mean we can\u0027t use the callback registry approach?\n\nI\u0027m also wondering, should we be setting ondelete\u003d\u0027restrict\u0027 in the db model for the port_id FK constraint in the Trunk and SubPort tables? Rather than ondelete\u003d\u0027cascade\u0027?","commit_id":"19ea6b916af7ec4ac5ae890eee0a751270eceac5"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"aca3bc72de93b44e47a3ea4997db34bf2095e750","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    port_id \u003d kwargs[\u0027port_id\u0027]"},{"line_number":44,"context_line":"    subport_obj \u003d trunk_objects.SubPort.get_object(context, port_id\u003dport_id)"},{"line_number":45,"context_line":"    if subport_obj:"},{"line_number":46,"context_line":"        raise trunk_exc.PortInUseByTrunk(port_id\u003dport_id,"},{"line_number":47,"context_line":"                                         trunk_id\u003dsubport_obj.trunk_id)"},{"line_number":48,"context_line":"    trunk_obj \u003d trunk_objects.Trunk.get_object(context, port_id\u003dport_id)"},{"line_number":49,"context_line":"    if trunk_obj:"}],"source_content_type":"text/x-python","patch_set":2,"id":"dada55a8_a9ebd4da","line":46,"range":{"start_line":46,"start_character":24,"end_line":46,"end_character":40},"updated":"2016-07-21 18:12:55.000000000","message":"Just thinking as a user for a second, I wonder if this could be a little confusing. I would appreciate a more specific message telling me that the port is in use by a subport on trunk x. PortInUseByTrunk feels to vague, and perhaps misleading in that it could convey that the port is a parent port for a trunk. Then when I go to debug the issue, I find no trunks that use this port as their parent and I file a bug because I wasn\u0027t led to the subports on the trunk :)","commit_id":"e6d098f14b98804e6692412de8b4a641379ab44d"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"e12ef38df2d55461df05a63bf431a0ba05ddc97f","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    port_id \u003d kwargs[\u0027port_id\u0027]"},{"line_number":44,"context_line":"    subport_obj \u003d trunk_objects.SubPort.get_object(context, port_id\u003dport_id)"},{"line_number":45,"context_line":"    if subport_obj:"},{"line_number":46,"context_line":"        raise trunk_exc.PortInUseByTrunk(port_id\u003dport_id,"},{"line_number":47,"context_line":"                                         trunk_id\u003dsubport_obj.trunk_id)"},{"line_number":48,"context_line":"    trunk_obj \u003d trunk_objects.Trunk.get_object(context, port_id\u003dport_id)"},{"line_number":49,"context_line":"    if trunk_obj:"}],"source_content_type":"text/x-python","patch_set":2,"id":"dada55a8_0b50100a","line":46,"range":{"start_line":46,"start_character":24,"end_line":46,"end_character":40},"in_reply_to":"dada55a8_094dc099","updated":"2016-07-21 20:19:17.000000000","message":"Done","commit_id":"e6d098f14b98804e6692412de8b4a641379ab44d"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"bdc0e4b6b6f819ff81dfb582a518d22a8820323c","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    port_id \u003d kwargs[\u0027port_id\u0027]"},{"line_number":44,"context_line":"    subport_obj \u003d trunk_objects.SubPort.get_object(context, port_id\u003dport_id)"},{"line_number":45,"context_line":"    if subport_obj:"},{"line_number":46,"context_line":"        raise trunk_exc.PortInUseByTrunk(port_id\u003dport_id,"},{"line_number":47,"context_line":"                                         trunk_id\u003dsubport_obj.trunk_id)"},{"line_number":48,"context_line":"    trunk_obj \u003d trunk_objects.Trunk.get_object(context, port_id\u003dport_id)"},{"line_number":49,"context_line":"    if trunk_obj:"}],"source_content_type":"text/x-python","patch_set":2,"id":"dada55a8_094dc099","line":46,"range":{"start_line":46,"start_character":24,"end_line":46,"end_character":40},"in_reply_to":"dada55a8_a9ebd4da","updated":"2016-07-21 18:22:56.000000000","message":"Ok, let\u0027s think this more carefully: this exception tells you that port FOO is in use by a trunk BAR, at this point you determined that you can\u0027t delete port FOO. What do you do to rectify the situation?\n\nYou either stop because you don\u0027t want to touch your trunk, or you want to unlock the situation.\n\nNow, a user has two options: he/she deletes the whole trunk BAR and he is done (assumed the parent port is unbound), or he/she is gonna have to check whether FOO is a subport in which case he/she can issue a remove-subport FOO. If port FOO is a parent port, he/she would need to unbind the port first, and consequently clean up the trunk.\n\nBottom line, you\u0027re right, it might be worth capturing the nature of the port and avoid the user the extra look up. You convinced me!","commit_id":"e6d098f14b98804e6692412de8b4a641379ab44d"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"603792b8769990bd897205b9ef40009f58526a30","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    # up in the plugin hierarchy."},{"line_number":42,"context_line":"    context \u003d kwargs[\u0027context\u0027]"},{"line_number":43,"context_line":"    port_id \u003d kwargs[\u0027port_id\u0027]"},{"line_number":44,"context_line":"    subport_obj \u003d trunk_objects.SubPort.get_object(context, port_id\u003dport_id)"},{"line_number":45,"context_line":"    if subport_obj:"},{"line_number":46,"context_line":"        raise trunk_exc.PortInUseBySubPort(port_id\u003dport_id,"},{"line_number":47,"context_line":"                                         trunk_id\u003dsubport_obj.trunk_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_ab4d83c5","line":44,"updated":"2016-07-21 21:00:22.000000000","message":"I have been thinking of the potential effect of this callback and I am wondering if there\u0027s a more efficient way of doing this by exploiting the backref. After all we don\u0027t want to cripple the performance of PORT DELETE in busy systems. Have you tried perhaps looking up the port and then walk the relationship [1,2]? I wonder if that\u0027s faster.\n\n[1] https://github.com/openstack/neutron/blob/master/neutron/services/trunk/models.py#L39\n[2] https://github.com/openstack/neutron/blob/master/neutron/services/trunk/models.py#L54","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"48c47035f7c1932f45e59c85b70bbac4c12bd3e6","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    # up in the plugin hierarchy."},{"line_number":42,"context_line":"    context \u003d kwargs[\u0027context\u0027]"},{"line_number":43,"context_line":"    port_id \u003d kwargs[\u0027port_id\u0027]"},{"line_number":44,"context_line":"    subport_obj \u003d trunk_objects.SubPort.get_object(context, port_id\u003dport_id)"},{"line_number":45,"context_line":"    if subport_obj:"},{"line_number":46,"context_line":"        raise trunk_exc.PortInUseBySubPort(port_id\u003dport_id,"},{"line_number":47,"context_line":"                                         trunk_id\u003dsubport_obj.trunk_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_4e005534","line":44,"in_reply_to":"dada55a8_2e57e1ca","updated":"2016-07-21 21:19:45.000000000","message":"yes, and if we improved the callback notification we could potentially save the extra lookup altogether but that\u0027s for another day.\n\nBTW, thanks for the compliment?! :)","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"84b23cf72e0960eba03f851754ebd52a79a7fe92","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    # up in the plugin hierarchy."},{"line_number":42,"context_line":"    context \u003d kwargs[\u0027context\u0027]"},{"line_number":43,"context_line":"    port_id \u003d kwargs[\u0027port_id\u0027]"},{"line_number":44,"context_line":"    subport_obj \u003d trunk_objects.SubPort.get_object(context, port_id\u003dport_id)"},{"line_number":45,"context_line":"    if subport_obj:"},{"line_number":46,"context_line":"        raise trunk_exc.PortInUseBySubPort(port_id\u003dport_id,"},{"line_number":47,"context_line":"                                         trunk_id\u003dsubport_obj.trunk_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_ea2f483c","line":44,"in_reply_to":"dada55a8_2efd216e","updated":"2016-07-22 15:56:48.000000000","message":"To try this out we\u0027d need to come up with our query rather than relying on get_port on the core plugin, which does not return a DB object","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"70e1bdb1059ff7625fe9fdeb9515b13fcb67e0da","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    # up in the plugin hierarchy."},{"line_number":42,"context_line":"    context \u003d kwargs[\u0027context\u0027]"},{"line_number":43,"context_line":"    port_id \u003d kwargs[\u0027port_id\u0027]"},{"line_number":44,"context_line":"    subport_obj \u003d trunk_objects.SubPort.get_object(context, port_id\u003dport_id)"},{"line_number":45,"context_line":"    if subport_obj:"},{"line_number":46,"context_line":"        raise trunk_exc.PortInUseBySubPort(port_id\u003dport_id,"},{"line_number":47,"context_line":"                                         trunk_id\u003dsubport_obj.trunk_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_2e57e1ca","line":44,"in_reply_to":"dada55a8_ab4d83c5","updated":"2016-07-21 21:17:27.000000000","message":"@Armando are you thinking something similar to this https://github.com/openstack/neutron/blob/master/neutron/services/trunk/plugin.py#L35? I know that example has the model object passed in but I think it illustrates what you\u0027re suggesting, yes? I think it\u0027s an astute observation that it might perform better. This way we can be assured of only 1 query thrown against the ports table, as opposed to at least 2 in the worst case as written.","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"9ab53e5dcdea6220e807d811ddfe3e681f7e8ba3","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    # up in the plugin hierarchy."},{"line_number":42,"context_line":"    context \u003d kwargs[\u0027context\u0027]"},{"line_number":43,"context_line":"    port_id \u003d kwargs[\u0027port_id\u0027]"},{"line_number":44,"context_line":"    subport_obj \u003d trunk_objects.SubPort.get_object(context, port_id\u003dport_id)"},{"line_number":45,"context_line":"    if subport_obj:"},{"line_number":46,"context_line":"        raise trunk_exc.PortInUseBySubPort(port_id\u003dport_id,"},{"line_number":47,"context_line":"                                         trunk_id\u003dsubport_obj.trunk_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_2efd216e","line":44,"in_reply_to":"dada55a8_ab4d83c5","updated":"2016-07-21 21:21:08.000000000","message":"I didn\u0027t look into that approach, but I will try it out.","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"87bb272f7df3fa56e4dcdf2c27a906b93db920bd","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    # up in the plugin hierarchy."},{"line_number":42,"context_line":"    context \u003d kwargs[\u0027context\u0027]"},{"line_number":43,"context_line":"    port_id \u003d kwargs[\u0027port_id\u0027]"},{"line_number":44,"context_line":"    subport_obj \u003d trunk_objects.SubPort.get_object(context, port_id\u003dport_id)"},{"line_number":45,"context_line":"    if subport_obj:"},{"line_number":46,"context_line":"        raise trunk_exc.PortInUseBySubPort(port_id\u003dport_id,"},{"line_number":47,"context_line":"                                         trunk_id\u003dsubport_obj.trunk_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_a2ac4b36","line":44,"in_reply_to":"dada55a8_e705b514","updated":"2016-07-22 20:58:30.000000000","message":"I think the most likely reason is that it\u0027s cheaper to do two simpler queries than a big one with a gazillion joins. I suppose this can in turn be more efficient if the notifier gave us the object directly, but this can be a follow-on optimization.\n\nThanks for looking into this","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"30e03efa7d9f0cbd1964ae0a59eb055a5afdfeec","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    # up in the plugin hierarchy."},{"line_number":42,"context_line":"    context \u003d kwargs[\u0027context\u0027]"},{"line_number":43,"context_line":"    port_id \u003d kwargs[\u0027port_id\u0027]"},{"line_number":44,"context_line":"    subport_obj \u003d trunk_objects.SubPort.get_object(context, port_id\u003dport_id)"},{"line_number":45,"context_line":"    if subport_obj:"},{"line_number":46,"context_line":"        raise trunk_exc.PortInUseBySubPort(port_id\u003dport_id,"},{"line_number":47,"context_line":"                                         trunk_id\u003dsubport_obj.trunk_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_e705b514","line":44,"in_reply_to":"dada55a8_ea2f483c","updated":"2016-07-22 20:36:08.000000000","message":"I tried out the approach Armando has in [1]. I did some rudimentary benchmarking of the two approaches, and it turned out that the OVO approach actually executes in about 50-60% of the time as the approach in [1] (when executing both the subport and trunk checks).\n\nI\u0027m not sure why that is, but maybe the OVO approach is actually more optimal than walking the relationship [2, 3]. Can you guys try it out too?\n\n[1] https://review.openstack.org/#/c/346137/\n[2] https://github.com/openstack/neutron/blob/master/neutron/services/trunk/models.py#L39\n[3] https://github.com/openstack/neutron/blob/master/neutron/services/trunk/models.py#L54","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"}],"neutron/tests/tempest/api/test_trunk_negative.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"603792b8769990bd897205b9ef40009f58526a30","unresolved":false,"context_lines":[{"line_number":220,"context_line":"    @test.attr(type\u003d\u0027negative\u0027)"},{"line_number":221,"context_line":"    @test.idempotent_id(\u00276c9c5126-4f61-11e6-8248-40a8f063c891\u0027)"},{"line_number":222,"context_line":"    def test_delete_port_in_use_by_trunk(self):"},{"line_number":223,"context_line":"        trunk \u003d self._create_trunk_with_network_and_parent(None)"},{"line_number":224,"context_line":"        self.assertRaises(lib_exc.Conflict, self.client.delete_port,"},{"line_number":225,"context_line":"                          trunk[\u0027trunk\u0027][\u0027port_id\u0027])"},{"line_number":226,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_2bd073c8","line":223,"updated":"2016-07-21 21:00:22.000000000","message":"I think None can be omitted","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"48c47035f7c1932f45e59c85b70bbac4c12bd3e6","unresolved":false,"context_lines":[{"line_number":220,"context_line":"    @test.attr(type\u003d\u0027negative\u0027)"},{"line_number":221,"context_line":"    @test.idempotent_id(\u00276c9c5126-4f61-11e6-8248-40a8f063c891\u0027)"},{"line_number":222,"context_line":"    def test_delete_port_in_use_by_trunk(self):"},{"line_number":223,"context_line":"        trunk \u003d self._create_trunk_with_network_and_parent(None)"},{"line_number":224,"context_line":"        self.assertRaises(lib_exc.Conflict, self.client.delete_port,"},{"line_number":225,"context_line":"                          trunk[\u0027trunk\u0027][\u0027port_id\u0027])"},{"line_number":226,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_ce9a8513","line":223,"in_reply_to":"dada55a8_0eb2dde4","updated":"2016-07-21 21:19:45.000000000","message":"ack, I was lazy and didn\u0027t want to check :P","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"70e1bdb1059ff7625fe9fdeb9515b13fcb67e0da","unresolved":false,"context_lines":[{"line_number":220,"context_line":"    @test.attr(type\u003d\u0027negative\u0027)"},{"line_number":221,"context_line":"    @test.idempotent_id(\u00276c9c5126-4f61-11e6-8248-40a8f063c891\u0027)"},{"line_number":222,"context_line":"    def test_delete_port_in_use_by_trunk(self):"},{"line_number":223,"context_line":"        trunk \u003d self._create_trunk_with_network_and_parent(None)"},{"line_number":224,"context_line":"        self.assertRaises(lib_exc.Conflict, self.client.delete_port,"},{"line_number":225,"context_line":"                          trunk[\u0027trunk\u0027][\u0027port_id\u0027])"},{"line_number":226,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_0eb2dde4","line":223,"in_reply_to":"dada55a8_2bd073c8","updated":"2016-07-21 21:17:27.000000000","message":"Passing None and [] are different cases, but something needs to be supplied to the method. In this case None is probably appropriate as we aren\u0027t passing any subports. [] would pass an empty list as opposed to None which omits a sub_ports field on the request altogether.","commit_id":"7c460f3970f9f9f183ade1ec714cc23a36e1c171"}],"neutron/tests/unit/services/trunk/test_plugin.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d1f51a7e270351c406af9b5e0f2f23e9f6310cc4","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        with self.port() as parent_port, self.port() as child_port:"},{"line_number":63,"context_line":"            subport \u003d create_subport_dict(child_port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":64,"context_line":"            self._create_test_trunk(parent_port, [subport])"},{"line_number":65,"context_line":"            core_plugin \u003d manager.NeutronManager.get_plugin()"},{"line_number":66,"context_line":"            port_id \u003d (parent_port[\u0027port\u0027][\u0027id\u0027]"},{"line_number":67,"context_line":"                       if trunk else child_port[\u0027port\u0027][\u0027id\u0027])"},{"line_number":68,"context_line":"            self.assertRaises(trunk_exc.PortInUseByTrunk,"}],"source_content_type":"text/x-python","patch_set":2,"id":"dada55a8_d21a36c5","line":65,"updated":"2016-07-21 17:38:35.000000000","message":"I would have personally did this differently, with one util methods that returns the trunk stuff and the other that did the assert but accepted the port_id as input rather than a bool; it seems more readable to me, but coverage is the same.","commit_id":"e6d098f14b98804e6692412de8b4a641379ab44d"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d1f51a7e270351c406af9b5e0f2f23e9f6310cc4","unresolved":false,"context_lines":[{"line_number":68,"context_line":"            self.assertRaises(trunk_exc.PortInUseByTrunk,"},{"line_number":69,"context_line":"                              core_plugin.delete_port, self.context, port_id)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def test_delete_port_raise_in_use_by_trunk(self):"},{"line_number":72,"context_line":"        self._test_delete_port_raise_in_use(trunk\u003dTrue)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    def test_delete_port_raise_in_use_by_subport(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"dada55a8_72d0aac9","line":71,"updated":"2016-07-21 17:38:35.000000000","message":"would be nice to add an API test too, this way other deployment options with CI using trunk can catch the error, and thus spot the NOTE, etc...","commit_id":"e6d098f14b98804e6692412de8b4a641379ab44d"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"e12ef38df2d55461df05a63bf431a0ba05ddc97f","unresolved":false,"context_lines":[{"line_number":68,"context_line":"            self.assertRaises(trunk_exc.PortInUseByTrunk,"},{"line_number":69,"context_line":"                              core_plugin.delete_port, self.context, port_id)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def test_delete_port_raise_in_use_by_trunk(self):"},{"line_number":72,"context_line":"        self._test_delete_port_raise_in_use(trunk\u003dTrue)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    def test_delete_port_raise_in_use_by_subport(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"dada55a8_8b44004a","line":71,"in_reply_to":"dada55a8_72d0aac9","updated":"2016-07-21 20:19:17.000000000","message":"Done","commit_id":"e6d098f14b98804e6692412de8b4a641379ab44d"}]}