)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"481b72fe2b1ef2f27a01fbb22122198b72ad3665","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2018-09-27 21:43:35 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Enable deleting bound trunk for agent which allows creating trunk on bound port"},{"line_number":8,"context_line":"For agent suchh as linux bridge which allows creating trunk on bound port, this patch provides a fix to allow deleting trunk without unbound port first.And it will help to keep the port (trunk\u0027s parent port) working while deleting the trunk."},{"line_number":9,"context_line":"Closes-Bug: #1794424"},{"line_number":10,"context_line":"Story: 2003889"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Iae2ae535bf3ba1548136bf3fe4306a42bad4e635"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_5faa2a84","line":9,"range":{"start_line":8,"start_character":0,"end_line":9,"end_character":2},"updated":"2018-09-27 06:23:03.000000000","message":"Please start new line for the commit message, follow the example here:\nhttps://wiki.openstack.org/wiki/GitCommitMessages","commit_id":"b89a16166d5264d78c7e244bdb8950d24f3a1fab"},{"author":{"_account_id":29093,"name":"Le, Huifeng","email":"huifeng.le@intel.com","username":"hle"},"change_message_id":"1a377388bd51b89d403cdf3e33995e2b886595c3","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2018-09-27 21:43:35 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Enable deleting bound trunk for agent which allows creating trunk on bound port"},{"line_number":8,"context_line":"For agent suchh as linux bridge which allows creating trunk on bound port, this patch provides a fix to allow deleting trunk without unbound port first.And it will help to keep the port (trunk\u0027s parent port) working while deleting the trunk."},{"line_number":9,"context_line":"Closes-Bug: #1794424"},{"line_number":10,"context_line":"Story: 2003889"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Iae2ae535bf3ba1548136bf3fe4306a42bad4e635"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3f79a3b5_9aa0d0a6","line":9,"range":{"start_line":8,"start_character":0,"end_line":9,"end_character":2},"in_reply_to":"3f79a3b5_5faa2a84","updated":"2018-09-27 07:21:39.000000000","message":"Thanks much for the comments! updated the commit message in the new patch.","commit_id":"b89a16166d5264d78c7e244bdb8950d24f3a1fab"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"bee7e6c9129af18f7c8cd47ddc04549555e9d56f","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Huifeng Le \u003chuifeng.le@intel.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2018-09-27 23:25:08 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Enable deleting bound trunk for agent which allows creating trunk on bound port"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"For agent such as linux bridge which allows creating trunk on bound port, this"},{"line_number":10,"context_line":"patch provides a fix to allow deleting trunk without unbound port first. And it"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_75637e1a","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":79},"updated":"2018-09-27 13:39:51.000000000","message":"This is a bit long:\n\"\"\"\nThe first line should be limited to 50 characters and should not end with a period.\nSubsequent lines should be wrapped at 72 characters.\n\"\"\"","commit_id":"2702e7177e0d17eff5d3eeccbd56d6e9399696bf"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a6c10e170ceed1c151d18288b7bb21a4d021e1f4","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Enable delete bound trunk for linux bridge agent"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"For agent such as linux bridge which allows creating trunk on"},{"line_number":10,"context_line":"bound port, this patch provides a fix to allow deleting trunk"},{"line_number":11,"context_line":"without unbound port first. And it will help to keep the port"},{"line_number":12,"context_line":"(trunk\u0027s parent port) working while deleting the trunk."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_b08fb674","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":10},"updated":"2018-11-06 18:05:51.000000000","message":"Actually this is the bug: to be able to create a trunk with a parent port already connected. As an example: in OVS [1] (similar in LB) the parent port and the subports are not connected directly to br-int, but a trunk bridge is created.\n\nIf the parent port is attached to br-int, there is nothing in the trunk manager to detach this trunk port and then create the trunk bridge and reattach this port to this bridge (aside from the possible connectivity lost).\n\n[1] https://wiki.openstack.org/wiki/File:TrunkOvsModel.svg","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"f6226b64e44a1ec1e2305b2eef852fa9b3d2905c","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Enable delete bound trunk for linux bridge agent"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"For agent such as linux bridge which allows creating trunk on"},{"line_number":10,"context_line":"bound port, this patch provides a fix to allow deleting trunk"},{"line_number":11,"context_line":"without unbound port first. And it will help to keep the port"},{"line_number":12,"context_line":"(trunk\u0027s parent port) working while deleting the trunk."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_de4fcebc","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":10},"in_reply_to":"3f79a3b5_11a6a20e","updated":"2018-11-07 15:53:32.000000000","message":"Reviewing the code for service trunk and LB, Hongbin is right: LB accepts to create a trunk using a parent port bound. OVS is not working the same but the service will take care of this. \n\ncan_trunk_bound_port is True in LB and False in OVS.\n\nI remove my -1.\n\nWaiting for the tempest tests!","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"b510f79f1abb4b9d0f2aba79a3fdb97e72402e04","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Enable delete bound trunk for linux bridge agent"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"For agent such as linux bridge which allows creating trunk on"},{"line_number":10,"context_line":"bound port, this patch provides a fix to allow deleting trunk"},{"line_number":11,"context_line":"without unbound port first. And it will help to keep the port"},{"line_number":12,"context_line":"(trunk\u0027s parent port) working while deleting the trunk."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_d657c726","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":10},"in_reply_to":"3f79a3b5_538ae919","updated":"2018-11-06 22:17:59.000000000","message":"I think this commit message could be a little more crisp. The idea is enable support for \"untrunking\" in drivers that support it. It\u0027s not necessarily specific to linux bridge, although that is what is motivating the change.","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":27654,"name":"Hongbin Lu","email":"kira034@163.com","username":"hongbin.lu"},"change_message_id":"256c7b9f9bacca2fac6590d9baf8712116ab9540","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Enable delete bound trunk for linux bridge agent"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"For agent such as linux bridge which allows creating trunk on"},{"line_number":10,"context_line":"bound port, this patch provides a fix to allow deleting trunk"},{"line_number":11,"context_line":"without unbound port first. And it will help to keep the port"},{"line_number":12,"context_line":"(trunk\u0027s parent port) working while deleting the trunk."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_538ae919","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":10},"in_reply_to":"3f79a3b5_b08fb674","updated":"2018-11-06 21:10:04.000000000","message":"@Rodolfo,\n\nI am not an expert of LB but I was looking at: https://github.com/openstack/neutron/commit/42f2ba88a58c04c0c119ec434acef2efe6589f06 . The commit message seems to suggest that this is not a bug. It is true that neutron restrict OVS in this case but it is intentional to relax this restriction for LB.\n\nAlthough I was having the same doubt on this (whether this scenario will work for LB) and requested @Huifeng to write a tempest test: https://review.openstack.org/#/c/615792/ . The test result does seem to confirm that everything will still work.","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":29093,"name":"Le, Huifeng","email":"huifeng.le@intel.com","username":"hle"},"change_message_id":"4e9ee806d74c5a9e67b7695d8789a4ddc8eab986","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Enable delete bound trunk for linux bridge agent"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"For agent such as linux bridge which allows creating trunk on"},{"line_number":10,"context_line":"bound port, this patch provides a fix to allow deleting trunk"},{"line_number":11,"context_line":"without unbound port first. And it will help to keep the port"},{"line_number":12,"context_line":"(trunk\u0027s parent port) working while deleting the trunk."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_11a6a20e","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":10},"in_reply_to":"3f79a3b5_d657c726","updated":"2018-11-07 08:08:43.000000000","message":"Maybe change the title to \"Bound trunk deletion support\"?","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":29093,"name":"Le, Huifeng","email":"huifeng.le@intel.com","username":"hle"},"change_message_id":"684525183f025f77c2f7cc1e93ee28239b249bb9","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Enable delete bound trunk for linux bridge agent"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"For agent such as linux bridge which allows creating trunk on"},{"line_number":10,"context_line":"bound port, this patch provides a fix to allow deleting trunk"},{"line_number":11,"context_line":"without unbound port first. And it will help to keep the port"},{"line_number":12,"context_line":"(trunk\u0027s parent port) working while deleting the trunk."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_5aff0efe","line":10,"range":{"start_line":9,"start_character":0,"end_line":10,"end_character":10},"in_reply_to":"3f79a3b5_de4fcebc","updated":"2018-11-28 02:10:41.000000000","message":"@Rodolfo, Thanks much for the comments, the tempest is submitted at https://review.openstack.org/#/c/615792/, please help to review, thanks!","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":27654,"name":"Hongbin Lu","email":"kira034@163.com","username":"hongbin.lu"},"change_message_id":"87460f88c11321394a7933228e1681760ee1cbf3","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Co-Authored-By: Allain Legacy \u003cAllain.legacy@windriver.com\u003e"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: #1794424"},{"line_number":17,"context_line":"Story: 2003889"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: Iae2ae535bf3ba1548136bf3fe4306a42bad4e635"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_0629e39a","line":16,"range":{"start_line":16,"start_character":13,"end_line":16,"end_character":20},"updated":"2018-10-19 21:32:08.000000000","message":"Mind adding a scenario test in here [1] to verify the scenario described in this bug. Ideally, there is a patch proposed for neutron-tempest-plugin for adding the test and that patch should depend on this patch to test this scenario end-to-end.\n\n[1] https://github.com/openstack/neutron-tempest-plugin/blob/master/neutron_tempest_plugin/scenario/test_trunk.py","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":27654,"name":"Hongbin Lu","email":"kira034@163.com","username":"hongbin.lu"},"change_message_id":"f61a9984a0c85d229ccc26dfa9d30c9c32010b55","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Co-Authored-By: Allain Legacy \u003cAllain.legacy@windriver.com\u003e"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: #1794424"},{"line_number":17,"context_line":"Story: 2003889"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: Iae2ae535bf3ba1548136bf3fe4306a42bad4e635"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_9c1f8259","line":16,"range":{"start_line":16,"start_character":13,"end_line":16,"end_character":20},"in_reply_to":"3f79a3b5_0629e39a","updated":"2018-10-19 22:36:37.000000000","message":"And most importantly, the bug said the port\u0027s connectivity should be kept while the trunk is deleted. If so, it should be verified in the scenario test as well.","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":29093,"name":"Le, Huifeng","email":"huifeng.le@intel.com","username":"hle"},"change_message_id":"a2151a4068ab3757a4e926e24ab8a423646ed4db","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Co-Authored-By: Allain Legacy \u003cAllain.legacy@windriver.com\u003e"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: #1794424"},{"line_number":17,"context_line":"Story: 2003889"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: Iae2ae535bf3ba1548136bf3fe4306a42bad4e635"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_18c6104c","line":16,"range":{"start_line":16,"start_character":13,"end_line":16,"end_character":20},"in_reply_to":"3f79a3b5_0629e39a","updated":"2018-10-22 06:52:20.000000000","message":"Thanks much for the comments! Will try to add a scenario test case in neutron-tempest-plugin project with a new patch based on the proposed patch (has it submitted for review or merged already?).","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":27654,"name":"Hongbin Lu","email":"kira034@163.com","username":"hongbin.lu"},"change_message_id":"ac8610470e16baab140a1b077b97d59a179a5c2b","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Co-Authored-By: Allain Legacy \u003cAllain.legacy@windriver.com\u003e"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: #1794424"},{"line_number":17,"context_line":"Story: 2003889"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: Iae2ae535bf3ba1548136bf3fe4306a42bad4e635"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_2ed278ff","line":16,"range":{"start_line":16,"start_character":13,"end_line":16,"end_character":20},"in_reply_to":"3f79a3b5_18c6104c","updated":"2018-10-23 21:52:10.000000000","message":"So, you can submit a patch for neutron-tempest-plugin with the scenario test, then add \"Depends-On: https://review.openstack.org/#/c/605589/\" in the commit message. This will kick the CI to run the test with the change in this patch. I would interest to see the test result in order to vote on this patch.\n\nBut, to be clear, the tempest test is not mandatory. Perhaps other reviewers are more happy to merge this patch without the test.","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"fcc25845d94947acf60fa2849eb732a44ea54062","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Co-Authored-By: Allain Legacy \u003cAllain.legacy@windriver.com\u003e"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: #1794424"},{"line_number":17,"context_line":"Story: 2003889"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: Iae2ae535bf3ba1548136bf3fe4306a42bad4e635"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_f67f6edb","line":16,"range":{"start_line":16,"start_character":13,"end_line":16,"end_character":20},"in_reply_to":"3f79a3b5_2ed278ff","updated":"2018-10-24 07:07:22.000000000","message":"I would like to see test for that case to see proof that it works actually :)","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":29093,"name":"Le, Huifeng","email":"huifeng.le@intel.com","username":"hle"},"change_message_id":"ccd80d55cc013429b1df7c9108e5a8edaa249d33","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Co-Authored-By: Allain Legacy \u003cAllain.legacy@windriver.com\u003e"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: #1794424"},{"line_number":17,"context_line":"Story: 2003889"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: Iae2ae535bf3ba1548136bf3fe4306a42bad4e635"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_0e8a1c19","line":16,"range":{"start_line":16,"start_character":13,"end_line":16,"end_character":20},"in_reply_to":"3f79a3b5_f67f6edb","updated":"2018-11-06 09:22:02.000000000","message":"Scenario test case had been added in neutron-tempest-plugin project (https://review.openstack.org/#/c/615792/1), please help to review, thanks!","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"}],"neutron/services/trunk/plugin.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"6238a8051e63ef34afb6e3dbd3de89c28657c014","unresolved":false,"context_lines":[{"line_number":264,"context_line":"            trunk \u003d self._get_trunk(context, trunk_id)"},{"line_number":265,"context_line":"            rules.trunk_can_be_managed(context, trunk)"},{"line_number":266,"context_line":"            trunk_port_validator \u003d rules.TrunkPortValidator(trunk.port_id)"},{"line_number":267,"context_line":"            if not trunk_port_validator.is_bound(context):"},{"line_number":268,"context_line":"                # NOTE(status_police): when a trunk is deleted, the logical"},{"line_number":269,"context_line":"                # object disappears from the datastore, therefore there is no"},{"line_number":270,"context_line":"                # status transition involved. If PRECOMMIT failures occur,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_188b9e69","side":"PARENT","line":267,"updated":"2018-10-18 16:10:03.000000000","message":"Does this change the API?  \"Deletes a trunk, if its state allows it.\" is all the API doc says.","commit_id":"56bcb6fe0aa545e5c6d4e208e9a0321929a353a4"},{"author":{"_account_id":29093,"name":"Le, Huifeng","email":"huifeng.le@intel.com","username":"hle"},"change_message_id":"4b2760fde83f0d42031004b71f8404ba8084a1ed","unresolved":false,"context_lines":[{"line_number":264,"context_line":"            trunk \u003d self._get_trunk(context, trunk_id)"},{"line_number":265,"context_line":"            rules.trunk_can_be_managed(context, trunk)"},{"line_number":266,"context_line":"            trunk_port_validator \u003d rules.TrunkPortValidator(trunk.port_id)"},{"line_number":267,"context_line":"            if not trunk_port_validator.is_bound(context):"},{"line_number":268,"context_line":"                # NOTE(status_police): when a trunk is deleted, the logical"},{"line_number":269,"context_line":"                # object disappears from the datastore, therefore there is no"},{"line_number":270,"context_line":"                # status transition involved. If PRECOMMIT failures occur,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_f9981d1b","side":"PARENT","line":267,"in_reply_to":"3f79a3b5_188b9e69","updated":"2018-10-19 02:25:21.000000000","message":"Thanks much for the comments! In my mind, if treated the state as \"whether port is bound\" (runtime port state) + \"whether the used agent supports creating/deleting trunk for bound trunk\" (static agent attribute), then the API is not changed. Does you think it is make sense?","commit_id":"56bcb6fe0aa545e5c6d4e208e9a0321929a353a4"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"b510f79f1abb4b9d0f2aba79a3fdb97e72402e04","unresolved":false,"context_lines":[{"line_number":264,"context_line":"            trunk \u003d self._get_trunk(context, trunk_id)"},{"line_number":265,"context_line":"            rules.trunk_can_be_managed(context, trunk)"},{"line_number":266,"context_line":"            trunk_port_validator \u003d rules.TrunkPortValidator(trunk.port_id)"},{"line_number":267,"context_line":"            if trunk_port_validator.can_be_trunked(context):"},{"line_number":268,"context_line":"                # NOTE(status_police): when a trunk is deleted, the logical"},{"line_number":269,"context_line":"                # object disappears from the datastore, therefore there is no"},{"line_number":270,"context_line":"                # status transition involved. If PRECOMMIT failures occur,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_7677d3d3","line":267,"updated":"2018-11-06 22:17:59.000000000","message":"I wonder if it would be more clear to add a method to the validator that checks if the trunk port can be \"untrunked\" rather than piggy-backing on can_be_trunked(). We may find things specific to \"untrunking\" that need to be validated, having the separation of checks might be a nice touch.","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":29093,"name":"Le, Huifeng","email":"huifeng.le@intel.com","username":"hle"},"change_message_id":"684525183f025f77c2f7cc1e93ee28239b249bb9","unresolved":false,"context_lines":[{"line_number":264,"context_line":"            trunk \u003d self._get_trunk(context, trunk_id)"},{"line_number":265,"context_line":"            rules.trunk_can_be_managed(context, trunk)"},{"line_number":266,"context_line":"            trunk_port_validator \u003d rules.TrunkPortValidator(trunk.port_id)"},{"line_number":267,"context_line":"            if trunk_port_validator.can_be_trunked(context):"},{"line_number":268,"context_line":"                # NOTE(status_police): when a trunk is deleted, the logical"},{"line_number":269,"context_line":"                # object disappears from the datastore, therefore there is no"},{"line_number":270,"context_line":"                # status transition involved. If PRECOMMIT failures occur,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_f7f87319","line":267,"in_reply_to":"3f79a3b5_08f67093","updated":"2018-11-28 02:10:41.000000000","message":"@Ryan, Thanks much for the comments! Yes, renaming the method looks to be more clear. but not sure whether it will result regression issue since it will be called by many other methods (including unit tests) internally. Any suggestion? Thanks!","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":4187,"name":"Ryan Tidwell","email":"rtidwell@suse.com","username":"ryan-tidwell"},"change_message_id":"71f1b2e21a21d7ff20f364b505edb5fd39a91b6a","unresolved":false,"context_lines":[{"line_number":264,"context_line":"            trunk \u003d self._get_trunk(context, trunk_id)"},{"line_number":265,"context_line":"            rules.trunk_can_be_managed(context, trunk)"},{"line_number":266,"context_line":"            trunk_port_validator \u003d rules.TrunkPortValidator(trunk.port_id)"},{"line_number":267,"context_line":"            if trunk_port_validator.can_be_trunked(context):"},{"line_number":268,"context_line":"                # NOTE(status_police): when a trunk is deleted, the logical"},{"line_number":269,"context_line":"                # object disappears from the datastore, therefore there is no"},{"line_number":270,"context_line":"                # status transition involved. If PRECOMMIT failures occur,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_08f67093","line":267,"in_reply_to":"3f79a3b5_51ee5a20","updated":"2018-11-07 14:33:40.000000000","message":"Maybe we can rename the method on the validator? I\u0027m just exploring ways to help this be more maintainable. can_be_trunked conveys the notion that a port can be trunked, not that a trunk can be removed from the port. I\u0027m simply exploring ways to make it clearer what we\u0027re checking here.","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":27654,"name":"Hongbin Lu","email":"kira034@163.com","username":"hongbin.lu"},"change_message_id":"2d323d2aee910b8271030e21668a4f508fb93578","unresolved":false,"context_lines":[{"line_number":264,"context_line":"            trunk \u003d self._get_trunk(context, trunk_id)"},{"line_number":265,"context_line":"            rules.trunk_can_be_managed(context, trunk)"},{"line_number":266,"context_line":"            trunk_port_validator \u003d rules.TrunkPortValidator(trunk.port_id)"},{"line_number":267,"context_line":"            if trunk_port_validator.can_be_trunked(context):"},{"line_number":268,"context_line":"                # NOTE(status_police): when a trunk is deleted, the logical"},{"line_number":269,"context_line":"                # object disappears from the datastore, therefore there is no"},{"line_number":270,"context_line":"                # status transition involved. If PRECOMMIT failures occur,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_ee04b91c","line":267,"in_reply_to":"3f79a3b5_5af5b51f","updated":"2018-12-03 22:06:34.000000000","message":"\"can_do_trunk_operation\" is fine with me. Another name in my mind is \"can_be_trunked_or_untrunked\".","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":29093,"name":"Le, Huifeng","email":"huifeng.le@intel.com","username":"hle"},"change_message_id":"4e9ee806d74c5a9e67b7695d8789a4ddc8eab986","unresolved":false,"context_lines":[{"line_number":264,"context_line":"            trunk \u003d self._get_trunk(context, trunk_id)"},{"line_number":265,"context_line":"            rules.trunk_can_be_managed(context, trunk)"},{"line_number":266,"context_line":"            trunk_port_validator \u003d rules.TrunkPortValidator(trunk.port_id)"},{"line_number":267,"context_line":"            if trunk_port_validator.can_be_trunked(context):"},{"line_number":268,"context_line":"                # NOTE(status_police): when a trunk is deleted, the logical"},{"line_number":269,"context_line":"                # object disappears from the datastore, therefore there is no"},{"line_number":270,"context_line":"                # status transition involved. If PRECOMMIT failures occur,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_51ee5a20","line":267,"in_reply_to":"3f79a3b5_7677d3d3","updated":"2018-11-07 08:08:43.000000000","message":"Thanks for the comments, Currently the function to check can_be_trunked and can_be_untrunked is same, it seems adding below function may also cause confusion, any suggestion?\ndef can_be_untrunked():\n    return can_be_trunked()","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":29093,"name":"Le, Huifeng","email":"huifeng.le@intel.com","username":"hle"},"change_message_id":"b996005aea36184e76a1ce6f05740f09e9a1f160","unresolved":false,"context_lines":[{"line_number":264,"context_line":"            trunk \u003d self._get_trunk(context, trunk_id)"},{"line_number":265,"context_line":"            rules.trunk_can_be_managed(context, trunk)"},{"line_number":266,"context_line":"            trunk_port_validator \u003d rules.TrunkPortValidator(trunk.port_id)"},{"line_number":267,"context_line":"            if trunk_port_validator.can_be_trunked(context):"},{"line_number":268,"context_line":"                # NOTE(status_police): when a trunk is deleted, the logical"},{"line_number":269,"context_line":"                # object disappears from the datastore, therefore there is no"},{"line_number":270,"context_line":"                # status transition involved. If PRECOMMIT failures occur,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_5af5b51f","line":267,"in_reply_to":"3f79a3b5_897318b3","updated":"2018-11-29 02:11:43.000000000","message":"in case for renaming, does \"can_do_trunk_operation\" make sense (suppose \"operation\" can include the meaning of both trunk and untrunk)? or any suggestion? Thanks!","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":27654,"name":"Hongbin Lu","email":"kira034@163.com","username":"hongbin.lu"},"change_message_id":"824e8bc374c354c3b6744a56f4c3c3e3f2782de8","unresolved":false,"context_lines":[{"line_number":264,"context_line":"            trunk \u003d self._get_trunk(context, trunk_id)"},{"line_number":265,"context_line":"            rules.trunk_can_be_managed(context, trunk)"},{"line_number":266,"context_line":"            trunk_port_validator \u003d rules.TrunkPortValidator(trunk.port_id)"},{"line_number":267,"context_line":"            if trunk_port_validator.can_be_trunked(context):"},{"line_number":268,"context_line":"                # NOTE(status_police): when a trunk is deleted, the logical"},{"line_number":269,"context_line":"                # object disappears from the datastore, therefore there is no"},{"line_number":270,"context_line":"                # status transition involved. If PRECOMMIT failures occur,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_24d79e4f","line":267,"in_reply_to":"3f79a3b5_d98048a0","updated":"2018-12-10 19:16:17.000000000","message":"lgtm","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":29093,"name":"Le, Huifeng","email":"huifeng.le@intel.com","username":"hle"},"change_message_id":"664c7da8b703f870e2757c25aed0a736826f2d71","unresolved":false,"context_lines":[{"line_number":264,"context_line":"            trunk \u003d self._get_trunk(context, trunk_id)"},{"line_number":265,"context_line":"            rules.trunk_can_be_managed(context, trunk)"},{"line_number":266,"context_line":"            trunk_port_validator \u003d rules.TrunkPortValidator(trunk.port_id)"},{"line_number":267,"context_line":"            if trunk_port_validator.can_be_trunked(context):"},{"line_number":268,"context_line":"                # NOTE(status_police): when a trunk is deleted, the logical"},{"line_number":269,"context_line":"                # object disappears from the datastore, therefore there is no"},{"line_number":270,"context_line":"                # status transition involved. If PRECOMMIT failures occur,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_d98048a0","line":267,"in_reply_to":"3f79a3b5_ee04b91c","updated":"2018-12-07 06:46:43.000000000","message":"@Hongbin,\nThanks for the comments! \"can_be_trunked_or_untrunked\" is more clear, and just use it to update the code.\n\n@Ryan, Hongbin,\nPlease help to review the code and comments again, thanks!","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"},{"author":{"_account_id":27654,"name":"Hongbin Lu","email":"kira034@163.com","username":"hongbin.lu"},"change_message_id":"6eec673cdcbe7e02c064c74a16fea334a509f146","unresolved":false,"context_lines":[{"line_number":264,"context_line":"            trunk \u003d self._get_trunk(context, trunk_id)"},{"line_number":265,"context_line":"            rules.trunk_can_be_managed(context, trunk)"},{"line_number":266,"context_line":"            trunk_port_validator \u003d rules.TrunkPortValidator(trunk.port_id)"},{"line_number":267,"context_line":"            if trunk_port_validator.can_be_trunked(context):"},{"line_number":268,"context_line":"                # NOTE(status_police): when a trunk is deleted, the logical"},{"line_number":269,"context_line":"                # object disappears from the datastore, therefore there is no"},{"line_number":270,"context_line":"                # status transition involved. If PRECOMMIT failures occur,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_897318b3","line":267,"in_reply_to":"3f79a3b5_f7f87319","updated":"2018-11-28 16:30:19.000000000","message":"It looks this method is only used in neutron: http://codesearch.openstack.org/?q\u003dcan_be_trunked\u0026i\u003dnope\u0026files\u003d\u0026repos\u003d . I think it is safe to rename if we can come up with a better name.","commit_id":"ad9b34cf229c06dab5c41ede68e5d3da23680188"}],"neutron/tests/unit/services/trunk/test_plugin.py":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"481b72fe2b1ef2f27a01fbb22122198b72ad3665","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                self.assertRaises(trunk_exc.TrunkInUse,"},{"line_number":110,"context_line":"                              self.trunk_plugin.delete_trunk,"},{"line_number":111,"context_line":"                              self.context, trunk[\u0027id\u0027])"},{"line_number":112,"context_line":"            else:"},{"line_number":113,"context_line":"                pass"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def _test_trunk_create_notify(self, event):"},{"line_number":116,"context_line":"        with self.port() as parent_port:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_7f9da66a","line":113,"range":{"start_line":112,"start_character":0,"end_line":113,"end_character":20},"updated":"2018-09-27 06:23:03.000000000","message":"This is not necessary. Or can you verify if no exception raised?","commit_id":"b89a16166d5264d78c7e244bdb8950d24f3a1fab"},{"author":{"_account_id":29093,"name":"Le, Huifeng","email":"huifeng.le@intel.com","username":"hle"},"change_message_id":"1a377388bd51b89d403cdf3e33995e2b886595c3","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                self.assertRaises(trunk_exc.TrunkInUse,"},{"line_number":110,"context_line":"                              self.trunk_plugin.delete_trunk,"},{"line_number":111,"context_line":"                              self.context, trunk[\u0027id\u0027])"},{"line_number":112,"context_line":"            else:"},{"line_number":113,"context_line":"                pass"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    def _test_trunk_create_notify(self, event):"},{"line_number":116,"context_line":"        with self.port() as parent_port:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_dae7280f","line":113,"range":{"start_line":112,"start_character":0,"end_line":113,"end_character":20},"in_reply_to":"3f79a3b5_7f9da66a","updated":"2018-09-27 07:21:39.000000000","message":"Thanks much for the comments! The code is not required and removed from the new patch.","commit_id":"b89a16166d5264d78c7e244bdb8950d24f3a1fab"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"bee7e6c9129af18f7c8cd47ddc04549555e9d56f","unresolved":false,"context_lines":[{"line_number":26,"context_line":"from neutron.services.trunk import constants"},{"line_number":27,"context_line":"from neutron.services.trunk import drivers"},{"line_number":28,"context_line":"from neutron.services.trunk import rules"},{"line_number":29,"context_line":"from neutron.services.trunk import utils as trunk_utils"},{"line_number":30,"context_line":"from neutron.services.trunk import exceptions as trunk_exc"},{"line_number":31,"context_line":"from neutron.services.trunk import plugin as trunk_plugin"},{"line_number":32,"context_line":"from neutron.services.trunk.seg_types import validators"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_d57372e7","line":29,"range":{"start_line":29,"start_character":0,"end_line":29,"end_character":55},"updated":"2018-09-27 13:39:51.000000000","message":"Not used.","commit_id":"2702e7177e0d17eff5d3eeccbd56d6e9399696bf"},{"author":{"_account_id":29093,"name":"Le, Huifeng","email":"huifeng.le@intel.com","username":"hle"},"change_message_id":"983aaef431e1a7712517a2033ff73d5839e36780","unresolved":false,"context_lines":[{"line_number":26,"context_line":"from neutron.services.trunk import constants"},{"line_number":27,"context_line":"from neutron.services.trunk import drivers"},{"line_number":28,"context_line":"from neutron.services.trunk import rules"},{"line_number":29,"context_line":"from neutron.services.trunk import utils as trunk_utils"},{"line_number":30,"context_line":"from neutron.services.trunk import exceptions as trunk_exc"},{"line_number":31,"context_line":"from neutron.services.trunk import plugin as trunk_plugin"},{"line_number":32,"context_line":"from neutron.services.trunk.seg_types import validators"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_e756f605","line":29,"range":{"start_line":29,"start_character":0,"end_line":29,"end_character":55},"in_reply_to":"3f79a3b5_d57372e7","updated":"2018-09-28 02:27:48.000000000","message":"Thanks for the comments! removed this line in new patch.","commit_id":"2702e7177e0d17eff5d3eeccbd56d6e9399696bf"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"6238a8051e63ef34afb6e3dbd3de89c28657c014","unresolved":false,"context_lines":[{"line_number":104,"context_line":"            core_plugin.update_port(self.context, port[\u0027port\u0027][\u0027id\u0027], port)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"            trunk_port_validator \u003d rules.TrunkPortValidator(trunk[\u0027port_id\u0027])"},{"line_number":107,"context_line":"            if not trunk_port_validator.can_be_trunked(self.context):"},{"line_number":108,"context_line":"                self.assertRaises(trunk_exc.TrunkInUse,"},{"line_number":109,"context_line":"                              self.trunk_plugin.delete_trunk,"},{"line_number":110,"context_line":"                              self.context, trunk[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_786092d1","line":107,"updated":"2018-10-18 16:10:03.000000000","message":"I don\u0027t think this is testing anything now since you\u0027ll only call delete_trunk when you know it will fail, and I don\u0027t think this will ever return True in this test right?","commit_id":"b1213451560301df1206f05e92d39382958f05bb"},{"author":{"_account_id":29093,"name":"Le, Huifeng","email":"huifeng.le@intel.com","username":"hle"},"change_message_id":"4b2760fde83f0d42031004b71f8404ba8084a1ed","unresolved":false,"context_lines":[{"line_number":104,"context_line":"            core_plugin.update_port(self.context, port[\u0027port\u0027][\u0027id\u0027], port)"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"            trunk_port_validator \u003d rules.TrunkPortValidator(trunk[\u0027port_id\u0027])"},{"line_number":107,"context_line":"            if not trunk_port_validator.can_be_trunked(self.context):"},{"line_number":108,"context_line":"                self.assertRaises(trunk_exc.TrunkInUse,"},{"line_number":109,"context_line":"                              self.trunk_plugin.delete_trunk,"},{"line_number":110,"context_line":"                              self.context, trunk[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_193ab9e5","line":107,"in_reply_to":"3f79a3b5_786092d1","updated":"2018-10-19 02:25:21.000000000","message":"Thanks much for the comments! \nI am using the test case \" unit.services.trunk.test_rules.test_can_be_trunked_returns_false\" for this case. since trunk_utils.is_driver_compatible() is not mocked (e.g. return true), so trunk_port_validator.can_be_trunked(self.context) will return false and the delete_trunk will still be tested in this case.\nand the trunk_plugin is required to be added to directory or trunk_port_validator.can_be_trunked will throw exception instead of return false.","commit_id":"b1213451560301df1206f05e92d39382958f05bb"}]}