)]}'
{"tempest/common/compute.py":[{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"ff9dac634e807150a3a9a2de5dcc49278f07ac7a","unresolved":false,"context_lines":[{"line_number":30,"context_line":"def create_test_server(clients, validatable\u003dFalse, validation_resources\u003dNone,"},{"line_number":31,"context_line":"                       tenant_network\u003dNone, wait_until\u003dNone,"},{"line_number":32,"context_line":"                       volume_backed\u003dFalse, name\u003dNone, flavor\u003dNone,"},{"line_number":33,"context_line":"                       image_id\u003dNone, delete_vol_on_termination\u003dTrue,"},{"line_number":34,"context_line":"                       **kwargs):"},{"line_number":35,"context_line":"    \"\"\"Common wrapper utility returning a test server."},{"line_number":36,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_44612e04","side":"PARENT","line":33,"range":{"start_line":33,"start_character":38,"end_line":33,"end_character":69},"updated":"2016-12-16 21:06:56.000000000","message":"I don\u0027t think this is an issue with the helper.\nWhen this is used to provision a volume backed server, the helper actually provisions two resources, a server and a volume. \n\nFor the server a cleanup must be scheduled by the caller.\nFor the volume it\u0027s the same. If delete_vol_on_termination is True the volume will be already cleaned-up,but that\u0027s not an issue.\n\nPorts are cleaned up on terminations unless they are specified in the network section, so it\u0027s the same thing there.\n\nIt may be worth considering instead to split the volume creation out of this helper, and to accept a volume-id in case of volume backed instances. That would make the need for a cleanup more obvious.\n\nThat said, I didn\u0027t find any single case where the delete_vol_on_termination flag is used, which means it may be ok to remove that from the helper. \nBut I really would prefer the volume_backed bit to go away and have tests create a volume and pass it to the create server, it would make the code clearer to read and debug.","commit_id":"c21931840ac0795066efeba56b0bb5e29421babb"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"773a5c9d8875c1cb66c58bb4110377d2e12604b8","unresolved":false,"context_lines":[{"line_number":30,"context_line":"def create_test_server(clients, validatable\u003dFalse, validation_resources\u003dNone,"},{"line_number":31,"context_line":"                       tenant_network\u003dNone, wait_until\u003dNone,"},{"line_number":32,"context_line":"                       volume_backed\u003dFalse, name\u003dNone, flavor\u003dNone,"},{"line_number":33,"context_line":"                       image_id\u003dNone, delete_vol_on_termination\u003dTrue,"},{"line_number":34,"context_line":"                       **kwargs):"},{"line_number":35,"context_line":"    \"\"\"Common wrapper utility returning a test server."},{"line_number":36,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_b6dc9419","side":"PARENT","line":33,"range":{"start_line":33,"start_character":38,"end_line":33,"end_character":69},"in_reply_to":"1a6eadb0_2f0cb993","updated":"2016-12-20 09:31:23.000000000","message":"Yeah, I agree, you\u0027re right.","commit_id":"c21931840ac0795066efeba56b0bb5e29421babb"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"c8d7231b8709a1ca46f9f8ccfcc1bbec00486793","unresolved":false,"context_lines":[{"line_number":30,"context_line":"def create_test_server(clients, validatable\u003dFalse, validation_resources\u003dNone,"},{"line_number":31,"context_line":"                       tenant_network\u003dNone, wait_until\u003dNone,"},{"line_number":32,"context_line":"                       volume_backed\u003dFalse, name\u003dNone, flavor\u003dNone,"},{"line_number":33,"context_line":"                       image_id\u003dNone, delete_vol_on_termination\u003dTrue,"},{"line_number":34,"context_line":"                       **kwargs):"},{"line_number":35,"context_line":"    \"\"\"Common wrapper utility returning a test server."},{"line_number":36,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_c67287b0","side":"PARENT","line":33,"range":{"start_line":33,"start_character":38,"end_line":33,"end_character":69},"in_reply_to":"1a6eadb0_44612e04","updated":"2016-12-19 02:27:30.000000000","message":"Yea I started this patch to remove the volume backed things completely and let tests create volume, prepare the bdm dict and pass it as kwargs.\n\nBut on second thought I considered to keep this as per below 2 cases:\n1. Non-cinder tests who just want a booted volume and server from it. They do not care with what all bdm param\nlike- test_shelve_instance, test_server_advanced_ops. etc\n In this case, helper will create bdm with \u0027delete_on_termination\u0027: True as hard coded. \n\n2. Tests need to boot server from volume with bdm1 or bdm2 with different combination of bdm param like- test_volume_boot_pattern\n\nSo with volume_backed in this helper, we can consume the 1st case and for 2nd one, test case can do with kwargs (i tried to put those in doc string also).","commit_id":"c21931840ac0795066efeba56b0bb5e29421babb"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"b966f8fdd5b85ed14e68308743395ec979587dae","unresolved":false,"context_lines":[{"line_number":30,"context_line":"def create_test_server(clients, validatable\u003dFalse, validation_resources\u003dNone,"},{"line_number":31,"context_line":"                       tenant_network\u003dNone, wait_until\u003dNone,"},{"line_number":32,"context_line":"                       volume_backed\u003dFalse, name\u003dNone, flavor\u003dNone,"},{"line_number":33,"context_line":"                       image_id\u003dNone, delete_vol_on_termination\u003dTrue,"},{"line_number":34,"context_line":"                       **kwargs):"},{"line_number":35,"context_line":"    \"\"\"Common wrapper utility returning a test server."},{"line_number":36,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_2f0cb993","side":"PARENT","line":33,"range":{"start_line":33,"start_character":38,"end_line":33,"end_character":69},"in_reply_to":"1a6eadb0_77d06cca","updated":"2016-12-20 02:35:09.000000000","message":"Yea but here we are giving partial responsibility for volume resource, create volume automatically in this method and let caller decide to delete that which they can not do.\n\nActually if any tests pass delete_vol_on_termination  as \"False\" then, neither tests nor this method can delete the volume. so taking this in kwargs becomes problematic. \nThere is no use case for delete_vol_on_termination as arg\n\nThis helper should provide 2 options:\n1. volume_backed \u003d True - request from boot from volume, everything will be done automatically for tests.\n2. tests want to handle booted volume things, they can do volume_backed\u003d false and pass all necessary kwargs as bdm dict","commit_id":"c21931840ac0795066efeba56b0bb5e29421babb"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"cf1ebf9ddddb219a0b3c73e49baef961db732caf","unresolved":false,"context_lines":[{"line_number":30,"context_line":"def create_test_server(clients, validatable\u003dFalse, validation_resources\u003dNone,"},{"line_number":31,"context_line":"                       tenant_network\u003dNone, wait_until\u003dNone,"},{"line_number":32,"context_line":"                       volume_backed\u003dFalse, name\u003dNone, flavor\u003dNone,"},{"line_number":33,"context_line":"                       image_id\u003dNone, delete_vol_on_termination\u003dTrue,"},{"line_number":34,"context_line":"                       **kwargs):"},{"line_number":35,"context_line":"    \"\"\"Common wrapper utility returning a test server."},{"line_number":36,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_77d06cca","side":"PARENT","line":33,"range":{"start_line":33,"start_character":38,"end_line":33,"end_character":69},"in_reply_to":"1a6eadb0_c67287b0","updated":"2016-12-19 11:38:53.000000000","message":"I think we should leave the argument \u0027delete_vol_on_termination\u0027 here, not suppress it. We shouldn\u0027t restrict what\u0027s possible with this method. The caller (i.e the test) should be in charge of not leaking resource, not this method.","commit_id":"c21931840ac0795066efeba56b0bb5e29421babb"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"773a5c9d8875c1cb66c58bb4110377d2e12604b8","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    :param wait_until: Server status to wait for the server to reach after"},{"line_number":45,"context_line":"        its creation."},{"line_number":46,"context_line":"    :param volume_backed: Whether the server is volume backed or not."},{"line_number":47,"context_line":"                          If this is true, a volume will be created and"},{"line_number":48,"context_line":"                          create server will be requested with"},{"line_number":49,"context_line":"                          \u0027block_device_mapping_v2\u0027 populated with below"},{"line_number":50,"context_line":"                          values:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_76dbdc1b","line":47,"updated":"2016-12-20 09:31:23.000000000","message":"Please indent as other comments.","commit_id":"1348f472dcd727627fc03ca4eb24a57953d76746"},{"author":{"_account_id":20190,"name":"zhufl","email":"zhu.fanglei@zte.com.cn","username":"zhufl"},"change_message_id":"b2e5e56f7b7c68a0ab2d3d76e4c35cf9d6132a50","unresolved":false,"context_lines":[{"line_number":58,"context_line":"                          kwargs[\u0027block_device_mapping_v2\u0027] \u003d bd_map_v2"},{"line_number":59,"context_line":"                          ---------------------------------------------"},{"line_number":60,"context_line":"                          If server needs to be booted from volume with other"},{"line_number":61,"context_line":"                          combination of bdm inputs than mentioned above then,"},{"line_number":62,"context_line":"                          pass the bdm inputs explicitly as kwargs and image_id"},{"line_number":63,"context_line":"                          as empty string (\u0027\u0027)."},{"line_number":64,"context_line":"    :param name: Name of the server to be provisioned. If not defined a random"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5201f7_77a09091","line":61,"range":{"start_line":61,"start_character":57,"end_line":61,"end_character":78},"updated":"2016-12-30 08:42:59.000000000","message":"\"mentioned above, then\" ?","commit_id":"1348f472dcd727627fc03ca4eb24a57953d76746"}]}