)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5292,"name":"Christopher Yeoh","email":"cbkyeoh@gmail.com","username":"cyeoh-0"},"change_message_id":"d557a7a0db80e9f769cd458b3465ba0f7ae2b659","unresolved":false,"context_lines":[{"line_number":15,"context_line":"* network api allocate_port_for_instance return multiple network"},{"line_number":16,"context_line":"* calling driver attach_interface failed"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This patch covers Alex\u0027s patch https://review.openstack.org/#/c/106719/"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: I4fe67b5ce1b5ed41412d824a8c10b9a5e4fb325f"},{"line_number":21,"context_line":"Closes-Bug: 1346108"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"baada198_64ac45a0","line":18,"updated":"2014-07-25 02:36:07.000000000","message":"I think you should reference the changeid rather than the gerrit url so it will referencable in the future","commit_id":"267be489724291c1ece5fb59ae077bc2fb788ca0"},{"author":{"_account_id":5292,"name":"Christopher Yeoh","email":"cbkyeoh@gmail.com","username":"cyeoh-0"},"change_message_id":"fa37d9565bc48744b5f79c83c0d4f9fba5e19db2","unresolved":false,"context_lines":[{"line_number":15,"context_line":"* network api allocate_port_for_instance return multiple network"},{"line_number":16,"context_line":"* calling driver attach_interface failed"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This patch covers Alex\u0027s patch https://review.openstack.org/#/c/106719/"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: I4fe67b5ce1b5ed41412d824a8c10b9a5e4fb325f"},{"line_number":21,"context_line":"Closes-Bug: 1346108"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"baada198_c4e0b199","line":18,"in_reply_to":"baada198_64ac45a0","updated":"2014-07-25 02:37:17.000000000","message":"oh I see you\u0027ve unabanoned one of his patches. Maybe just add him as a co-author?","commit_id":"267be489724291c1ece5fb59ae077bc2fb788ca0"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"48183b0faf4aa4c8b059ee3567c30bb6495e0ce2","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This patch add exception handle, and it will cover 2 cases:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"* network api allocate_port_for_instance return multiple network"},{"line_number":16,"context_line":"* calling driver attach_interface failed"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This patch covers Alex\u0027s patch https://review.openstack.org/#/c/106719/"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"baada198_02c984aa","line":15,"updated":"2014-08-04 07:45:05.000000000","message":"I don\u0027t think you change the logic here\nyou only update the log message format, right?","commit_id":"49b0942f32e6d9006185daa56832ea50e1f3ef8b"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"b18b07dd896cd79ba23fd164c49c3995cdd9dae9","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This patch add exception handle, and it will cover 2 cases:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"* network api allocate_port_for_instance return multiple network"},{"line_number":16,"context_line":"* calling driver attach_interface failed"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This patch covers Alex\u0027s patch https://review.openstack.org/#/c/106719/"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"baada198_050b4e7c","line":15,"in_reply_to":"baada198_02c984aa","updated":"2014-08-04 08:32:48.000000000","message":"Yes ,I changed, please see my comment in manager.py","commit_id":"49b0942f32e6d9006185daa56832ea50e1f3ef8b"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"8b4cc2947f3f8d3cb185cd241eb4ce2cd483fda1","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Eli Qiao \u003ctaget@linux.vnet.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-08-09 19:09:26 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Hanle exception better when attaching interface"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently, when attaching interface to an instance, nova doesn\u0027t"},{"line_number":10,"context_line":"handle the exception when calling `driver.attach_interface`, so"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":20,"id":"baada198_f3464ea7","line":7,"updated":"2014-08-09 13:51:12.000000000","message":"s/Hanle/Handle/","commit_id":"83b4b4f0725e4b079f38f0a91235f3490c334402"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"5461e58aa8772fc593fb1f52994149f33cd2e93a","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Eli Qiao \u003ctaget@linux.vnet.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-08-09 19:09:26 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Hanle exception better when attaching interface"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently, when attaching interface to an instance, nova doesn\u0027t"},{"line_number":10,"context_line":"handle the exception when calling `driver.attach_interface`, so"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":20,"id":"baada198_a46632ba","line":7,"in_reply_to":"baada198_f3464ea7","updated":"2014-08-11 12:57:55.000000000","message":"Done","commit_id":"83b4b4f0725e4b079f38f0a91235f3490c334402"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"d11e444ce17e718b7d6e7a3ff0f916dd81df1e44","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Eli Qiao \u003ctaget@linux.vnet.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-08-18 15:33:41 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Hanle exception better when attaching interface"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently, when attaching interface to an instance, nova doesn\u0027t"},{"line_number":10,"context_line":"handle the exception when calling `driver.attach_interface`, so"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":24,"id":"1abeadc6_bf46e521","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":5},"updated":"2014-08-18 10:27:53.000000000","message":"nit: s/Hanle/Handle","commit_id":"0148cfd560183182eb5182f0dfd667101c6202ac"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"be1c6568b91b82c94d74f83f8c547dc90b840ad7","unresolved":false,"context_lines":[{"line_number":13,"context_line":"This patch add exception handle, and it will cover 2 cases:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"* network api allocate_port_for_instance will raise an exception.NetworkAmbiguous"},{"line_number":16,"context_line":"  error if multiple networks exists, will be handled by API layer."},{"line_number":17,"context_line":"* exception happened when calling attach_interface driver, then calling"},{"line_number":18,"context_line":"  deallocate_interface to release the ports."},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":25,"id":"1abeadc6_94ee59eb","line":16,"updated":"2014-08-18 18:38:18.000000000","message":"So I think you\u0027re trying to justify the removal of the \"if len(network_info) !\u003d 1\" check in the compute manager here, right?  But that doesn\u0027t really have anything to do with the port cleanup when the virt driver fails to attach the interface, so I think it should probably be a separate change if it\u0027s just redundant.  Plus I don\u0027t think the check in the compute manager is the same as checking for multiple networks in the request, it\u0027s about checking to see that only 1 port is created (the naming of the variables makes this confusing).\n\nI also think you\u0027d want to say \u0027will be handled by the network API layer\u0027 here, there are many API layers. :)","commit_id":"da609d016634a3df610b1dbfc68e2c92f5a60fcb"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"c8cf98979879a0ad9c774e64e35c0d11d0954fa6","unresolved":false,"context_lines":[{"line_number":13,"context_line":"This patch add exception handle, and it will cover 2 cases:"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"* network api allocate_port_for_instance will raise an exception.NetworkAmbiguous"},{"line_number":16,"context_line":"  error if multiple networks exists, will be handled by API layer."},{"line_number":17,"context_line":"* exception happened when calling attach_interface driver, then calling"},{"line_number":18,"context_line":"  deallocate_interface to release the ports."},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":25,"id":"1abeadc6_4bf972e8","line":16,"in_reply_to":"1abeadc6_94ee59eb","updated":"2014-08-19 02:23:25.000000000","message":"hi Matt, thanks for you comments,\nif you take a loot at the code in allocate_for_instance in neutronv2/api.py , you will find this. we don\u0027t need to check the network_info length , neutronv2 api will raise an\nexception exception.NetworkAmbiguous(msg), so we won\u0027t clean up the port since is it not allocated even.\n\nthis exception will be handled by nova api layer(sorry for the confused, you can check the test case in nova v3  attach_interface api) \n        if (not requested_networks\n            or requested_networks \u003d\u003d [(None, None, None)]):\n            # bug/1267723 - if no network is requested and more\n            # than one is available then raise NetworkAmbiguous Exception\n            if len(nets) \u003e 1:\n                msg \u003d _(\"Multiple possible networks found, use a Network \"\n                         \"ID to be more specific.\")\n                raise exception.NetworkAmbiguous(msg)\n            ordered_networks.append((nets[0][\u0027id\u0027], None, None))","commit_id":"da609d016634a3df610b1dbfc68e2c92f5a60fcb"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"be1c6568b91b82c94d74f83f8c547dc90b840ad7","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I4fe67b5ce1b5ed41412d824a8c10b9a5e4fb325f"},{"line_number":23,"context_line":"Closes-Bug: 1346108"},{"line_number":24,"context_line":"Closes-Bug: 1341498"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":25,"id":"1abeadc6_d9abaa87","line":24,"updated":"2014-08-18 18:38:18.000000000","message":"nit: bug 1341498 was marked as a duplicate of bug 1338551 so could probably update that here.","commit_id":"da609d016634a3df610b1dbfc68e2c92f5a60fcb"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"c8cf98979879a0ad9c774e64e35c0d11d0954fa6","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I4fe67b5ce1b5ed41412d824a8c10b9a5e4fb325f"},{"line_number":23,"context_line":"Closes-Bug: 1346108"},{"line_number":24,"context_line":"Closes-Bug: 1341498"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":25,"id":"1abeadc6_8bdaaa76","line":24,"in_reply_to":"1abeadc6_d9abaa87","updated":"2014-08-19 02:23:25.000000000","message":"yes, it is partially fix bug 1338551 , I will update it in next ps.","commit_id":"da609d016634a3df610b1dbfc68e2c92f5a60fcb"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"97dcfdc6719b6220713008662a6fef78a3c1b493","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently, when attaching interface to an instance, nova doesn\u0027t"},{"line_number":10,"context_line":"handle the exception when calling `driver.attach_interface`, so"},{"line_number":11,"context_line":"the network info will leak when exception happened."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This patch deallocates port when attaching interface failed,"},{"line_number":14,"context_line":"and give warn messages when deallocate port failed."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":33,"id":"fa81d914_8a9a7261","line":11,"updated":"2015-01-29 09:35:13.000000000","message":"a question \nthe network info will leak means leak info to end user who should not know it or something like memory leak?","commit_id":"249dad6f084e40eb627848dad6c1fcc8166fbc3b"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"e9719f2ca4922d8218407570d48b882bf2b6e9dd","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently, when attaching interface to an instance, nova doesn\u0027t"},{"line_number":10,"context_line":"handle the exception when calling `driver.attach_interface`, so"},{"line_number":11,"context_line":"the network info will leak when exception happened."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This patch deallocates port when attaching interface failed,"},{"line_number":14,"context_line":"and give warn messages when deallocate port failed."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":33,"id":"fa81d914_4e014337","line":11,"in_reply_to":"fa81d914_8a9a7261","updated":"2015-01-30 01:54:29.000000000","message":"previous one.\nnova will create a new networkinfo (new port in neutron) ,but won\u0027t clenaup the port if exception happen. this port can be seen in nova network-list, but it should be clean up since this is not created by end user.","commit_id":"249dad6f084e40eb627848dad6c1fcc8166fbc3b"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"97dcfdc6719b6220713008662a6fef78a3c1b493","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I4fe67b5ce1b5ed41412d824a8c10b9a5e4fb325f"},{"line_number":17,"context_line":"Closes-bug: #1338551"},{"line_number":18,"context_line":"Co-authored-by: Alex Xu \u003cxuhj@linux.vnet.ibm.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":33,"id":"fa81d914_2a16e6f1","line":18,"updated":"2015-01-29 09:35:13.000000000","message":"change his mail :)","commit_id":"249dad6f084e40eb627848dad6c1fcc8166fbc3b"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"e9719f2ca4922d8218407570d48b882bf2b6e9dd","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I4fe67b5ce1b5ed41412d824a8c10b9a5e4fb325f"},{"line_number":17,"context_line":"Closes-bug: #1338551"},{"line_number":18,"context_line":"Co-authored-by: Alex Xu \u003cxuhj@linux.vnet.ibm.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":33,"id":"fa81d914_6efa8744","line":18,"in_reply_to":"fa81d914_2a16e6f1","updated":"2015-01-30 01:54:29.000000000","message":"sure, thanks, me too :)","commit_id":"249dad6f084e40eb627848dad6c1fcc8166fbc3b"}],"nova/compute/manager.py":[{"author":{"_account_id":5292,"name":"Christopher Yeoh","email":"cbkyeoh@gmail.com","username":"cyeoh-0"},"change_message_id":"d557a7a0db80e9f769cd458b3465ba0f7ae2b659","unresolved":false,"context_lines":[{"line_number":4507,"context_line":"                context, self.image_api, image_ref, instance)"},{"line_number":4508,"context_line":""},{"line_number":4509,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4510,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4511,"context_line":"            for net in network_info:"},{"line_number":4512,"context_line":"                port_id \u003d net[\u0027id\u0027]"},{"line_number":4513,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocated the \""}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_2465ed44","line":4510,"updated":"2014-07-25 02:36:07.000000000","message":"have a look at uses of with excutils.save_and_reraise_exception in this file to make sure we don\u0027t accidentally lose the exception here in case another one gets raised","commit_id":"267be489724291c1ece5fb59ae077bc2fb788ca0"},{"author":{"_account_id":5292,"name":"Christopher Yeoh","email":"cbkyeoh@gmail.com","username":"cyeoh-0"},"change_message_id":"d557a7a0db80e9f769cd458b3465ba0f7ae2b659","unresolved":false,"context_lines":[{"line_number":4512,"context_line":"                port_id \u003d net[\u0027id\u0027]"},{"line_number":4513,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocated the \""},{"line_number":4514,"context_line":"                                     \"port %s\"), port_id)"},{"line_number":4515,"context_line":"                self.network_api.deallocate_port_for_instance("},{"line_number":4516,"context_line":"                    context, instance, port_id)"},{"line_number":4517,"context_line":"            raise"},{"line_number":4518,"context_line":"        return network_info[0]"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_c44991b9","line":4515,"updated":"2014-07-25 02:36:07.000000000","message":"THis could fail and we probably want to log if that happens","commit_id":"267be489724291c1ece5fb59ae077bc2fb788ca0"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"eb36242ef5029f572ea4aa831ff492b91a17d717","unresolved":false,"context_lines":[{"line_number":4514,"context_line":"        try:"},{"line_number":4515,"context_line":"            if len(network_info) !\u003d 1:"},{"line_number":4516,"context_line":"                LOG.error(_LE(\"allocate_port_for_instance returned %(ports)s \""},{"line_number":4517,"context_line":"                             \"ports\"), {\u0027ports\u0027: len(network_info)})"},{"line_number":4518,"context_line":"                raise exception.InterfaceAttachFailed(instance\u003dinstance)"},{"line_number":4519,"context_line":"            image_ref \u003d instance.get(\u0027image_ref\u0027)"},{"line_number":4520,"context_line":"            image_meta \u003d compute_utils.get_image_metadata("}],"source_content_type":"text/x-python","patch_set":12,"id":"baada198_64e6da17","line":4517,"updated":"2014-08-03 17:35:07.000000000","message":"alignment. the \" should be under the \"","commit_id":"2c61d8ad58bacffdbe4569cf4dbcd02a00e993bb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"eb36242ef5029f572ea4aa831ff492b91a17d717","unresolved":false,"context_lines":[{"line_number":4521,"context_line":"                context, self.image_api, image_ref, instance)"},{"line_number":4522,"context_line":""},{"line_number":4523,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4524,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4525,"context_line":"            for net in network_info:"},{"line_number":4526,"context_line":"                port_id \u003d net[\u0027id\u0027]"},{"line_number":4527,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate the \""}],"source_content_type":"text/x-python","patch_set":12,"id":"baada198_c4e7a60d","line":4524,"updated":"2014-08-03 17:35:07.000000000","message":"you should use with excutils.save_and_reraise_exception(): here and then you do not need to do the raise in line 4531","commit_id":"2c61d8ad58bacffdbe4569cf4dbcd02a00e993bb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"eb36242ef5029f572ea4aa831ff492b91a17d717","unresolved":false,"context_lines":[{"line_number":4522,"context_line":""},{"line_number":4523,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4524,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4525,"context_line":"            for net in network_info:"},{"line_number":4526,"context_line":"                port_id \u003d net[\u0027id\u0027]"},{"line_number":4527,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate the \""},{"line_number":4528,"context_line":"                                     \"port %s\"), port_id)"}],"source_content_type":"text/x-python","patch_set":12,"id":"baada198_04a43eb7","line":4525,"updated":"2014-08-03 17:35:07.000000000","message":"will this deallocate all ports? i was under the impression that we should just deallocate the one that was allocated","commit_id":"2c61d8ad58bacffdbe4569cf4dbcd02a00e993bb"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"f8dab8daa7fae418c4c5f39710a5a0bc4bd90a14","unresolved":false,"context_lines":[{"line_number":4522,"context_line":""},{"line_number":4523,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4524,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4525,"context_line":"            for net in network_info:"},{"line_number":4526,"context_line":"                port_id \u003d net[\u0027id\u0027]"},{"line_number":4527,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate the \""},{"line_number":4528,"context_line":"                                     \"port %s\"), port_id)"}],"source_content_type":"text/x-python","patch_set":12,"id":"baada198_5aa44a06","line":4525,"in_reply_to":"baada198_04a43eb7","updated":"2014-08-04 02:32:14.000000000","message":"hi garyk,\nthanks for you comments.\nnetwork_info is newly allocated at 4512. so we need to deallocated.\nAnd as you can see from the bug https://bugs.launchpad.net/nova/+bug/1346108.\nself.network_api.allocate_port_for_instance in line 4521 may return multiple network if we defined multiple networks and don\u0027t specify networkid when attach a new port.\nso len(network_info) !\u003d 1:  and will raise exception.InterfaceAttachFailed(instance\u003dinstance), at 4516 , we need to deallocate them.\nso this patches fix two cases:\n1 exception comes from self.driver.attach_interface (len(network_info) \u003d\u003d 1)\n2 exception comes from if len(network_info) !\u003d 1:","commit_id":"2c61d8ad58bacffdbe4569cf4dbcd02a00e993bb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"eb36242ef5029f572ea4aa831ff492b91a17d717","unresolved":false,"context_lines":[{"line_number":4524,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4525,"context_line":"            for net in network_info:"},{"line_number":4526,"context_line":"                port_id \u003d net[\u0027id\u0027]"},{"line_number":4527,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate the \""},{"line_number":4528,"context_line":"                                     \"port %s\"), port_id)"},{"line_number":4529,"context_line":"                self.network_api.deallocate_port_for_instance("},{"line_number":4530,"context_line":"                    context, instance, port_id)"}],"source_content_type":"text/x-python","patch_set":12,"id":"baada198_04bd1ed7","line":4527,"updated":"2014-08-03 17:35:07.000000000","message":"alignment","commit_id":"2c61d8ad58bacffdbe4569cf4dbcd02a00e993bb"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"865b503a93f0f6dc3fda4bccf31a866e4c6be718","unresolved":false,"context_lines":[{"line_number":4510,"context_line":"        network_info \u003d self.network_api.allocate_port_for_instance("},{"line_number":4511,"context_line":"            context, instance, port_id, network_id, requested_ip)"},{"line_number":4512,"context_line":"        try:"},{"line_number":4513,"context_line":"            if len(network_info) !\u003d 1:"},{"line_number":4514,"context_line":"                LOG.error(_LE(\"allocate_port_for_instance returned %(ports)s \""},{"line_number":4515,"context_line":"                              \"ports\"), {\u0027ports\u0027: len(network_info)})"},{"line_number":4516,"context_line":"                raise exception.InterfaceAttachFailed(instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_bec7e863","line":4513,"updated":"2014-08-05 06:28:31.000000000","message":"why would this return more than one port?","commit_id":"49b0942f32e6d9006185daa56832ea50e1f3ef8b"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"bafaf9470dea32ef7f36ec7e446239263796d9e2","unresolved":false,"context_lines":[{"line_number":4510,"context_line":"        network_info \u003d self.network_api.allocate_port_for_instance("},{"line_number":4511,"context_line":"            context, instance, port_id, network_id, requested_ip)"},{"line_number":4512,"context_line":"        try:"},{"line_number":4513,"context_line":"            if len(network_info) !\u003d 1:"},{"line_number":4514,"context_line":"                LOG.error(_LE(\"allocate_port_for_instance returned %(ports)s \""},{"line_number":4515,"context_line":"                              \"ports\"), {\u0027ports\u0027: len(network_info)})"},{"line_number":4516,"context_line":"                raise exception.InterfaceAttachFailed(instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_a7d514ba","line":4513,"in_reply_to":"baada198_bec7e863","updated":"2014-08-05 11:41:54.000000000","message":"if a tenant has 1 public and 1 private network, when booting a instance without specify the networkid. boom...\nplease see https://bugs.launchpad.net/nova/+bug/1346108.\n\nbesides, I have test case for this situation:\nsee test_attach_interface_with_multi_networks_failed() in test.py\n\nEli.","commit_id":"49b0942f32e6d9006185daa56832ea50e1f3ef8b"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"48183b0faf4aa4c8b059ee3567c30bb6495e0ce2","unresolved":false,"context_lines":[{"line_number":4521,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4522,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4523,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4524,"context_line":"                for net in network_info:"},{"line_number":4525,"context_line":"                    port_id \u003d net[\u0027id\u0027]"},{"line_number":4526,"context_line":"                    LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4527,"context_line":"                                 \"the port %s\"), port_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_5c11a25a","line":4524,"updated":"2014-08-04 07:45:05.000000000","message":"we already got a len \u003d 1 check before\n\ndo we need a for loop here?","commit_id":"49b0942f32e6d9006185daa56832ea50e1f3ef8b"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"292184d56ef8fe2eacdb0748b3ac6fdcc09562ef","unresolved":false,"context_lines":[{"line_number":4521,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4522,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4523,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4524,"context_line":"                for net in network_info:"},{"line_number":4525,"context_line":"                    port_id \u003d net[\u0027id\u0027]"},{"line_number":4526,"context_line":"                    LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4527,"context_line":"                                 \"the port %s\"), port_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_e8ee3120","line":4524,"in_reply_to":"baada198_4539c63d","updated":"2014-08-04 08:46:08.000000000","message":"ok, I see, I missed that part","commit_id":"49b0942f32e6d9006185daa56832ea50e1f3ef8b"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"b18b07dd896cd79ba23fd164c49c3995cdd9dae9","unresolved":false,"context_lines":[{"line_number":4521,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4522,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4523,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4524,"context_line":"                for net in network_info:"},{"line_number":4525,"context_line":"                    port_id \u003d net[\u0027id\u0027]"},{"line_number":4526,"context_line":"                    LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4527,"context_line":"                                 \"the port %s\"), port_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_4539c63d","line":4524,"in_reply_to":"baada198_5c11a25a","updated":"2014-08-04 08:32:48.000000000","message":"we need the loop.\nif the exception comes from  if len(network_info) !\u003d 1: \nthen we need the loop to release the multiple ports we allocated.\nAs I comment in the commit message , it cover 2 cases:\n1 exception from 4516, we need loop\n2 exception from self.driver.attach_interface","commit_id":"49b0942f32e6d9006185daa56832ea50e1f3ef8b"},{"author":{"_account_id":5292,"name":"Christopher Yeoh","email":"cbkyeoh@gmail.com","username":"cyeoh-0"},"change_message_id":"e76dea30d7acc4458528130c7d1a1740c797a379","unresolved":false,"context_lines":[{"line_number":4525,"context_line":"                    port_id \u003d net[\u0027id\u0027]"},{"line_number":4526,"context_line":"                    LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4527,"context_line":"                                 \"the port %s\"), port_id)"},{"line_number":4528,"context_line":"                    self.network_api.deallocate_port_for_instance("},{"line_number":4529,"context_line":"                        context, instance, port_id)"},{"line_number":4530,"context_line":"        return network_info[0]"},{"line_number":4531,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_0a308e05","line":4528,"updated":"2014-08-05 08:51:15.000000000","message":"Need to handle exceptions from here too? Also will need a testcase for it","commit_id":"49b0942f32e6d9006185daa56832ea50e1f3ef8b"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"bafaf9470dea32ef7f36ec7e446239263796d9e2","unresolved":false,"context_lines":[{"line_number":4525,"context_line":"                    port_id \u003d net[\u0027id\u0027]"},{"line_number":4526,"context_line":"                    LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4527,"context_line":"                                 \"the port %s\"), port_id)"},{"line_number":4528,"context_line":"                    self.network_api.deallocate_port_for_instance("},{"line_number":4529,"context_line":"                        context, instance, port_id)"},{"line_number":4530,"context_line":"        return network_info[0]"},{"line_number":4531,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_077960c5","line":4528,"in_reply_to":"baada198_0a308e05","updated":"2014-08-05 11:41:54.000000000","message":"I will catch the exception when calling deallocate_port_for_instance, then logging a warning message or try to deallocate it again?\n\nfor the testcases, what the testcases would be like? test if we handle the exception when deallocate failed correctly?\n\nEli.","commit_id":"49b0942f32e6d9006185daa56832ea50e1f3ef8b"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"be11f65a76562fe9a84d63229fe3dca84691c170","unresolved":false,"context_lines":[{"line_number":4522,"context_line":"        network_info \u003d self.network_api.allocate_port_for_instance("},{"line_number":4523,"context_line":"            context, instance, port_id, network_id, requested_ip)"},{"line_number":4524,"context_line":"        try:"},{"line_number":4525,"context_line":"            if len(network_info) !\u003d 1:"},{"line_number":4526,"context_line":"                LOG.error(_LE(\"allocate_port_for_instance returned %(ports)s \""},{"line_number":4527,"context_line":"                              \"ports\"), {\u0027ports\u0027: len(network_info)})"},{"line_number":4528,"context_line":"                raise exception.InterfaceAttachFailed(instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":14,"id":"baada198_be8fba21","line":4525,"updated":"2014-08-06 07:03:47.000000000","message":"Feels like this check is just asserting the network code is not broken. I\u0027m not sure this is needed, but... I guess I\u0027m okay with it.","commit_id":"b27831d760b8ac349ab65832bcf9773ed960f55b"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"change_message_id":"be11f65a76562fe9a84d63229fe3dca84691c170","unresolved":false,"context_lines":[{"line_number":4524,"context_line":"        try:"},{"line_number":4525,"context_line":"            if len(network_info) !\u003d 1:"},{"line_number":4526,"context_line":"                LOG.error(_LE(\"allocate_port_for_instance returned %(ports)s \""},{"line_number":4527,"context_line":"                              \"ports\"), {\u0027ports\u0027: len(network_info)})"},{"line_number":4528,"context_line":"                raise exception.InterfaceAttachFailed(instance\u003dinstance)"},{"line_number":4529,"context_line":"            image_ref \u003d instance.get(\u0027image_ref\u0027)"},{"line_number":4530,"context_line":"            image_meta \u003d compute_utils.get_image_metadata("}],"source_content_type":"text/x-python","patch_set":14,"id":"baada198_5ec3fe57","line":4527,"updated":"2014-08-06 07:03:47.000000000","message":"I\u0027d prefer the log messages in here have instance\u003dinstance as kwargs passed to the logging calls so we get information about the instance in the log messages. You can find other examples of this in this file..","commit_id":"b27831d760b8ac349ab65832bcf9773ed960f55b"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"7c1c4dbd2daf9e625a6c343ca26c8416d98994d9","unresolved":false,"context_lines":[{"line_number":4537,"context_line":"                for net in network_info:"},{"line_number":4538,"context_line":"                    port_id \u003d net[\u0027id\u0027]"},{"line_number":4539,"context_line":"                    LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4540,"context_line":"                                 \"the port %s\"), port_id)"},{"line_number":4541,"context_line":"                    try:"},{"line_number":4542,"context_line":"                        self.network_api.deallocate_port_for_instance("},{"line_number":4543,"context_line":"                            context, instance, port_id)"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_3456692c","line":4540,"updated":"2014-08-06 08:12:56.000000000","message":"add instance\u003dinstance too.","commit_id":"b9f45774ba7bc3267c8a371095409789e0aca1fa"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"35184af7928d6985fc2bdba0a30dd71b2d809fe2","unresolved":false,"context_lines":[{"line_number":4542,"context_line":"                        self.network_api.deallocate_port_for_instance("},{"line_number":4543,"context_line":"                            context, instance, port_id)"},{"line_number":4544,"context_line":"                    except Exception:"},{"line_number":4545,"context_line":"                        LOG.warn(_LW(\"deallocate port %s failed\"), port_id)"},{"line_number":4546,"context_line":"        return network_info[0]"},{"line_number":4547,"context_line":""},{"line_number":4548,"context_line":"    @object_compat"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_7479c1c4","line":4545,"updated":"2014-08-06 08:12:01.000000000","message":"this LOG.warn will hide detail messsage, how about use LOG.exception","commit_id":"b9f45774ba7bc3267c8a371095409789e0aca1fa"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"7c1c4dbd2daf9e625a6c343ca26c8416d98994d9","unresolved":false,"context_lines":[{"line_number":4542,"context_line":"                        self.network_api.deallocate_port_for_instance("},{"line_number":4543,"context_line":"                            context, instance, port_id)"},{"line_number":4544,"context_line":"                    except Exception:"},{"line_number":4545,"context_line":"                        LOG.warn(_LW(\"deallocate port %s failed\"), port_id)"},{"line_number":4546,"context_line":"        return network_info[0]"},{"line_number":4547,"context_line":""},{"line_number":4548,"context_line":"    @object_compat"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_7450e12c","line":4545,"in_reply_to":"baada198_7479c1c4","updated":"2014-08-06 08:12:56.000000000","message":"and add instance\u003dinstance","commit_id":"b9f45774ba7bc3267c8a371095409789e0aca1fa"},{"author":{"_account_id":9623,"name":"Paul Ward","email":"wpward@linux.vnet.ibm.com","username":"wpward"},"change_message_id":"1de90b01c4948ba5172818e88642c48b66d766ad","unresolved":false,"context_lines":[{"line_number":4522,"context_line":"        network_info \u003d self.network_api.allocate_port_for_instance("},{"line_number":4523,"context_line":"            context, instance, port_id, network_id, requested_ip)"},{"line_number":4524,"context_line":"        try:"},{"line_number":4525,"context_line":"            if len(network_info) !\u003d 1:"},{"line_number":4526,"context_line":"                LOG.error(_LE(\"allocate_port_for_instance returned %(ports)s \""},{"line_number":4527,"context_line":"                              \"ports\"), {\u0027ports\u0027: len(network_info)},"},{"line_number":4528,"context_line":"                              instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_41a06f42","line":4525,"updated":"2014-08-06 16:00:02.000000000","message":"As I mentioned in email, I feel like this is a bandaid check rather than fixing the root cause.  Now... it\u0027s valid for allocate_port_for_instance to return more than one network in certain cases, as Eli mentioned in a previous patch set (if tenant has a public and private network and no network_id was passed in), however, in this path, it seems we would be better served if we actually validate the input BEFORE calling allocate_port_for_instance and raise the exception there.  Then there\u0027s no cleanup to take care of.  I\u0027d probably keep this existing check here for paranoia\u0027s sake, better safe than sorry.  But I think this could be handled much earlier and avoid unnecessary churn and cleanup.","commit_id":"0d452e802e866cbbd9052237ef0219b625798b2f"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"0e6c80966df97e16bc9859398f4eec2d1960fb75","unresolved":false,"context_lines":[{"line_number":4522,"context_line":"        network_info \u003d self.network_api.allocate_port_for_instance("},{"line_number":4523,"context_line":"            context, instance, port_id, network_id, requested_ip)"},{"line_number":4524,"context_line":"        try:"},{"line_number":4525,"context_line":"            if len(network_info) !\u003d 1:"},{"line_number":4526,"context_line":"                LOG.error(_LE(\"allocate_port_for_instance returned %(ports)s \""},{"line_number":4527,"context_line":"                              \"ports\"), {\u0027ports\u0027: len(network_info)},"},{"line_number":4528,"context_line":"                              instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_d2635a43","line":4525,"in_reply_to":"baada198_41a06f42","updated":"2014-08-07 03:10:13.000000000","message":"yea, I think Paul is right. We should call, network_api.validate_networks at nova-api side. \u0027validate_networks\u0027 already doing that check, and also have other checks for input.","commit_id":"0d452e802e866cbbd9052237ef0219b625798b2f"},{"author":{"_account_id":9623,"name":"Paul Ward","email":"wpward@linux.vnet.ibm.com","username":"wpward"},"change_message_id":"1de90b01c4948ba5172818e88642c48b66d766ad","unresolved":false,"context_lines":[{"line_number":4534,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4535,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4536,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4537,"context_line":"                for net in network_info:"},{"line_number":4538,"context_line":"                    port_id \u003d net[\u0027id\u0027]"},{"line_number":4539,"context_line":"                    LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4540,"context_line":"                                 \"the port %(port_id)s\"),"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_e1134366","line":4537,"updated":"2014-08-06 16:00:02.000000000","message":"I think this needs to be a combination of the code here along with what we were doing in this fix: https://review.openstack.org/#/c/105238/5/nova/compute/manager.py\n\nif port_id is None, meaning we implicitly created the port during this attach, then we need to do a full deallocate_port_for_instance, thus deleting the neutron port that was created.  However, if port_id is not None, we must somehow remove it from the instance but NOT delete it from neutron.  I don\u0027t think we should be deleting objects the user explicitly created before calling this code path.","commit_id":"0d452e802e866cbbd9052237ef0219b625798b2f"},{"author":{"_account_id":9623,"name":"Paul Ward","email":"wpward@linux.vnet.ibm.com","username":"wpward"},"change_message_id":"55495582983dfc7206d7addbb58f0440b3a1bf3e","unresolved":false,"context_lines":[{"line_number":4532,"context_line":"                LOG.exception(_LE(\"mutiple networks were found\"),"},{"line_number":4533,"context_line":"                                  instance\u003dinstance)"},{"line_number":4534,"context_line":"        network_info \u003d self.network_api.allocate_port_for_instance("},{"line_number":4535,"context_line":"             context, instance, port_id, network_id, requested_ip)"},{"line_number":4536,"context_line":"        image_ref \u003d instance.get(\u0027image_ref\u0027)"},{"line_number":4537,"context_line":"        image_meta \u003d compute_utils.get_image_metadata("},{"line_number":4538,"context_line":"            context, self.image_api, image_ref, instance)"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_1ca159a8","line":4535,"updated":"2014-08-07 13:42:33.000000000","message":"This extra space is probably causing pep8 failure","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"8351863363c4f7fd2890ec10114f4414eecc975a","unresolved":false,"context_lines":[{"line_number":4532,"context_line":"                LOG.exception(_LE(\"mutiple networks were found\"),"},{"line_number":4533,"context_line":"                                  instance\u003dinstance)"},{"line_number":4534,"context_line":"        network_info \u003d self.network_api.allocate_port_for_instance("},{"line_number":4535,"context_line":"             context, instance, port_id, network_id, requested_ip)"},{"line_number":4536,"context_line":"        image_ref \u003d instance.get(\u0027image_ref\u0027)"},{"line_number":4537,"context_line":"        image_meta \u003d compute_utils.get_image_metadata("},{"line_number":4538,"context_line":"            context, self.image_api, image_ref, instance)"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_3f960f3f","line":4535,"in_reply_to":"baada198_1ca159a8","updated":"2014-08-07 14:56:42.000000000","message":"Done","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"ee656df3b8d582437adb0b02c7d6789eb1e8a7a4","unresolved":false,"context_lines":[{"line_number":4541,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4542,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4543,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4544,"context_line":"                if port_id is None:"},{"line_number":4545,"context_line":"                    tmp_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4546,"context_line":"                    LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4547,"context_line":"                                 \"the port %(port_id)s\"),"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_bc54ed22","line":4544,"updated":"2014-08-07 13:49:34.000000000","message":"A comment here explaining why we check for port_id being none may be good.  In the other change set where this was being worked, that question came up a bit.","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"8351863363c4f7fd2890ec10114f4414eecc975a","unresolved":false,"context_lines":[{"line_number":4541,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4542,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4543,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4544,"context_line":"                if port_id is None:"},{"line_number":4545,"context_line":"                    tmp_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4546,"context_line":"                    LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4547,"context_line":"                                 \"the port %(port_id)s\"),"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_9f9abb5e","line":4544,"in_reply_to":"baada198_bc54ed22","updated":"2014-08-07 14:56:42.000000000","message":"Done","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":9623,"name":"Paul Ward","email":"wpward@linux.vnet.ibm.com","username":"wpward"},"change_message_id":"55495582983dfc7206d7addbb58f0440b3a1bf3e","unresolved":false,"context_lines":[{"line_number":4544,"context_line":"                if port_id is None:"},{"line_number":4545,"context_line":"                    tmp_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4546,"context_line":"                    LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4547,"context_line":"                                 \"the port %(port_id)s\"),"},{"line_number":4548,"context_line":"                                 {\u0027port_id\u0027: tmp_id},"},{"line_number":4549,"context_line":"                                 instance\u003dinstance)"},{"line_number":4550,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_fcb665fb","line":4547,"updated":"2014-08-07 13:42:33.000000000","message":"\"try to deallocate the port\" sounds like a command rather than a statement.  I think it\u0027d be clearer to say \"attempting to deallocate port\"","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"8351863363c4f7fd2890ec10114f4414eecc975a","unresolved":false,"context_lines":[{"line_number":4544,"context_line":"                if port_id is None:"},{"line_number":4545,"context_line":"                    tmp_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4546,"context_line":"                    LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4547,"context_line":"                                 \"the port %(port_id)s\"),"},{"line_number":4548,"context_line":"                                 {\u0027port_id\u0027: tmp_id},"},{"line_number":4549,"context_line":"                                 instance\u003dinstance)"},{"line_number":4550,"context_line":"                    try:"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_bfa93f7e","line":4547,"in_reply_to":"baada198_fcb665fb","updated":"2014-08-07 14:56:42.000000000","message":"Done","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"8b4cc2947f3f8d3cb185cd241eb4ce2cd483fda1","unresolved":false,"context_lines":[{"line_number":4522,"context_line":"        requested_networks \u003d [(network_id, requested_ip, port_id)]"},{"line_number":4523,"context_line":"        if not network_id and not port_id:"},{"line_number":4524,"context_line":"            requested_networks \u003d None"},{"line_number":4525,"context_line":"        self.network_api.validate_networks(context, requested_networks, 1)"},{"line_number":4526,"context_line":"        network_info \u003d self.network_api.allocate_port_for_instance("},{"line_number":4527,"context_line":"            context, instance, port_id, network_id, requested_ip)"},{"line_number":4528,"context_line":"        image_ref \u003d instance.get(\u0027image_ref\u0027)"}],"source_content_type":"text/x-python","patch_set":20,"id":"baada198_b3acd637","line":4525,"updated":"2014-08-09 13:51:12.000000000","message":"Is it worth making the call to validate_networks if requested_networks \u003d None?  Maybe you should change L4523-4525 to:\n\n\nif not network_id and not port_id:\n    requested_networks \u003d None\nelse:\n    self.network_api.validate_networks(context, requested_networks, 1)","commit_id":"83b4b4f0725e4b079f38f0a91235f3490c334402"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"2583256c0bc3d42d342c63a8f4d419ab1881b20f","unresolved":false,"context_lines":[{"line_number":4522,"context_line":"        requested_networks \u003d [(network_id, requested_ip, port_id)]"},{"line_number":4523,"context_line":"        if not network_id and not port_id:"},{"line_number":4524,"context_line":"            requested_networks \u003d None"},{"line_number":4525,"context_line":"        self.network_api.validate_networks(context, requested_networks, 1)"},{"line_number":4526,"context_line":"        network_info \u003d self.network_api.allocate_port_for_instance("},{"line_number":4527,"context_line":"            context, instance, port_id, network_id, requested_ip)"},{"line_number":4528,"context_line":"        image_ref \u003d instance.get(\u0027image_ref\u0027)"}],"source_content_type":"text/x-python","patch_set":20,"id":"baada198_3cd40be9","line":4525,"in_reply_to":"baada198_b3acd637","updated":"2014-08-11 02:22:25.000000000","message":"yes, I need to call validate_networks even requested_networks is None.\nyou can see the origin code , validate_networks will raise networkambigious error is len(network_info) \u003e 1 , this will be handled by API layer","commit_id":"83b4b4f0725e4b079f38f0a91235f3490c334402"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"b20b68263e85e39affe05b6f72a7e237609637d1","unresolved":false,"context_lines":[{"line_number":4536,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4537,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4538,"context_line":"                             \"the port %(port_id)s\"),"},{"line_number":4539,"context_line":"                             {\u0027port_id\u0027: port_id},"},{"line_number":4540,"context_line":"                             instance\u003dinstance)"},{"line_number":4541,"context_line":"                try:"},{"line_number":4542,"context_line":"                    self.network_api.deallocate_port_for_instance("}],"source_content_type":"text/x-python","patch_set":20,"id":"baada198_008fd52e","line":4539,"updated":"2014-08-11 08:08:57.000000000","message":"in general if there is one param you do not need a dictionary","commit_id":"83b4b4f0725e4b079f38f0a91235f3490c334402"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"5461e58aa8772fc593fb1f52994149f33cd2e93a","unresolved":false,"context_lines":[{"line_number":4536,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4537,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4538,"context_line":"                             \"the port %(port_id)s\"),"},{"line_number":4539,"context_line":"                             {\u0027port_id\u0027: port_id},"},{"line_number":4540,"context_line":"                             instance\u003dinstance)"},{"line_number":4541,"context_line":"                try:"},{"line_number":4542,"context_line":"                    self.network_api.deallocate_port_for_instance("}],"source_content_type":"text/x-python","patch_set":20,"id":"baada198_64db3a7d","line":4539,"in_reply_to":"baada198_008fd52e","updated":"2014-08-11 12:57:55.000000000","message":"Done","commit_id":"83b4b4f0725e4b079f38f0a91235f3490c334402"},{"author":{"_account_id":5292,"name":"Christopher Yeoh","email":"cbkyeoh@gmail.com","username":"cyeoh-0"},"change_message_id":"7c52a81de67d3c2aa931044d3a82ba0fa64ed555","unresolved":false,"context_lines":[{"line_number":4524,"context_line":"        if len(network_info) !\u003d 1:"},{"line_number":4525,"context_line":"            LOG.error(_(\u0027allocate_port_for_instance returned %(ports)s ports\u0027)"},{"line_number":4526,"context_line":"                      % dict(ports\u003dlen(network_info)))"},{"line_number":4527,"context_line":"            raise exception.InterfaceAttachFailed(instance\u003dinstance)"},{"line_number":4528,"context_line":"        image_ref \u003d instance.get(\u0027image_ref\u0027)"},{"line_number":4529,"context_line":"        image_meta \u003d compute_utils.get_image_metadata("},{"line_number":4530,"context_line":"            context, self.image_api, image_ref, instance)"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_0f70bdd8","side":"PARENT","line":4527,"updated":"2014-08-12 04:21:06.000000000","message":"I don\u0027t think you can just delete this bit. Isn\u0027t it still possible that len(network_info) \u003e 1 and the excess ports won\u0027t be cleaned up?","commit_id":"ad7436585b651197f870c90af7d52ce7050f5b08"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"ac50360faa107e572e3c9e1c2319e1e2a9d776fc","unresolved":false,"context_lines":[{"line_number":4524,"context_line":"        if len(network_info) !\u003d 1:"},{"line_number":4525,"context_line":"            LOG.error(_(\u0027allocate_port_for_instance returned %(ports)s ports\u0027)"},{"line_number":4526,"context_line":"                      % dict(ports\u003dlen(network_info)))"},{"line_number":4527,"context_line":"            raise exception.InterfaceAttachFailed(instance\u003dinstance)"},{"line_number":4528,"context_line":"        image_ref \u003d instance.get(\u0027image_ref\u0027)"},{"line_number":4529,"context_line":"        image_meta \u003d compute_utils.get_image_metadata("},{"line_number":4530,"context_line":"            context, self.image_api, image_ref, instance)"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_20f10e96","side":"PARENT","line":4527,"in_reply_to":"baada198_0f70bdd8","updated":"2014-08-12 06:15:10.000000000","message":"validate_networks will do the validation , if len(network_info) \u003e 1 , it will raise NetworkAmbiguous error, which will be handled by API layer, and the port is not allocated ether, so we don\u0027t need to clean it up.\n\nps. validate_networks will not allocate port actually.","commit_id":"ad7436585b651197f870c90af7d52ce7050f5b08"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"be1c6568b91b82c94d74f83f8c547dc90b840ad7","unresolved":false,"context_lines":[{"line_number":4533,"context_line":"        \"\"\"Use hotplug to add an network adapter to an instance.\"\"\""},{"line_number":4534,"context_line":"        network_info \u003d self.network_api.allocate_port_for_instance("},{"line_number":4535,"context_line":"            context, instance, port_id, network_id, requested_ip)"},{"line_number":4536,"context_line":"        if len(network_info) !\u003d 1:"},{"line_number":4537,"context_line":"            LOG.error(_(\u0027allocate_port_for_instance returned %(ports)s ports\u0027)"},{"line_number":4538,"context_line":"                      % dict(ports\u003dlen(network_info)))"},{"line_number":4539,"context_line":"            raise exception.InterfaceAttachFailed("}],"source_content_type":"text/x-python","patch_set":25,"id":"1abeadc6_94c5795d","side":"PARENT","line":4536,"updated":"2014-08-18 18:38:18.000000000","message":"I think this should be in a separate change for bug 1346108, it seems related but it\u0027s removing a check that seems necessary, i.e. assume you get 2 ports and driver.attach_interface does not raise an exception, then you\u0027ve got 2 ports but you\u0027re only attached to 1, that\u0027s another bug right?\n\nShouldn\u0027t this feed into the InterfaceAttachFailed exception handling below and actually loop over network_info, deallocating ports as you go?\n\nThe NetworkAmbiguous checks in the neutronv2 API\u0027s \"allocate_for_instance\" method seem similar but not entirely the same conditions (honestly the logic around that is confusing enough to make me worry about removing this check in the compute layer).","commit_id":"c2ee61e595842513ce43eca31e878dc15038a972"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"2e14810c2f572b40788e31894062776eac4724f1","unresolved":false,"context_lines":[{"line_number":4533,"context_line":"        \"\"\"Use hotplug to add an network adapter to an instance.\"\"\""},{"line_number":4534,"context_line":"        network_info \u003d self.network_api.allocate_port_for_instance("},{"line_number":4535,"context_line":"            context, instance, port_id, network_id, requested_ip)"},{"line_number":4536,"context_line":"        if len(network_info) !\u003d 1:"},{"line_number":4537,"context_line":"            LOG.error(_(\u0027allocate_port_for_instance returned %(ports)s ports\u0027)"},{"line_number":4538,"context_line":"                      % dict(ports\u003dlen(network_info)))"},{"line_number":4539,"context_line":"            raise exception.InterfaceAttachFailed("}],"source_content_type":"text/x-python","patch_set":25,"id":"1abeadc6_c631f9ed","side":"PARENT","line":4536,"in_reply_to":"1abeadc6_8b6f0a92","updated":"2014-08-19 02:56:40.000000000","message":"I\u0027m not sure we can remove this. Because there is case len(network_info) \u003d\u003d 0, please reference https://github.com/openstack/nova/blob/master/nova/network/neutronv2/api.py#L303","commit_id":"c2ee61e595842513ce43eca31e878dc15038a972"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"c8cf98979879a0ad9c774e64e35c0d11d0954fa6","unresolved":false,"context_lines":[{"line_number":4533,"context_line":"        \"\"\"Use hotplug to add an network adapter to an instance.\"\"\""},{"line_number":4534,"context_line":"        network_info \u003d self.network_api.allocate_port_for_instance("},{"line_number":4535,"context_line":"            context, instance, port_id, network_id, requested_ip)"},{"line_number":4536,"context_line":"        if len(network_info) !\u003d 1:"},{"line_number":4537,"context_line":"            LOG.error(_(\u0027allocate_port_for_instance returned %(ports)s ports\u0027)"},{"line_number":4538,"context_line":"                      % dict(ports\u003dlen(network_info)))"},{"line_number":4539,"context_line":"            raise exception.InterfaceAttachFailed("}],"source_content_type":"text/x-python","patch_set":25,"id":"1abeadc6_8b6f0a92","side":"PARENT","line":4536,"in_reply_to":"1abeadc6_94c5795d","updated":"2014-08-19 02:23:25.000000000","message":"actually , if we call attach_interface without specify network id or port id. we may get multiple network. this will trigger exception in allocate_for_instance , and will be handled be nova network api.\n\nyou wont\u0027 get 2 ports if you specify port id or network id. only 1 port will be returned.\n\nno , I am not going to deallocate ports in a loop since multiple ports are not happening in allocate_for_instance","commit_id":"c2ee61e595842513ce43eca31e878dc15038a972"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"be1c6568b91b82c94d74f83f8c547dc90b840ad7","unresolved":false,"context_lines":[{"line_number":4544,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4545,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4546,"context_line":"                             \"the port %(port_id)s\"),"},{"line_number":4547,"context_line":"                             {\u0027port_id\u0027: port_id},"},{"line_number":4548,"context_line":"                             instance\u003dinstance)"},{"line_number":4549,"context_line":"                try:"},{"line_number":4550,"context_line":"                    self.network_api.deallocate_port_for_instance("}],"source_content_type":"text/x-python","patch_set":25,"id":"1abeadc6_395bbe55","line":4547,"updated":"2014-08-18 18:38:18.000000000","message":"This is in the wrong spot, it should be interpolated with the _LW message object, but this is passing it as an arg to LOG.warn, which is wrong (plus it\u0027s a dict so you\u0027re trying to use named interpolation incorrectly).\n\nSee:\n\nhttp://docs.openstack.org/developer/oslo.i18n/guidelines.html#adding-variables-to-translated-messages\n\nSo you could make this positional, or do this instead:\n\nLOG.warn(_LW(\"attach interface failed, try to deallocate the port %(port_id)s\") % {\u0027port_id\u0027: port_id}, instance\u003dinstance)\n\nAlso, the instance kwarg should be aligned with the _LW message above.","commit_id":"da609d016634a3df610b1dbfc68e2c92f5a60fcb"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"c8cf98979879a0ad9c774e64e35c0d11d0954fa6","unresolved":false,"context_lines":[{"line_number":4544,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4545,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4546,"context_line":"                             \"the port %(port_id)s\"),"},{"line_number":4547,"context_line":"                             {\u0027port_id\u0027: port_id},"},{"line_number":4548,"context_line":"                             instance\u003dinstance)"},{"line_number":4549,"context_line":"                try:"},{"line_number":4550,"context_line":"                    self.network_api.deallocate_port_for_instance("}],"source_content_type":"text/x-python","patch_set":25,"id":"1abeadc6_cb69028d","line":4547,"in_reply_to":"1abeadc6_395bbe55","updated":"2014-08-19 02:23:25.000000000","message":"Done","commit_id":"da609d016634a3df610b1dbfc68e2c92f5a60fcb"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"2e14810c2f572b40788e31894062776eac4724f1","unresolved":false,"context_lines":[{"line_number":4544,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4545,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4546,"context_line":"                             \"the port %(port_id)s\"),"},{"line_number":4547,"context_line":"                             {\u0027port_id\u0027: port_id},"},{"line_number":4548,"context_line":"                             instance\u003dinstance)"},{"line_number":4549,"context_line":"                try:"},{"line_number":4550,"context_line":"                    self.network_api.deallocate_port_for_instance("}],"source_content_type":"text/x-python","patch_set":25,"id":"1abeadc6_c648195e","line":4547,"in_reply_to":"1abeadc6_cb69028d","updated":"2014-08-19 02:56:40.000000000","message":"I guess it should follow this rule http://docs.openstack.org/developer/oslo.i18n/guidelines.html#adding-variables-to-log-messages\n\nIt should be:\nLOG.warn(_LW(\"attach interface failed, try to deallocate the port %s\"), port_id, instance\u003dinstance)","commit_id":"da609d016634a3df610b1dbfc68e2c92f5a60fcb"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"be1c6568b91b82c94d74f83f8c547dc90b840ad7","unresolved":false,"context_lines":[{"line_number":4550,"context_line":"                    self.network_api.deallocate_port_for_instance("},{"line_number":4551,"context_line":"                        context, instance, port_id)"},{"line_number":4552,"context_line":"                except Exception:"},{"line_number":4553,"context_line":"                    LOG.exception(_LE(\"deallocate port %(port_id)s \""},{"line_number":4554,"context_line":"                                      \"failed\"), {\u0027port_id\u0027: port_id},"},{"line_number":4555,"context_line":"                                      instance\u003dinstance)"},{"line_number":4556,"context_line":"        return network_info[0]"}],"source_content_type":"text/x-python","patch_set":25,"id":"1abeadc6_9448195d","line":4553,"updated":"2014-08-18 18:38:18.000000000","message":"Don\u0027t use LOG.exception, we don\u0027t really need a stacktrace for this do we?  But we should have the error in the message, so maybe LOG.warn since that\u0027s what we\u0027re doing above when the attach fails and then add the error to the message.","commit_id":"da609d016634a3df610b1dbfc68e2c92f5a60fcb"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"c8cf98979879a0ad9c774e64e35c0d11d0954fa6","unresolved":false,"context_lines":[{"line_number":4550,"context_line":"                    self.network_api.deallocate_port_for_instance("},{"line_number":4551,"context_line":"                        context, instance, port_id)"},{"line_number":4552,"context_line":"                except Exception:"},{"line_number":4553,"context_line":"                    LOG.exception(_LE(\"deallocate port %(port_id)s \""},{"line_number":4554,"context_line":"                                      \"failed\"), {\u0027port_id\u0027: port_id},"},{"line_number":4555,"context_line":"                                      instance\u003dinstance)"},{"line_number":4556,"context_line":"        return network_info[0]"}],"source_content_type":"text/x-python","patch_set":25,"id":"1abeadc6_2b10b600","line":4553,"in_reply_to":"1abeadc6_9448195d","updated":"2014-08-19 02:23:25.000000000","message":"hmm.. okay.\nbut seems other has difference options on this one.","commit_id":"da609d016634a3df610b1dbfc68e2c92f5a60fcb"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"be1c6568b91b82c94d74f83f8c547dc90b840ad7","unresolved":false,"context_lines":[{"line_number":4551,"context_line":"                        context, instance, port_id)"},{"line_number":4552,"context_line":"                except Exception:"},{"line_number":4553,"context_line":"                    LOG.exception(_LE(\"deallocate port %(port_id)s \""},{"line_number":4554,"context_line":"                                      \"failed\"), {\u0027port_id\u0027: port_id},"},{"line_number":4555,"context_line":"                                      instance\u003dinstance)"},{"line_number":4556,"context_line":"        return network_info[0]"},{"line_number":4557,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"1abeadc6_f4094d2d","line":4554,"updated":"2014-08-18 18:38:18.000000000","message":"The _LE usage with the name interpolation is wrong, same as above, see:\n\nhttp://docs.openstack.org/developer/oslo.i18n/guidelines.html#adding-variables-to-translated-messages","commit_id":"da609d016634a3df610b1dbfc68e2c92f5a60fcb"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"be1c6568b91b82c94d74f83f8c547dc90b840ad7","unresolved":false,"context_lines":[{"line_number":4552,"context_line":"                except Exception:"},{"line_number":4553,"context_line":"                    LOG.exception(_LE(\"deallocate port %(port_id)s \""},{"line_number":4554,"context_line":"                                      \"failed\"), {\u0027port_id\u0027: port_id},"},{"line_number":4555,"context_line":"                                      instance\u003dinstance)"},{"line_number":4556,"context_line":"        return network_info[0]"},{"line_number":4557,"context_line":""},{"line_number":4558,"context_line":"    @object_compat"}],"source_content_type":"text/x-python","patch_set":25,"id":"1abeadc6_592f9aa1","line":4555,"updated":"2014-08-18 18:38:18.000000000","message":"The instance kwarg should be aligned with _LE above.","commit_id":"da609d016634a3df610b1dbfc68e2c92f5a60fcb"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"c8cf98979879a0ad9c774e64e35c0d11d0954fa6","unresolved":false,"context_lines":[{"line_number":4552,"context_line":"                except Exception:"},{"line_number":4553,"context_line":"                    LOG.exception(_LE(\"deallocate port %(port_id)s \""},{"line_number":4554,"context_line":"                                      \"failed\"), {\u0027port_id\u0027: port_id},"},{"line_number":4555,"context_line":"                                      instance\u003dinstance)"},{"line_number":4556,"context_line":"        return network_info[0]"},{"line_number":4557,"context_line":""},{"line_number":4558,"context_line":"    @object_compat"}],"source_content_type":"text/x-python","patch_set":25,"id":"1abeadc6_ab03e6b6","line":4555,"in_reply_to":"1abeadc6_592f9aa1","updated":"2014-08-19 02:23:25.000000000","message":"Done","commit_id":"da609d016634a3df610b1dbfc68e2c92f5a60fcb"},{"author":{"_account_id":8574,"name":"Qin Zhao","username":"zhaoqin","inactive":true},"change_message_id":"02a9cceb8c9abc0ca9b88d5a6a48031a92cf4dc6","unresolved":false,"context_lines":[{"line_number":4544,"context_line":""},{"line_number":4545,"context_line":"        try:"},{"line_number":4546,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4547,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4548,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4549,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4550,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""}],"source_content_type":"text/x-python","patch_set":27,"id":"9aa7fdbe_f0fdbe3c","line":4547,"updated":"2014-10-21 15:11:07.000000000","message":"Do we also need to catch other exception? InterfaceAttachFailed seems not be the only type of exception that might be thrown during attach_interface(), but deallocating port seems to be a required work for any type of exception handling. Is that correct?","commit_id":"9d52b6a99096e6090ee423af775a02f19f9ec5c2"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"16d6c9cf8dbceeda169511b9953c0fe1e4ebf852","unresolved":false,"context_lines":[{"line_number":4544,"context_line":""},{"line_number":4545,"context_line":"        try:"},{"line_number":4546,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4547,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4548,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4549,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4550,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""}],"source_content_type":"text/x-python","patch_set":27,"id":"9aa7fdbe_2468911c","line":4547,"in_reply_to":"9aa7fdbe_8e607c02","updated":"2014-10-24 08:42:18.000000000","message":"yes, correct, thanks Qin\n\nI can also find there may raise NovaException and InstanceNotFound in libvirt driver.\nwe need to handle them too.","commit_id":"9d52b6a99096e6090ee423af775a02f19f9ec5c2"},{"author":{"_account_id":8574,"name":"Qin Zhao","username":"zhaoqin","inactive":true},"change_message_id":"4d09a724ad356925632468698b6835cd13f26d48","unresolved":false,"context_lines":[{"line_number":4544,"context_line":""},{"line_number":4545,"context_line":"        try:"},{"line_number":4546,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4547,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4548,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4549,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4550,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""}],"source_content_type":"text/x-python","patch_set":27,"id":"9aa7fdbe_8e607c02","line":4547,"in_reply_to":"9aa7fdbe_c2545d56","updated":"2014-10-24 07:15:28.000000000","message":"For example, in attach_interface() of  vmware driver, the code need to get vm object reference from vcenter. It may throw exception, although its possibility is not high. Do you think it is necessary to consider that?","commit_id":"9d52b6a99096e6090ee423af775a02f19f9ec5c2"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"761474d5390255dddc83441ddc785778222002f0","unresolved":false,"context_lines":[{"line_number":4544,"context_line":""},{"line_number":4545,"context_line":"        try:"},{"line_number":4546,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4547,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4548,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4549,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4550,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""}],"source_content_type":"text/x-python","patch_set":27,"id":"9aa7fdbe_c2545d56","line":4547,"in_reply_to":"9aa7fdbe_f0fdbe3c","updated":"2014-10-22 05:24:27.000000000","message":"hi Qing, thanks for the comments.\nyes we need to handle the exception to deallocate the ports.\nI checked the driver code(libvirt/vmwareapi)\nI don\u0027t find any other exception except InterfaceAttachFailed will be raised from attach_interface function.\n\nwhat do you mean by any other types exception?","commit_id":"9d52b6a99096e6090ee423af775a02f19f9ec5c2"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"cc104ffd135e94d484b2ff0e2d924f3a2203ad46","unresolved":false,"context_lines":[{"line_number":4545,"context_line":"        try:"},{"line_number":4546,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4547,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4548,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4549,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4550,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4551,"context_line":"                             \"the port %(port_id)s\") % {\u0027port_id\u0027: port_id},"}],"source_content_type":"text/x-python","patch_set":27,"id":"1abeadc6_c9b2a505","line":4548,"updated":"2014-08-20 13:53:12.000000000","message":"If the user passes in a port (indicating it was created ahead of time in neutron), we would deallocate it.  The original change set I proposed didn\u0027t have that behavior.  \n\nIs the consensus that we should deallocate a pre-allocated port?","commit_id":"9d52b6a99096e6090ee423af775a02f19f9ec5c2"},{"author":{"_account_id":5292,"name":"Christopher Yeoh","email":"cbkyeoh@gmail.com","username":"cyeoh-0"},"change_message_id":"761cb4ec9546879b584837c79375f2e639725393","unresolved":false,"context_lines":[{"line_number":4545,"context_line":"        try:"},{"line_number":4546,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4547,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4548,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4549,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4550,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4551,"context_line":"                             \"the port %(port_id)s\") % {\u0027port_id\u0027: port_id},"}],"source_content_type":"text/x-python","patch_set":27,"id":"fa98f980_0ee04a95","line":4548,"in_reply_to":"1abeadc6_2c7b2729","updated":"2014-09-10 03:22:48.000000000","message":"I think this is ok, but Aaron\u0027s patch needs to merge before yours does so perhaps rebase on top of his (though it looks like it still needs a bit of work)","commit_id":"9d52b6a99096e6090ee423af775a02f19f9ec5c2"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"53c29cc3120e2f53a011b4436bebb5ec1b45f44b","unresolved":false,"context_lines":[{"line_number":4545,"context_line":"        try:"},{"line_number":4546,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4547,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4548,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4549,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4550,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4551,"context_line":"                             \"the port %(port_id)s\") % {\u0027port_id\u0027: port_id},"}],"source_content_type":"text/x-python","patch_set":27,"id":"1abeadc6_2c7b2729","line":4548,"in_reply_to":"1abeadc6_c9b2a505","updated":"2014-08-20 14:20:14.000000000","message":"As I comment in the commit message, it is partial fix #1338551.\nI don\u0027t handle the situation if the port is pre-allocated because there is a patch handle this situation.\n\nplease see this patch https://review.openstack.org/#/c/77043/\n\nand I have a comments on that.\n\nif Aaron cover deallocate_port_for_instance. then my patch will not deallocate the pre-fix port from neutron.\n\nthanks. Eli.","commit_id":"9d52b6a99096e6090ee423af775a02f19f9ec5c2"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"8ac4744af41d65cfbdc18dc13890c5ac17fd6721","unresolved":false,"context_lines":[{"line_number":4545,"context_line":"        try:"},{"line_number":4546,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4547,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4548,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4549,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4550,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4551,"context_line":"                             \"the port %(port_id)s\") % {\u0027port_id\u0027: port_id},"}],"source_content_type":"text/x-python","patch_set":27,"id":"9aa7fdbe_659f3bb5","line":4548,"in_reply_to":"9aa7fdbe_10ba4223","updated":"2014-10-22 06:12:25.000000000","message":"yeah, I am keeping a eye on it. thanks.","commit_id":"9d52b6a99096e6090ee423af775a02f19f9ec5c2"},{"author":{"_account_id":9623,"name":"Paul Ward","email":"wpward@linux.vnet.ibm.com","username":"wpward"},"change_message_id":"06068f9b9cfeaa8bc098b21e107196c60ea5ad32","unresolved":false,"context_lines":[{"line_number":4545,"context_line":"        try:"},{"line_number":4546,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4547,"context_line":"        except exception.InterfaceAttachFailed:"},{"line_number":4548,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4549,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4550,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4551,"context_line":"                             \"the port %(port_id)s\") % {\u0027port_id\u0027: port_id},"}],"source_content_type":"text/x-python","patch_set":27,"id":"9aa7fdbe_10ba4223","line":4548,"in_reply_to":"fa98f980_0ee04a95","updated":"2014-10-21 14:52:15.000000000","message":"FYI - Aaron\u0027s patch was abandoned in favor of this one: https://review.openstack.org/#/c/126309/","commit_id":"9d52b6a99096e6090ee423af775a02f19f9ec5c2"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"b6e87ce59713f3911d45b8ce03654df1f57b3ac9","unresolved":false,"context_lines":[{"line_number":4554,"context_line":"                    self.network_api.deallocate_port_for_instance("},{"line_number":4555,"context_line":"                        context, instance, port_id)"},{"line_number":4556,"context_line":"                except Exception:"},{"line_number":4557,"context_line":"                    LOG.warn(_LW(\"deallocate port %(port_id)s failed\")"},{"line_number":4558,"context_line":"                                 % {\u0027port_id\u0027: port_id}, instance\u003dinstance)"},{"line_number":4559,"context_line":"        return network_info[0]"},{"line_number":4560,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"1abeadc6_2474bccd","line":4557,"updated":"2014-08-29 18:36:08.000000000","message":"minor nit. please use a , instead of the %","commit_id":"9d52b6a99096e6090ee423af775a02f19f9ec5c2"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"8ac4744af41d65cfbdc18dc13890c5ac17fd6721","unresolved":false,"context_lines":[{"line_number":4554,"context_line":"                    self.network_api.deallocate_port_for_instance("},{"line_number":4555,"context_line":"                        context, instance, port_id)"},{"line_number":4556,"context_line":"                except Exception:"},{"line_number":4557,"context_line":"                    LOG.warn(_LW(\"deallocate port %(port_id)s failed\")"},{"line_number":4558,"context_line":"                                 % {\u0027port_id\u0027: port_id}, instance\u003dinstance)"},{"line_number":4559,"context_line":"        return network_info[0]"},{"line_number":4560,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"9aa7fdbe_255e1313","line":4557,"in_reply_to":"1abeadc6_2474bccd","updated":"2014-10-22 06:12:25.000000000","message":"hi garyk, thanks, I will update it in next PS","commit_id":"9d52b6a99096e6090ee423af775a02f19f9ec5c2"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ee2b07c4ca1850aa59710566519c168891f3ae7a","unresolved":false,"context_lines":[{"line_number":4807,"context_line":"                    self.network_api.deallocate_port_for_instance("},{"line_number":4808,"context_line":"                        context, instance, port_id)"},{"line_number":4809,"context_line":"                except Exception:"},{"line_number":4810,"context_line":"                    LOG.warn(_LW(\"deallocate port %(port_id)s failed\"),"},{"line_number":4811,"context_line":"                                 {\u0027port_id\u0027: port_id}, instance\u003dinstance)"},{"line_number":4812,"context_line":"        return network_info[0]"},{"line_number":4813,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"5a890539_f40560c2","line":4810,"updated":"2014-12-02 21:14:40.000000000","message":"This really should be LOG.exception() since if we got here, there\u0027s some exception somewhere in the network API\u0027s handling of the deallocate_port_for_instance() call, and we will lose that traceback information unless LOG.exception is used, becasue the save_and_reraise_exception() context manager will be raising the exception from attach_interface().","commit_id":"44ec88e1bc96750d5728f55f6ff4bb520cfe65f1"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"099944462af8e295beb5976ef6c44a2931c7c6b6","unresolved":false,"context_lines":[{"line_number":4807,"context_line":"                    self.network_api.deallocate_port_for_instance("},{"line_number":4808,"context_line":"                        context, instance, port_id)"},{"line_number":4809,"context_line":"                except Exception:"},{"line_number":4810,"context_line":"                    LOG.warn(_LW(\"deallocate port %(port_id)s failed\"),"},{"line_number":4811,"context_line":"                                 {\u0027port_id\u0027: port_id}, instance\u003dinstance)"},{"line_number":4812,"context_line":"        return network_info[0]"},{"line_number":4813,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"5a890539_c8735c14","line":4810,"in_reply_to":"5a890539_f40560c2","updated":"2014-12-03 03:12:23.000000000","message":"Done","commit_id":"44ec88e1bc96750d5728f55f6ff4bb520cfe65f1"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"97dcfdc6719b6220713008662a6fef78a3c1b493","unresolved":false,"context_lines":[{"line_number":4868,"context_line":"                exception.NotFound):"},{"line_number":4869,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4870,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4871,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4872,"context_line":"                             \"the port %(port_id)s\"), {\u0027port_id\u0027: port_id},"},{"line_number":4873,"context_line":"                         instance\u003dinstance)"},{"line_number":4874,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":33,"id":"fa81d914_8a835203","line":4871,"updated":"2015-01-29 09:35:13.000000000","message":"I will submit a patch to drop N311\nfor now ,suggest you to update it to warning to pass test\n\nhttps://review.openstack.org/#/c/145506/\nhttps://review.openstack.org/#/c/135888/","commit_id":"249dad6f084e40eb627848dad6c1fcc8166fbc3b"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"e9719f2ca4922d8218407570d48b882bf2b6e9dd","unresolved":false,"context_lines":[{"line_number":4868,"context_line":"                exception.NotFound):"},{"line_number":4869,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4870,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4871,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4872,"context_line":"                             \"the port %(port_id)s\"), {\u0027port_id\u0027: port_id},"},{"line_number":4873,"context_line":"                         instance\u003dinstance)"},{"line_number":4874,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":33,"id":"fa81d914_ee9d970b","line":4871,"in_reply_to":"fa81d914_8a835203","updated":"2015-01-30 01:54:29.000000000","message":"sure, will update it as your suggestion, thanks.","commit_id":"249dad6f084e40eb627848dad6c1fcc8166fbc3b"},{"author":{"_account_id":9623,"name":"Paul Ward","email":"wpward@linux.vnet.ibm.com","username":"wpward"},"change_message_id":"709f99c4382470ca996b285e4f39e5a0fc0b9934","unresolved":false,"context_lines":[{"line_number":4875,"context_line":"                exception.NovaException,"},{"line_number":4876,"context_line":"                exception.NotFound):"},{"line_number":4877,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4878,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4879,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4880,"context_line":"                             \"the port %(port_id)s\"), {\u0027port_id\u0027: port_id},"},{"line_number":4881,"context_line":"                         instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":35,"id":"da86d52c_09769cd8","line":4878,"updated":"2015-02-16 15:02:57.000000000","message":"Back in patch set 17 there was a check here to see if the port was passed in vs created by the allocate above.  If it was passed in, we did NOT clean up the port as we don\u0027t want to delete a port someone created prior to the attach.... is there a reason we no longer check this?","commit_id":"b7dea9f8241acba3143016b029aec90d96c1a75d"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"c69e2bcbe1c3efb4e333fc5210c92868e6855726","unresolved":false,"context_lines":[{"line_number":4875,"context_line":"                exception.NovaException,"},{"line_number":4876,"context_line":"                exception.NotFound):"},{"line_number":4877,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4878,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4879,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4880,"context_line":"                             \"the port %(port_id)s\"), {\u0027port_id\u0027: port_id},"},{"line_number":4881,"context_line":"                         instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":35,"id":"da86d52c_30b492f1","line":4878,"in_reply_to":"da86d52c_09769cd8","updated":"2015-02-17 01:05:04.000000000","message":"hi paul, because the check has done in https://review.openstack.org/#/c/126309/19 , so I depend on that one.","commit_id":"b7dea9f8241acba3143016b029aec90d96c1a75d"},{"author":{"_account_id":9623,"name":"Paul Ward","email":"wpward@linux.vnet.ibm.com","username":"wpward"},"change_message_id":"578b6b18f0aba69669efe165dc9984ee1c30b670","unresolved":false,"context_lines":[{"line_number":4875,"context_line":"                exception.NovaException,"},{"line_number":4876,"context_line":"                exception.NotFound):"},{"line_number":4877,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4878,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4879,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4880,"context_line":"                             \"the port %(port_id)s\"), {\u0027port_id\u0027: port_id},"},{"line_number":4881,"context_line":"                         instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":35,"id":"da86d52c_932b6d32","line":4878,"in_reply_to":"da86d52c_30b492f1","updated":"2015-02-17 17:39:10.000000000","message":"Ah nice, I like that change.  I\u0027ll remove my -1. :)","commit_id":"b7dea9f8241acba3143016b029aec90d96c1a75d"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3fff3a9dbd1b4c96f975b4d2cb3bb2048ce24fe9","unresolved":false,"context_lines":[{"line_number":4888,"context_line":"        try:"},{"line_number":4889,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4890,"context_line":"        except (exception.InterfaceAttachFailed,"},{"line_number":4891,"context_line":"                exception.NovaException,"},{"line_number":4892,"context_line":"                exception.NotFound):"},{"line_number":4893,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4894,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_3d68d9b9","line":4891,"updated":"2015-03-12 14:00:06.000000000","message":"If you\u0027re going to catch NovaException here, you don\u0027t need to explicitly handle InterfaceAttachFailed or NotFound, since those extend NovaException already.  So do we want to be more specific about what we handle here or more generic?\n\nThe libvirt vif driver plug() method can also raise VirtualInterfacePlugException.  We could also get ProcessExecutionError or OSError because the libvirt vif driver is running local commands.  The vmware driver can raise InstanceNotFound if it can\u0027t find the vm_ref.\n\nSo it\u0027s probably best to just handle NovaException here.","commit_id":"71ee54e6264f8961f1f3b13c361e905137822e59"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3fff3a9dbd1b4c96f975b4d2cb3bb2048ce24fe9","unresolved":false,"context_lines":[{"line_number":4890,"context_line":"        except (exception.InterfaceAttachFailed,"},{"line_number":4891,"context_line":"                exception.NovaException,"},{"line_number":4892,"context_line":"                exception.NotFound):"},{"line_number":4893,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4894,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4895,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4896,"context_line":"                             \"the port %(port_id)s\"), {\u0027port_id\u0027: port_id},"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_9d9aa56c","line":4893,"updated":"2015-03-12 14:00:06.000000000","message":"Given the list of exceptions I found that can come up from the libvirt and vmware drivers, it seems it\u0027d be safest to just log the original exception and raise InterfaceAttachFailed so the API extension doesn\u0027t need to handle 20 different exceptions that can happen down in the virt driver layer.  The compute manager should handle those, translate them to InterfaceAttachFailed, and the API extension could handle that (which it already does).\n\nThis may be open to debate, so would like others to chime in.","commit_id":"71ee54e6264f8961f1f3b13c361e905137822e59"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"56d2fcf939c37136a0b89dc7fc579a043a4ee7b6","unresolved":false,"context_lines":[{"line_number":4890,"context_line":"        except (exception.InterfaceAttachFailed,"},{"line_number":4891,"context_line":"                exception.NovaException,"},{"line_number":4892,"context_line":"                exception.NotFound):"},{"line_number":4893,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4894,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4895,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4896,"context_line":"                             \"the port %(port_id)s\"), {\u0027port_id\u0027: port_id},"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_c6ffeeff","line":4893,"in_reply_to":"9a80dd14_9d9aa56c","updated":"2015-03-12 14:38:29.000000000","message":"I agree with this.  I prefer to get back exceptions that match the action I\u0027m calling.  If I call attach_interface and it fails, just give me a simple exception to let me know.  Unless it\u0027s important to distinguish between them at the call site, which shouldn\u0027t often be the case.","commit_id":"71ee54e6264f8961f1f3b13c361e905137822e59"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"83f07ac04ba544c6f8fbab6d522a55a1a48ffb70","unresolved":false,"context_lines":[{"line_number":4890,"context_line":"        except (exception.InterfaceAttachFailed,"},{"line_number":4891,"context_line":"                exception.NovaException,"},{"line_number":4892,"context_line":"                exception.NotFound):"},{"line_number":4893,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4894,"context_line":"                port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4895,"context_line":"                LOG.warn(_LW(\"attach interface failed, try to deallocate \""},{"line_number":4896,"context_line":"                             \"the port %(port_id)s\"), {\u0027port_id\u0027: port_id},"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_ef8e6c2d","line":4893,"in_reply_to":"9a80dd14_c6ffeeff","updated":"2015-03-12 23:34:33.000000000","message":"thanks matt and andrew for the reviewing, will update it per your comments.","commit_id":"71ee54e6264f8961f1f3b13c361e905137822e59"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"05042609e53adae3639c9fbffbff6d6794c973f1","unresolved":false,"context_lines":[{"line_number":4898,"context_line":"            self.driver.attach_interface(instance, image_meta, network_info[0])"},{"line_number":4899,"context_line":"        except exception.NovaException as ex:"},{"line_number":4900,"context_line":"            port_id \u003d network_info[0].get(\u0027id\u0027)"},{"line_number":4901,"context_line":"            LOG.warn(_LW(\"attach interface failed , try to deallocate \""},{"line_number":4902,"context_line":"                         \"port %(port_id)s, reason: %(msg)s\"),"},{"line_number":4903,"context_line":"                     {\u0027port_id\u0027: port_id, \u0027msg\u0027: ex},"},{"line_number":4904,"context_line":"                     instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":39,"id":"9a80dd14_f08c6d82","line":4901,"updated":"2015-03-13 14:21:47.000000000","message":"s/failed ,/failed,/","commit_id":"78f7c744f675d618048968850259422338dd4955"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"05042609e53adae3639c9fbffbff6d6794c973f1","unresolved":false,"context_lines":[{"line_number":4907,"context_line":"                    context, instance, port_id)"},{"line_number":4908,"context_line":"            except Exception:"},{"line_number":4909,"context_line":"                LOG.warn(_LW(\"deallocate port %(port_id)s failed\"),"},{"line_number":4910,"context_line":"                             {\u0027port_id\u0027: port_id}, instance\u003dinstance)"},{"line_number":4911,"context_line":"            raise exception.InterfaceAttachFailed("},{"line_number":4912,"context_line":"                instance_uuid\u003dinstance.uuid)"},{"line_number":4913,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"9a80dd14_104f4135","line":4910,"updated":"2015-03-13 14:21:47.000000000","message":"nit: these should be aligned with _LW above.","commit_id":"78f7c744f675d618048968850259422338dd4955"}],"nova/network/base_api.py":[{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"ee656df3b8d582437adb0b02c7d6789eb1e8a7a4","unresolved":false,"context_lines":[{"line_number":212,"context_line":"        raise NotImplementedError()"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    def release_port(self, context, port_id):"},{"line_number":215,"context_line":"        \"\"\"Release a port from a instance.\"\"\""},{"line_number":216,"context_line":"        raise NotImplementedError()"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    def add_fixed_ip_to_instance(self, context, instance, network_id):"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_7f0fa72f","line":215,"updated":"2014-08-07 13:49:34.000000000","message":"By introducing these new methods, I worry that we introduce inconsistent behavior for nova networks.  Those are still fairly prevalent in deployments.\n\nThoughts?","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"8351863363c4f7fd2890ec10114f4414eecc975a","unresolved":false,"context_lines":[{"line_number":212,"context_line":"        raise NotImplementedError()"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    def release_port(self, context, port_id):"},{"line_number":215,"context_line":"        \"\"\"Release a port from a instance.\"\"\""},{"line_number":216,"context_line":"        raise NotImplementedError()"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    def add_fixed_ip_to_instance(self, context, instance, network_id):"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_dfe993ac","line":215,"in_reply_to":"baada198_7f0fa72f","updated":"2014-08-07 14:56:42.000000000","message":"hmm... I don\u0027t have any other good solution.\nyes, it would bring inconsistent, but this won\u0027t effect other developers to use the api from nova networks since they won\u0027t call this api in nova-network.","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"}],"nova/network/neutronv2/api.py":[{"author":{"_account_id":9623,"name":"Paul Ward","email":"wpward@linux.vnet.ibm.com","username":"wpward"},"change_message_id":"55495582983dfc7206d7addbb58f0440b3a1bf3e","unresolved":false,"context_lines":[{"line_number":485,"context_line":"                                   network_id\u003dNone, requested_ip\u003dNone):"},{"line_number":486,"context_line":"        \"\"\"Allocate a port for the instance"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"        Do not update the port to instance."},{"line_number":489,"context_line":"        \"\"\""},{"line_number":490,"context_line":"        return self.allocate_for_instance(context, instance, noupdate\u003dTrue,"},{"line_number":491,"context_line":"                requested_networks\u003d[(network_id, requested_ip, port_id)])"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_3c329d5e","line":488,"updated":"2014-08-07 13:42:33.000000000","message":"This is new behavior so I think we need to be very clear here for future coders.... perhaps:\n\n\"Do not add the port info to the instance yet.  A call to add_nw_info_to_instance should be done to add it.\"\n\nFor that matter... did you scan the code to see who else uses this method and if we\u0027re breaking them?","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"8351863363c4f7fd2890ec10114f4414eecc975a","unresolved":false,"context_lines":[{"line_number":485,"context_line":"                                   network_id\u003dNone, requested_ip\u003dNone):"},{"line_number":486,"context_line":"        \"\"\"Allocate a port for the instance"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"        Do not update the port to instance."},{"line_number":489,"context_line":"        \"\"\""},{"line_number":490,"context_line":"        return self.allocate_for_instance(context, instance, noupdate\u003dTrue,"},{"line_number":491,"context_line":"                requested_networks\u003d[(network_id, requested_ip, port_id)])"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_826422e7","line":488,"in_reply_to":"baada198_3c329d5e","updated":"2014-08-07 14:56:42.000000000","message":"sure, I searched, only attach_interface use it.","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":9623,"name":"Paul Ward","email":"wpward@linux.vnet.ibm.com","username":"wpward"},"change_message_id":"55495582983dfc7206d7addbb58f0440b3a1bf3e","unresolved":false,"context_lines":[{"line_number":511,"context_line":"        \"\"\"Return the port for the client given the port id.\"\"\""},{"line_number":512,"context_line":"        return neutronv2.get_client(context).show_port(port_id)"},{"line_number":513,"context_line":""},{"line_number":514,"context_line":"    def release_port(self, context, port_id):"},{"line_number":515,"context_line":"        \"\"\"Release a port from a instance.\"\"\""},{"line_number":516,"context_line":""},{"line_number":517,"context_line":"        port_client \u003d neutronv2.get_client(context)"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_5c0f51e0","line":514,"updated":"2014-08-07 13:42:33.000000000","message":"release_port_from_instance would probably be a more descriptive name","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":9623,"name":"Paul Ward","email":"wpward@linux.vnet.ibm.com","username":"wpward"},"change_message_id":"55495582983dfc7206d7addbb58f0440b3a1bf3e","unresolved":false,"context_lines":[{"line_number":518,"context_line":"        port_req_body \u003d {\u0027port\u0027: {\u0027device_id\u0027: \u0027\u0027}}"},{"line_number":519,"context_line":"        port_client.update_port(port_id, port_req_body)"},{"line_number":520,"context_line":""},{"line_number":521,"context_line":"    def add_nw_info(self, context, instance, nw_info):"},{"line_number":522,"context_line":"        \"\"\"Append a nw_info to an instance.\"\"\""},{"line_number":523,"context_line":"        orig_nw_info \u003d self._get_instance_nw_info(context, instance)"},{"line_number":524,"context_line":"        result \u003d network_model.NetworkInfo.hydrate(orig_nw_info + nw_info)"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_dcfa81b9","line":521,"updated":"2014-08-07 13:42:33.000000000","message":"add_nw_info_to_instance would probably be a more descriptive name","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"8351863363c4f7fd2890ec10114f4414eecc975a","unresolved":false,"context_lines":[{"line_number":518,"context_line":"        port_req_body \u003d {\u0027port\u0027: {\u0027device_id\u0027: \u0027\u0027}}"},{"line_number":519,"context_line":"        port_client.update_port(port_id, port_req_body)"},{"line_number":520,"context_line":""},{"line_number":521,"context_line":"    def add_nw_info(self, context, instance, nw_info):"},{"line_number":522,"context_line":"        \"\"\"Append a nw_info to an instance.\"\"\""},{"line_number":523,"context_line":"        orig_nw_info \u003d self._get_instance_nw_info(context, instance)"},{"line_number":524,"context_line":"        result \u003d network_model.NetworkInfo.hydrate(orig_nw_info + nw_info)"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_9fd2bbcd","line":521,"in_reply_to":"baada198_dcfa81b9","updated":"2014-08-07 14:56:42.000000000","message":"Done","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":9623,"name":"Paul Ward","email":"wpward@linux.vnet.ibm.com","username":"wpward"},"change_message_id":"55495582983dfc7206d7addbb58f0440b3a1bf3e","unresolved":false,"context_lines":[{"line_number":532,"context_line":"        \"\"\""},{"line_number":533,"context_line":"        # NOTE(geekinutah): It would be nice if use_slave had us call"},{"line_number":534,"context_line":"        #                   special APIs that pummeled slaves instead of"},{"line_number":535,"context_line":"        #                   the master. For now we just ignore this arg."},{"line_number":536,"context_line":"        result \u003d self._get_instance_nw_info(context, instance, networks,"},{"line_number":537,"context_line":"                                            port_ids)"},{"line_number":538,"context_line":"        base_api.update_instance_cache_with_nw_info(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_3cad3d85","line":535,"updated":"2014-08-07 13:42:33.000000000","message":"Add a LOG.debug() here, similar to the one in _get_instance_nw_info, so we can see when we go down one path vs the other.","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"8351863363c4f7fd2890ec10114f4414eecc975a","unresolved":false,"context_lines":[{"line_number":532,"context_line":"        \"\"\""},{"line_number":533,"context_line":"        # NOTE(geekinutah): It would be nice if use_slave had us call"},{"line_number":534,"context_line":"        #                   special APIs that pummeled slaves instead of"},{"line_number":535,"context_line":"        #                   the master. For now we just ignore this arg."},{"line_number":536,"context_line":"        result \u003d self._get_instance_nw_info(context, instance, networks,"},{"line_number":537,"context_line":"                                            port_ids)"},{"line_number":538,"context_line":"        base_api.update_instance_cache_with_nw_info(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_c25e1a13","line":535,"in_reply_to":"baada198_3cad3d85","updated":"2014-08-07 14:56:42.000000000","message":"Done","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"f1611bc90535f727a642c9724c9f7c9de2ac211d","unresolved":false,"context_lines":[{"line_number":299,"context_line":"            return network_model.NetworkInfo([])"},{"line_number":300,"context_line":"        security_groups \u003d kwargs.get(\u0027security_groups\u0027, [])"},{"line_number":301,"context_line":"        security_group_ids \u003d []"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"        if not requested_networks and not len(security_groups) \\"},{"line_number":304,"context_line":"           and len(nets) \u003e 1:"},{"line_number":305,"context_line":"            msg \u003d _(\"Multiple possible networks found, use a Network \""}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_2ae68f4a","line":302,"updated":"2014-08-09 05:33:44.000000000","message":"Like Paul mentioned in the previous patchset #17 - this seems to be a new behavior and there are multiple places where from this method gets called. There can be a possibility of multiple nets being set as a result. This will fail all the tests where nets is returned to be more than 1. Can you please re-check this validation below?","commit_id":"968604a15abc4463b6f3b9bfa2378dd8dd955bdc"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"1826b2cce08dfea9469177a2183af63fecf4588b","unresolved":false,"context_lines":[{"line_number":299,"context_line":"            return network_model.NetworkInfo([])"},{"line_number":300,"context_line":"        security_groups \u003d kwargs.get(\u0027security_groups\u0027, [])"},{"line_number":301,"context_line":"        security_group_ids \u003d []"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"        if not requested_networks and not len(security_groups) \\"},{"line_number":304,"context_line":"           and len(nets) \u003e 1:"},{"line_number":305,"context_line":"            msg \u003d _(\"Multiple possible networks found, use a Network \""}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_1886815f","line":302,"in_reply_to":"baada198_2ae68f4a","updated":"2014-08-09 08:28:41.000000000","message":"yes, it will fail many test case, I need investigate if the failed test case are wrote correctly.","commit_id":"968604a15abc4463b6f3b9bfa2378dd8dd955bdc"},{"author":{"_account_id":9775,"name":"Sudipta Biswas","email":"sbiswas7@in.ibm.com","username":"sudswas"},"change_message_id":"306f18e21921345930834e29960cd6a678c22948","unresolved":false,"context_lines":[{"line_number":300,"context_line":"        security_groups \u003d kwargs.get(\u0027security_groups\u0027, [])"},{"line_number":301,"context_line":"        security_group_ids \u003d []"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"        if not requested_networks and not len(security_groups) \\"},{"line_number":304,"context_line":"           and len(nets) \u003e 1:"},{"line_number":305,"context_line":"            msg \u003d _(\"Multiple possible networks found, use a Network \""},{"line_number":306,"context_line":"                    \"ID to be more specific.\")"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_8ac87ba8","line":303,"updated":"2014-08-09 05:40:43.000000000","message":"It\u0027s probably easier if you do this outside of this method and before calling this method from manager.py","commit_id":"968604a15abc4463b6f3b9bfa2378dd8dd955bdc"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"1826b2cce08dfea9469177a2183af63fecf4588b","unresolved":false,"context_lines":[{"line_number":300,"context_line":"        security_groups \u003d kwargs.get(\u0027security_groups\u0027, [])"},{"line_number":301,"context_line":"        security_group_ids \u003d []"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"        if not requested_networks and not len(security_groups) \\"},{"line_number":304,"context_line":"           and len(nets) \u003e 1:"},{"line_number":305,"context_line":"            msg \u003d _(\"Multiple possible networks found, use a Network \""},{"line_number":306,"context_line":"                    \"ID to be more specific.\")"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_b8d91586","line":303,"in_reply_to":"baada198_8ac87ba8","updated":"2014-08-09 08:28:41.000000000","message":"yes, actually , I prefer validate_networks to validate the requested networks. \n\nin validate_networks: \n\n            if len(nets) \u003e 1:\n                # Attaching to more than one network by default doesn\u0027t\n                # make sense, as the order will be arbitrary and the guest OS\n                # won\u0027t know which to configure\n                msg \u003d _(\"Multiple possible networks found, use a Network \"\n                         \"ID to be more specific.\")\n                raise exception.NetworkAmbiguous(msg)\n            else:\n                ports_needed_per_instance \u003d 1\n\nit has the networking validation, but , the unit test can not be passed.\n\n    \u0027No reply on topic %s\u0027 % target.topic)\nMessagingTimeout: No reply on topic network\n\nany suggestion on this ?\n\nEli","commit_id":"968604a15abc4463b6f3b9bfa2378dd8dd955bdc"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"725b6af252c95882c21301a21f5e3530fff8da95","unresolved":false,"context_lines":[{"line_number":300,"context_line":"        security_groups \u003d kwargs.get(\u0027security_groups\u0027, [])"},{"line_number":301,"context_line":"        security_group_ids \u003d []"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"        if not requested_networks and not len(security_groups) \\"},{"line_number":304,"context_line":"           and len(nets) \u003e 1:"},{"line_number":305,"context_line":"            msg \u003d _(\"Multiple possible networks found, use a Network \""},{"line_number":306,"context_line":"                    \"ID to be more specific.\")"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_b392169e","line":303,"in_reply_to":"baada198_b8d91586","updated":"2014-08-09 10:55:23.000000000","message":"see from test case call trace.\n\n  File \"nova/network/api.py\", line 44, in wrapped\n    return func(self, context, *args, **kwargs)\n  File \"nova/network/api.py\", line 370, in validate_networks\n    requested_networks)\n  File \"nova/network/rpcapi.py\", line 232, in validate_networks\n\nI supported to call validate_networks in nova/network/neutronv2/api.py but , why it is nova/network/api.py?","commit_id":"968604a15abc4463b6f3b9bfa2378dd8dd955bdc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"be1c6568b91b82c94d74f83f8c547dc90b840ad7","unresolved":false,"context_lines":[{"line_number":540,"context_line":"        # NOTE(geekinutah): It would be nice if use_slave had us call"},{"line_number":541,"context_line":"        #                   special APIs that pummeled slaves instead of"},{"line_number":542,"context_line":"        #                   the master. For now we just ignore this arg."},{"line_number":543,"context_line":"        LOG.debug(\u0027get_instance_nw_info()\u0027, instance\u003dinstance)"},{"line_number":544,"context_line":"        result \u003d self._get_instance_nw_info(context, instance, networks,"},{"line_number":545,"context_line":"                                            port_ids)"},{"line_number":546,"context_line":"        base_api.update_instance_cache_with_nw_info(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":25,"id":"1abeadc6_19544253","line":543,"updated":"2014-08-18 18:38:18.000000000","message":"This cleanup should be reserved for a separate change.","commit_id":"da609d016634a3df610b1dbfc68e2c92f5a60fcb"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"c8cf98979879a0ad9c774e64e35c0d11d0954fa6","unresolved":false,"context_lines":[{"line_number":540,"context_line":"        # NOTE(geekinutah): It would be nice if use_slave had us call"},{"line_number":541,"context_line":"        #                   special APIs that pummeled slaves instead of"},{"line_number":542,"context_line":"        #                   the master. For now we just ignore this arg."},{"line_number":543,"context_line":"        LOG.debug(\u0027get_instance_nw_info()\u0027, instance\u003dinstance)"},{"line_number":544,"context_line":"        result \u003d self._get_instance_nw_info(context, instance, networks,"},{"line_number":545,"context_line":"                                            port_ids)"},{"line_number":546,"context_line":"        base_api.update_instance_cache_with_nw_info(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":25,"id":"1abeadc6_cb0022ad","line":543,"in_reply_to":"1abeadc6_19544253","updated":"2014-08-19 02:23:25.000000000","message":"agreed","commit_id":"da609d016634a3df610b1dbfc68e2c92f5a60fcb"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"be1c6568b91b82c94d74f83f8c547dc90b840ad7","unresolved":false,"context_lines":[{"line_number":551,"context_line":"                              port_ids\u003dNone):"},{"line_number":552,"context_line":"        # keep this caching-free version of the get_instance_nw_info method"},{"line_number":553,"context_line":"        # because it is used by the caching logic itself."},{"line_number":554,"context_line":"        LOG.debug(\u0027_get_instance_nw_info()\u0027, instance\u003dinstance)"},{"line_number":555,"context_line":"        nw_info \u003d self._build_network_info_model(context, instance, networks,"},{"line_number":556,"context_line":"                                                 port_ids)"},{"line_number":557,"context_line":"        return network_model.NetworkInfo.hydrate(nw_info)"}],"source_content_type":"text/x-python","patch_set":25,"id":"1abeadc6_794b76b0","line":554,"updated":"2014-08-18 18:38:18.000000000","message":"ditto","commit_id":"da609d016634a3df610b1dbfc68e2c92f5a60fcb"}],"nova/tests/api/openstack/compute/plugins/v3/test_attach_interfaces.py":[{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"752df8694abd59618d3e97978333e6db0372cc9a","unresolved":false,"context_lines":[{"line_number":299,"context_line":"        self.assertEqual(result[\u0027interface_attachment\u0027][\u0027net_id\u0027],"},{"line_number":300,"context_line":"                         FAKE_NET_ID1)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def test_attach_interface_with_mutiple_networks(self):"},{"line_number":303,"context_line":"        def fake_attach_interface_with_mutiple_net(self, context,"},{"line_number":304,"context_line":"                     instance, network_id, port_id, requested_ip):"},{"line_number":305,"context_line":"            raise exception.NetworkAmbiguous()"}],"source_content_type":"text/x-python","patch_set":18,"id":"baada198_096d8003","line":302,"updated":"2014-08-08 07:09:25.000000000","message":"You can use mock instead stubs :)\n\n    @mock.patch.object(compute_api.API, \u0027attach_interface\u0027,\n                       side_effect\u003dexception.NetworkAmbiguous())\n    def test_attach_interface_with_mutiple_networks(self, mock_attach):\n    ...","commit_id":"95fd615f8edc435abb074dde431e00edf4e32774"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"8b4cc2947f3f8d3cb185cd241eb4ce2cd483fda1","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                         FAKE_NET_ID1)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    @mock.patch.object(compute_api.API, \u0027attach_interface\u0027,"},{"line_number":303,"context_line":"                      side_effect\u003dexception.NetworkAmbiguous())"},{"line_number":304,"context_line":"    def test_attach_interface_with_mutiple_networks(self, mock_attach):"},{"line_number":305,"context_line":"        attachments \u003d attach_interfaces.InterfaceAttachmentController()"},{"line_number":306,"context_line":"        req \u003d webob.Request.blank("}],"source_content_type":"text/x-python","patch_set":20,"id":"baada198_73e15e96","line":303,"updated":"2014-08-09 13:51:12.000000000","message":"\"side_effect\" in L303 should line up with \"compute_api\" in L302","commit_id":"83b4b4f0725e4b079f38f0a91235f3490c334402"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"2583256c0bc3d42d342c63a8f4d419ab1881b20f","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                         FAKE_NET_ID1)"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    @mock.patch.object(compute_api.API, \u0027attach_interface\u0027,"},{"line_number":303,"context_line":"                      side_effect\u003dexception.NetworkAmbiguous())"},{"line_number":304,"context_line":"    def test_attach_interface_with_mutiple_networks(self, mock_attach):"},{"line_number":305,"context_line":"        attachments \u003d attach_interfaces.InterfaceAttachmentController()"},{"line_number":306,"context_line":"        req \u003d webob.Request.blank("}],"source_content_type":"text/x-python","patch_set":20,"id":"baada198_5c6d3f2e","line":303,"in_reply_to":"baada198_73e15e96","updated":"2014-08-11 02:22:25.000000000","message":"Done","commit_id":"83b4b4f0725e4b079f38f0a91235f3490c334402"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"b20b68263e85e39affe05b6f72a7e237609637d1","unresolved":false,"context_lines":[{"line_number":312,"context_line":"        self.assertRaises(exc.HTTPBadRequest,"},{"line_number":313,"context_line":"                          attachments.create, req, FAKE_UUID1,"},{"line_number":314,"context_line":"                          body\u003djsonutils.loads(req.body))"},{"line_number":315,"context_line":"        mock_attach.assert_called_once_with(self.context, {},"},{"line_number":316,"context_line":"                                           None, None, None)"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    def test_attach_interface_with_network_id(self):"}],"source_content_type":"text/x-python","patch_set":20,"id":"baada198_e078212e","line":315,"updated":"2014-08-11 08:08:57.000000000","message":"alignment","commit_id":"83b4b4f0725e4b079f38f0a91235f3490c334402"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"5461e58aa8772fc593fb1f52994149f33cd2e93a","unresolved":false,"context_lines":[{"line_number":312,"context_line":"        self.assertRaises(exc.HTTPBadRequest,"},{"line_number":313,"context_line":"                          attachments.create, req, FAKE_UUID1,"},{"line_number":314,"context_line":"                          body\u003djsonutils.loads(req.body))"},{"line_number":315,"context_line":"        mock_attach.assert_called_once_with(self.context, {},"},{"line_number":316,"context_line":"                                           None, None, None)"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    def test_attach_interface_with_network_id(self):"}],"source_content_type":"text/x-python","patch_set":20,"id":"baada198_a48ab262","line":315,"in_reply_to":"baada198_e078212e","updated":"2014-08-11 12:57:55.000000000","message":"Done","commit_id":"83b4b4f0725e4b079f38f0a91235f3490c334402"}],"nova/tests/compute/test_compute.py":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"dba925ce0cebbbe003f24fb48699a488f885ee88","unresolved":false,"context_lines":[{"line_number":8715,"context_line":"        sys_meta \u003d flavors.save_flavor_info({}, new_type)"},{"line_number":8716,"context_line":""},{"line_number":8717,"context_line":"        instance \u003d objects.Instance(image_ref\u003d\u0027foo\u0027,"},{"line_number":8718,"context_line":"                                    system_metadata\u003dsys_meta)"},{"line_number":8719,"context_line":"        nwinfo \u003d [fake_network_cache_model.new_vif()]"},{"line_number":8720,"context_line":"        network_id \u003d nwinfo[0][\u0027network\u0027][\u0027id\u0027]"},{"line_number":8721,"context_line":"        port_id \u003d nwinfo[0][\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_e8e391d5","line":8718,"updated":"2014-07-28 02:29:46.000000000","message":"The unittest log said, the instance object missing uuid attribute. So there need provide uuid at here:\n\n        instance \u003d objects.Instance(uuid\u003d\u0027fake_id\u0027, image_ref\u003d\u0027foo\u0027,\n                                    system_metadata\u003dsys_meta)","commit_id":"8282082617d8f4f25d410e67d8c5ab155359effb"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"dba925ce0cebbbe003f24fb48699a488f885ee88","unresolved":false,"context_lines":[{"line_number":8748,"context_line":"        sys_meta \u003d flavors.save_flavor_info({}, new_type)"},{"line_number":8749,"context_line":""},{"line_number":8750,"context_line":"        instance \u003d objects.Instance(image_ref\u003d\u0027foo\u0027,"},{"line_number":8751,"context_line":"                                    system_metadata\u003dsys_meta)"},{"line_number":8752,"context_line":"        vif_pub \u003d dict("},{"line_number":8753,"context_line":"            id\u003d1,"},{"line_number":8754,"context_line":"            address\u003d\u0027aa:aa:aa:aa:aa:aa\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_28d109fb","line":8751,"updated":"2014-07-28 02:29:46.000000000","message":"ditto","commit_id":"8282082617d8f4f25d410e67d8c5ab155359effb"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"13d444bbf1dd5a2f892962aaa680f44cae542289","unresolved":false,"context_lines":[{"line_number":8717,"context_line":"        self.assertEqual(vif[\u0027id\u0027], network_id)"},{"line_number":8718,"context_line":"        return nwinfo, port_id"},{"line_number":8719,"context_line":""},{"line_number":8720,"context_line":"    def test_attach_interface_with_port_id_failed(self):"},{"line_number":8721,"context_line":"        new_type \u003d flavors.get_flavor_by_flavor_id(\u00274\u0027)"},{"line_number":8722,"context_line":"        sys_meta \u003d flavors.save_flavor_info({}, new_type)"},{"line_number":8723,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_62dd4e41","line":8720,"updated":"2014-08-07 14:25:51.000000000","message":"Try to use mock-decorators instead `contextlib.nested`","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"8351863363c4f7fd2890ec10114f4414eecc975a","unresolved":false,"context_lines":[{"line_number":8717,"context_line":"        self.assertEqual(vif[\u0027id\u0027], network_id)"},{"line_number":8718,"context_line":"        return nwinfo, port_id"},{"line_number":8719,"context_line":""},{"line_number":8720,"context_line":"    def test_attach_interface_with_port_id_failed(self):"},{"line_number":8721,"context_line":"        new_type \u003d flavors.get_flavor_by_flavor_id(\u00274\u0027)"},{"line_number":8722,"context_line":"        sys_meta \u003d flavors.save_flavor_info({}, new_type)"},{"line_number":8723,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_a5c5b05e","line":8720,"in_reply_to":"baada198_62dd4e41","updated":"2014-08-07 14:56:42.000000000","message":"I just don\u0027t know how to use it :(\nI will take a look at other test case example.\nthanks.","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"13d444bbf1dd5a2f892962aaa680f44cae542289","unresolved":false,"context_lines":[{"line_number":8783,"context_line":""},{"line_number":8784,"context_line":"        instance \u003d objects.Instance(id\u003d1, uuid\u003d\u0027fake_id\u0027,"},{"line_number":8785,"context_line":"                                    image_ref\u003d\u0027foo\u0027, system_metadata\u003dsys_meta)"},{"line_number":8786,"context_line":"        with contextlib.nested("},{"line_number":8787,"context_line":"            mock.patch.object(self.compute.network_api, \u0027validate_networks\u0027"},{"line_number":8788,"context_line":"            )) as (mock_validate,):"},{"line_number":8789,"context_line":"            mock_validate.side_effect \u003d exception.NetworkAmbiguous("}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_426beab8","line":8786,"updated":"2014-08-07 14:25:51.000000000","message":"Why you use `contextlib.nested` here?","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"8351863363c4f7fd2890ec10114f4414eecc975a","unresolved":false,"context_lines":[{"line_number":8783,"context_line":""},{"line_number":8784,"context_line":"        instance \u003d objects.Instance(id\u003d1, uuid\u003d\u0027fake_id\u0027,"},{"line_number":8785,"context_line":"                                    image_ref\u003d\u0027foo\u0027, system_metadata\u003dsys_meta)"},{"line_number":8786,"context_line":"        with contextlib.nested("},{"line_number":8787,"context_line":"            mock.patch.object(self.compute.network_api, \u0027validate_networks\u0027"},{"line_number":8788,"context_line":"            )) as (mock_validate,):"},{"line_number":8789,"context_line":"            mock_validate.side_effect \u003d exception.NetworkAmbiguous("}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_25a1a0a8","line":8786,"in_reply_to":"baada198_426beab8","updated":"2014-08-07 14:56:42.000000000","message":":(, will update it.","commit_id":"d2ea8a09cff9e20d8170fea0b5fb69f05a776f80"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"752df8694abd59618d3e97978333e6db0372cc9a","unresolved":false,"context_lines":[{"line_number":8734,"context_line":"                           \u0027allocate_port_for_instance\u0027, return_value\u003dnwinfo)"},{"line_number":8735,"context_line":"        @mock.patch.object(self.compute.network_api,"},{"line_number":8736,"context_line":"                           \u0027deallocate_port_for_instance\u0027)"},{"line_number":8737,"context_line":"        def do_test(mock_deallocate, mock_allocate, mock_attach):"},{"line_number":8738,"context_line":"            self.assertRaises(exception.InterfaceAttachFailed,"},{"line_number":8739,"context_line":"                              self.compute.attach_interface, self.context,"},{"line_number":8740,"context_line":"                              instance, network_id, port_id, req_ip)"}],"source_content_type":"text/x-python","patch_set":18,"id":"baada198_4955b81d","line":8737,"range":{"start_line":8737,"start_character":12,"end_line":8737,"end_character":19},"updated":"2014-08-08 07:09:25.000000000","message":"Please try use mock-decorators for `test_attach_interface_failed` without this method - `do_test`.","commit_id":"95fd615f8edc435abb074dde431e00edf4e32774"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"8b4cc2947f3f8d3cb185cd241eb4ce2cd483fda1","unresolved":false,"context_lines":[{"line_number":8743,"context_line":"            mock_deallocate.assert_called_once_with(self.context, instance,"},{"line_number":8744,"context_line":"                                                    port_id)"},{"line_number":8745,"context_line":""},{"line_number":8746,"context_line":"        do_test()"},{"line_number":8747,"context_line":""},{"line_number":8748,"context_line":"    def test_detach_interface(self):"},{"line_number":8749,"context_line":"        nwinfo, port_id \u003d self.test_attach_interface()"}],"source_content_type":"text/x-python","patch_set":20,"id":"baada198_939d120c","line":8746,"updated":"2014-08-09 13:51:12.000000000","message":"Why not use a \"with contextlib.nested\" instead of a nested function?","commit_id":"83b4b4f0725e4b079f38f0a91235f3490c334402"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"2583256c0bc3d42d342c63a8f4d419ab1881b20f","unresolved":false,"context_lines":[{"line_number":8743,"context_line":"            mock_deallocate.assert_called_once_with(self.context, instance,"},{"line_number":8744,"context_line":"                                                    port_id)"},{"line_number":8745,"context_line":""},{"line_number":8746,"context_line":"        do_test()"},{"line_number":8747,"context_line":""},{"line_number":8748,"context_line":"    def test_detach_interface(self):"},{"line_number":8749,"context_line":"        nwinfo, port_id \u003d self.test_attach_interface()"}],"source_content_type":"text/x-python","patch_set":20,"id":"baada198_bc53bb6c","line":8746,"in_reply_to":"baada198_939d120c","updated":"2014-08-11 02:22:25.000000000","message":"Hehe,  I used with contextlib.nested before, but Mikhail Durnosvistov require me to use mock-decorators,  but I found I can not use self.compute.driver out of test_attach_interface_failed , su I use an nested do_test, any suggestion ? should I switch it back to with contextlib.nested","commit_id":"83b4b4f0725e4b079f38f0a91235f3490c334402"},{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"fad71221df5b78da1dff52d40287edab97f2c84c","unresolved":false,"context_lines":[{"line_number":8866,"context_line":"                           \u0027allocate_port_for_instance\u0027, return_value\u003dnwinfo)"},{"line_number":8867,"context_line":"        @mock.patch.object(self.compute.network_api,"},{"line_number":8868,"context_line":"                           \u0027deallocate_port_for_instance\u0027)"},{"line_number":8869,"context_line":"        def do_test(mock_deallocate, mock_allocate, mock_attach):"},{"line_number":8870,"context_line":"            self.assertRaises(exception.InterfaceAttachFailed,"},{"line_number":8871,"context_line":"                              self.compute.attach_interface, self.context,"},{"line_number":8872,"context_line":"                              instance, network_id, port_id, req_ip)"}],"source_content_type":"text/x-python","patch_set":26,"id":"1abeadc6_41ba33df","line":8869,"updated":"2014-08-19 04:35:04.000000000","message":"i think we should use contextlib instead of making this inlined function","commit_id":"4a5e2501b4aca2a6aad6c301d8a59f8ce642da55"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"f43e55788e7ed4321ca0f3cb3836bb82293af143","unresolved":false,"context_lines":[{"line_number":8866,"context_line":"                           \u0027allocate_port_for_instance\u0027, return_value\u003dnwinfo)"},{"line_number":8867,"context_line":"        @mock.patch.object(self.compute.network_api,"},{"line_number":8868,"context_line":"                           \u0027deallocate_port_for_instance\u0027)"},{"line_number":8869,"context_line":"        def do_test(mock_deallocate, mock_allocate, mock_attach):"},{"line_number":8870,"context_line":"            self.assertRaises(exception.InterfaceAttachFailed,"},{"line_number":8871,"context_line":"                              self.compute.attach_interface, self.context,"},{"line_number":8872,"context_line":"                              instance, network_id, port_id, req_ip)"}],"source_content_type":"text/x-python","patch_set":26,"id":"1abeadc6_01691bf9","line":8869,"in_reply_to":"1abeadc6_41ba33df","updated":"2014-08-19 05:12:46.000000000","message":"okay , then ,I need to revert back to historical version.. :(","commit_id":"4a5e2501b4aca2a6aad6c301d8a59f8ce642da55"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3fff3a9dbd1b4c96f975b4d2cb3bb2048ce24fe9","unresolved":false,"context_lines":[{"line_number":9147,"context_line":"        self.assertEqual(vif[\u0027id\u0027], network_id)"},{"line_number":9148,"context_line":"        return nwinfo, port_id"},{"line_number":9149,"context_line":""},{"line_number":9150,"context_line":"    def test_attach_interface_failed(self):"},{"line_number":9151,"context_line":"        new_type \u003d flavors.get_flavor_by_flavor_id(\u00274\u0027)"},{"line_number":9152,"context_line":"        sys_meta \u003d flavors.save_flavor_info({}, new_type)"},{"line_number":9153,"context_line":"        instance \u003d objects.Instance(uuid\u003d\u0027fake_id\u0027, image_ref\u003d\u0027foo\u0027,"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_bd604936","line":9150,"updated":"2015-03-12 14:00:06.000000000","message":"nit: if you\u0027re going to mock everything out and not rely on the database with a running instance, I\u0027d prefer to see this test moved to test_compute_mgr.py since we have more non-db mock only tests in there, rather than add more tests to the legacy test_compute.py module which relies on the database for a lot of it\u0027s tests and is more integration style testing in nature.","commit_id":"71ee54e6264f8961f1f3b13c361e905137822e59"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"6631dafbefbc7fb2d75e0af41baf5d47f8ee43c9","unresolved":false,"context_lines":[{"line_number":9147,"context_line":"        self.assertEqual(vif[\u0027id\u0027], network_id)"},{"line_number":9148,"context_line":"        return nwinfo, port_id"},{"line_number":9149,"context_line":""},{"line_number":9150,"context_line":"    def test_attach_interface_failed(self):"},{"line_number":9151,"context_line":"        new_type \u003d flavors.get_flavor_by_flavor_id(\u00274\u0027)"},{"line_number":9152,"context_line":"        sys_meta \u003d flavors.save_flavor_info({}, new_type)"},{"line_number":9153,"context_line":"        instance \u003d objects.Instance(uuid\u003d\u0027fake_id\u0027, image_ref\u003d\u0027foo\u0027,"}],"source_content_type":"text/x-python","patch_set":38,"id":"9a80dd14_5d4c8942","line":9150,"in_reply_to":"9a80dd14_bd604936","updated":"2015-03-13 02:58:32.000000000","message":"hi matt, thanks for your reviewing, good to know it.\n\nbut for this case, I need to use a fake nwinfo, which is from db, so maybe I can not move this test case to test_compute_mgr.py","commit_id":"71ee54e6264f8961f1f3b13c361e905137822e59"}]}