)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"67318d637c704518c1283f9284297869295a28a0","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This patch proposes to add the tempest tests the available volumes"},{"line_number":10,"context_line":"to migrate from LVM to LVM, from LVM to Storwize, and from"},{"line_number":11,"context_line":"Storwize to LVM."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Other tests for other drivers need other folks to implement."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Iff4d880a0f8e928cbc07b84ff0c357ad59ef929c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"fa32b979_640c023c","line":12,"updated":"2015-06-26 11:56:49.000000000","message":"This patch is not supported only for LVM right? IINW, it can add test any multiple backends. If so, would be better to change the commit message as it gives the idea that its only possible to test LVM and Storwise.","commit_id":"13f5e4d77c8b544b0e40c0ab9c99169c58704bf6"},{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"a71c1651367edb2b4687347bd59a0dbc845fcd5a","unresolved":false,"context_lines":[{"line_number":25,"context_line":"to True."},{"line_number":26,"context_line":"* Set the local variable OS_TEST_TIMEOUT to a value greater than"},{"line_number":27,"context_line":"build_timeout."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Change-Id: Iff4d880a0f8e928cbc07b84ff0c357ad59ef929c"},{"line_number":30,"context_line":"Partial-Implements: blueprint migration-improvement"},{"line_number":31,"context_line":"Depends-On: I89a158a6869e0754e820d42e1f7e30ad9f068b4d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"3a50d1a3_a41fe606","line":28,"updated":"2015-07-22 13:09:17.000000000","message":"Does this have an API or Doc impact? Just wondering!","commit_id":"8779e73090a58e0722a1fe16fe6cc5504184b774"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"f642b3f2cb447268146ba33555553e853ff3f239","unresolved":false,"context_lines":[{"line_number":25,"context_line":"to True."},{"line_number":26,"context_line":"* Set the local variable OS_TEST_TIMEOUT to a value greater than"},{"line_number":27,"context_line":"build_timeout."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Change-Id: Iff4d880a0f8e928cbc07b84ff0c357ad59ef929c"},{"line_number":30,"context_line":"Partial-Implements: blueprint migration-improvement"},{"line_number":31,"context_line":"Depends-On: I89a158a6869e0754e820d42e1f7e30ad9f068b4d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"3a50d1a3_7d07fe81","line":28,"in_reply_to":"3a50d1a3_a41fe606","updated":"2015-07-29 05:48:48.000000000","message":"Doc impact is for sure.\nApi impact is not necessary, since I have added it into other patches I have been working for the migration. https://review.openstack.org/#/c/186312/","commit_id":"8779e73090a58e0722a1fe16fe6cc5504184b774"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"d29a256fec0f80f773818742ea885ad75a5ed33b","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     david liu \u003clzbj@cn.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-08-14 16:25:29 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"modify tempest test for migration according to comments."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"fixed code inconsistency, removed unnecessary configurations."},{"line_number":10,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"da20952f_cf585334","line":7,"updated":"2015-09-09 20:18:28.000000000","message":"Placeholder needs to be replaced with a proper commit message","commit_id":"f815bcab0c1098ecf6d1639f3fae390bec89e1d6"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"cf9dba0bfe2a7e6166b5ec2919a558cab3d21c85","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     david liu \u003clzbj@cn.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-08-14 16:25:29 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"modify tempest test for migration according to comments."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"fixed code inconsistency, removed unnecessary configurations."},{"line_number":10,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":31,"id":"9a1a9d01_765ef2ea","line":7,"in_reply_to":"da20952f_cf585334","updated":"2015-09-25 07:10:18.000000000","message":"I will completely change the commit message.","commit_id":"f815bcab0c1098ecf6d1639f3fae390bec89e1d6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"06b2531fa97ba445afa5b44658d3ef45a70ba466","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Vincent Hou \u003csbhou@cn.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-11-02 20:50:24 -0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add tempest tests for volume migration"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch proposes to add the tempest tests for the available volumes"},{"line_number":10,"context_line":"to migrate from one driver to another. Before running the tests,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":43,"id":"ba8a016a_9e8b5a68","line":7,"updated":"2015-11-23 11:50:12.000000000","message":"-1: I disagree, this is a test for retype with migration, not just migration.","commit_id":"9a0980c9edcf94b691f5145edae4f810a5e193fe"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"06b2531fa97ba445afa5b44658d3ef45a70ba466","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add tempest tests for volume migration"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch proposes to add the tempest tests for the available volumes"},{"line_number":10,"context_line":"to migrate from one driver to another. Before running the tests,"},{"line_number":11,"context_line":"please configure your local cinder deployment with a proper"},{"line_number":12,"context_line":"multi-backend support."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":43,"id":"ba8a016a_fe1f0659","line":10,"updated":"2015-11-23 11:50:12.000000000","message":"-1: You are not mentioning that volume is migrated back, thus testing migration path in both directions.","commit_id":"9a0980c9edcf94b691f5145edae4f810a5e193fe"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"06b2531fa97ba445afa5b44658d3ef45a70ba466","unresolved":false,"context_lines":[{"line_number":11,"context_line":"please configure your local cinder deployment with a proper"},{"line_number":12,"context_line":"multi-backend support."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Configurations in tempest.conf:"},{"line_number":15,"context_line":"* The option backend1_name determines the back-end name of the source."},{"line_number":16,"context_line":"* The option backend2_name determines the back-end name of the"},{"line_number":17,"context_line":"destination."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":43,"id":"ba8a016a_830305b8","line":14,"updated":"2015-11-23 11:50:12.000000000","message":"nit: Maybe mention that these are not new configuration options.","commit_id":"9a0980c9edcf94b691f5145edae4f810a5e193fe"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"06b2531fa97ba445afa5b44658d3ef45a70ba466","unresolved":false,"context_lines":[{"line_number":12,"context_line":"multi-backend support."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Configurations in tempest.conf:"},{"line_number":15,"context_line":"* The option backend1_name determines the back-end name of the source."},{"line_number":16,"context_line":"* The option backend2_name determines the back-end name of the"},{"line_number":17,"context_line":"destination."},{"line_number":18,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":43,"id":"ba8a016a_de46c220","line":15,"updated":"2015-11-23 11:50:12.000000000","message":"nit: Since we are testing migration in both directions: s/source/initial source","commit_id":"9a0980c9edcf94b691f5145edae4f810a5e193fe"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"4c362833758d3c25f6fe4be3a6eafb63e2d777c5","unresolved":false,"context_lines":[{"line_number":18,"context_line":"* The option backend2_name determines the back-end name of the"},{"line_number":19,"context_line":"initial destination."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"DocImpact"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Iff4d880a0f8e928cbc07b84ff0c357ad59ef929c"},{"line_number":24,"context_line":"Partial-Implements: blueprint migration-improvement"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":44,"id":"9a8ffd7b_a20b9e7c","line":21,"updated":"2015-12-01 17:23:21.000000000","message":"What is the docimpact. This flag is used to open a docs bug automagically when this commit lands. Tempest doesn\u0027t have docs team coverage and owns its documentation. If you need to update the docs do it in this patch","commit_id":"055763728fd5a86d7837ec0486613dd1984628ef"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"5d8ed1267858b30579c9a17a802bcdff13a16638","unresolved":false,"context_lines":[{"line_number":18,"context_line":"* The option backend2_name determines the back-end name of the"},{"line_number":19,"context_line":"initial destination."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"DocImpact"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Iff4d880a0f8e928cbc07b84ff0c357ad59ef929c"},{"line_number":24,"context_line":"Partial-Implements: blueprint migration-improvement"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":44,"id":"9a8ffd7b_b1298f8b","line":21,"in_reply_to":"9a8ffd7b_a20b9e7c","updated":"2015-12-02 04:31:13.000000000","message":"Thank you, Matt. Since this feature is based on a Cinder BP, I will remove this flag for Tempest.","commit_id":"055763728fd5a86d7837ec0486613dd1984628ef"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"4c362833758d3c25f6fe4be3a6eafb63e2d777c5","unresolved":false,"context_lines":[{"line_number":21,"context_line":"DocImpact"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Iff4d880a0f8e928cbc07b84ff0c357ad59ef929c"},{"line_number":24,"context_line":"Partial-Implements: blueprint migration-improvement"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":44,"id":"9a8ffd7b_8268da22","line":24,"updated":"2015-12-01 17:23:21.000000000","message":"This isn\u0027t a tempest bp, but whatever","commit_id":"055763728fd5a86d7837ec0486613dd1984628ef"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"5d8ed1267858b30579c9a17a802bcdff13a16638","unresolved":false,"context_lines":[{"line_number":21,"context_line":"DocImpact"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Iff4d880a0f8e928cbc07b84ff0c357ad59ef929c"},{"line_number":24,"context_line":"Partial-Implements: blueprint migration-improvement"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":44,"id":"9a8ffd7b_3147df1e","line":24,"in_reply_to":"9a8ffd7b_8268da22","updated":"2015-12-02 04:31:13.000000000","message":"It is supposed to be part of a cinder project, about adding an integration functional test for volume retype/migration, since migration is a complex feature, depending on many different cinder back-end drivers.","commit_id":"055763728fd5a86d7837ec0486613dd1984628ef"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b94d219451eb83e9ce6b5cd98e30b91d3bfa3f5a","unresolved":false,"context_lines":[{"line_number":18,"context_line":"* The option backend2_name determines the back-end name of the"},{"line_number":19,"context_line":"initial destination."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"change-Id: Iff4d880a0f8e928cbc07b84ff0c357ad59ef929c"},{"line_number":22,"context_line":"Partial-Implements: blueprint migration-improvement"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":47,"id":"7a740942_880feb2d","line":21,"updated":"2015-12-09 13:04:31.000000000","message":"Why the change to a small c?","commit_id":"4c4889a21b75e7e87444d30365a6218c01bbae1f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"1ef158e976d00e3d1e09d79c515827fd1d02eb0e","unresolved":false,"context_lines":[{"line_number":15,"context_line":"tempest.conf using the following syntax:"},{"line_number":16,"context_line":"backend_names \u003d lvmdriver-1,lvmdriver-2"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This test is not covered in the gate job yet, because it"},{"line_number":19,"context_line":"needs at least two cinder back-ends configured and we only have"},{"line_number":20,"context_line":"got one cinder back-end so far."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Depends-on: I5fa16ad3786dd488136b821d3c2e95e41cbc4df5"},{"line_number":23,"context_line":"Depends-on: I7d428d6dfb8f03e94fdd0d4a9ff2abf7344e5ac1"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":80,"id":"ba5201f7_139af114","line":20,"range":{"start_line":18,"start_character":0,"end_line":20,"end_character":31},"updated":"2016-12-30 14:54:44.000000000","message":"so this can  be removed now as it runs as experimental job","commit_id":"256911793c75d2abacd72dd9002dbbfd82f50bf6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8e38da374e42af2b9abca2f22f5c991ac7539d91","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add tempest tests for volume retype with migration"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch proposes to add the tempest tests for the available volumes"},{"line_number":10,"context_line":"to migrate from one driver to another in both directions. Before"},{"line_number":11,"context_line":"running the tests, please configure your local cinder deployment"},{"line_number":12,"context_line":"with a proper multi-backend support."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":84,"id":"5a3905b3_4f25bbc0","line":10,"range":{"start_line":10,"start_character":38,"end_line":10,"end_character":56},"updated":"2017-01-19 11:01:08.000000000","message":"-1: The test is only doing it in one direction","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9b9b80cf3c94953f8c974d1b9e0d35678902f277","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add tempest tests for volume retype with migration"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch proposes to add the tempest tests for the available volumes"},{"line_number":10,"context_line":"to migrate from one driver to another in both directions. Before"},{"line_number":11,"context_line":"running the tests, please configure your local cinder deployment"},{"line_number":12,"context_line":"with a proper multi-backend support."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":84,"id":"5a3905b3_8e5dceeb","line":10,"range":{"start_line":10,"start_character":38,"end_line":10,"end_character":56},"in_reply_to":"5a3905b3_2a885de7","updated":"2017-01-19 17:00:24.000000000","message":"OK, good to know","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"759bbfc9c4d030c1dc7fe94d0099114b4662969a","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add tempest tests for volume retype with migration"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch proposes to add the tempest tests for the available volumes"},{"line_number":10,"context_line":"to migrate from one driver to another in both directions. Before"},{"line_number":11,"context_line":"running the tests, please configure your local cinder deployment"},{"line_number":12,"context_line":"with a proper multi-backend support."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":84,"id":"5a3905b3_2a885de7","line":10,"range":{"start_line":10,"start_character":38,"end_line":10,"end_character":56},"in_reply_to":"5a3905b3_4f25bbc0","updated":"2017-01-19 16:48:06.000000000","message":"It use to be like that in until PS 50 I think. Now it is not anymore, it migrates from the first backend in the list to the second, so we can have better control when testing multiple backend combinations[1].\n\nIll update the commit message.\n[1] https://review.openstack.org/#/c/381736/13/tools/hooks/run_multi_backend_matrix.sh","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8e38da374e42af2b9abca2f22f5c991ac7539d91","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This test is not covered in the gate job yet, because it"},{"line_number":19,"context_line":"needs at least two cinder back-ends configured and we only have"},{"line_number":20,"context_line":"got one cinder back-end so far."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Depends-on: I4ed9b1f30d4e4e595c44a0ce243c2463069833d7"},{"line_number":23,"context_line":"Change-Id: Iff4d880a0f8e928cbc07b84ff0c357ad59ef929c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":84,"id":"5a3905b3_ef1ca779","line":20,"updated":"2017-01-19 11:01:08.000000000","message":"This should be easy to do, right? We can easily have 2 LVM backends in devstack using CINDER_ENABLED_BACKENDS.","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"759bbfc9c4d030c1dc7fe94d0099114b4662969a","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"This test is not covered in the gate job yet, because it"},{"line_number":19,"context_line":"needs at least two cinder back-ends configured and we only have"},{"line_number":20,"context_line":"got one cinder back-end so far."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Depends-on: I4ed9b1f30d4e4e595c44a0ce243c2463069833d7"},{"line_number":23,"context_line":"Change-Id: Iff4d880a0f8e928cbc07b84ff0c357ad59ef929c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":84,"id":"5a3905b3_1509062d","line":20,"in_reply_to":"5a3905b3_ef1ca779","updated":"2017-01-19 16:48:06.000000000","message":"Its done already. Check the experimental runs of this job.","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b767c5f67d7074f606707956c0250452c522db58","unresolved":false,"context_lines":[{"line_number":24,"context_line":"This test can be tested by \u0027check experimental\u0027 and must run and pass"},{"line_number":25,"context_line":"the job\u0027lvm-multibackend\u0027."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Depends-on: I4ed9b1f30d4e4e595c44a0ce243c2463069833d7"},{"line_number":28,"context_line":"Depends-on: Id9f39ddab3a3648b8721b711ecf78c18c8293451"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"Change-Id: Iff4d880a0f8e928cbc07b84ff0c357ad59ef929c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":88,"id":"5a3905b3_7f04cd7d","line":27,"updated":"2017-01-19 18:23:00.000000000","message":"nit: This is not necessary if your patches are already chained.","commit_id":"2104f82e06cf558b2ad2033a3dbf21b938e4db8d"}],"etc/tempest.conf.sample":[{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"a71c1651367edb2b4687347bd59a0dbc845fcd5a","unresolved":false,"context_lines":[{"line_number":1172,"context_line":"# the source to the destination and from the destination to the"},{"line_number":1173,"context_line":"# source. Otherwise, only migration from the source to the destination"},{"line_number":1174,"context_line":"# will be tested. (boolean value)"},{"line_number":1175,"context_line":"#full_migration_test \u003d true"},{"line_number":1176,"context_line":""},{"line_number":1177,"context_line":"# Backend protocol to target when creating volume types (string value)"},{"line_number":1178,"context_line":"#storage_protocol \u003d iSCSI"}],"source_content_type":"application/octet-stream","patch_set":21,"id":"3a50d1a3_24f0b6fd","line":1175,"updated":"2015-07-22 13:09:17.000000000","message":"true-\u003e True","commit_id":"8779e73090a58e0722a1fe16fe6cc5504184b774"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"f642b3f2cb447268146ba33555553e853ff3f239","unresolved":false,"context_lines":[{"line_number":1172,"context_line":"# the source to the destination and from the destination to the"},{"line_number":1173,"context_line":"# source. Otherwise, only migration from the source to the destination"},{"line_number":1174,"context_line":"# will be tested. (boolean value)"},{"line_number":1175,"context_line":"#full_migration_test \u003d true"},{"line_number":1176,"context_line":""},{"line_number":1177,"context_line":"# Backend protocol to target when creating volume types (string value)"},{"line_number":1178,"context_line":"#storage_protocol \u003d iSCSI"}],"source_content_type":"application/octet-stream","patch_set":21,"id":"3a50d1a3_5d5c827c","line":1175,"in_reply_to":"3a50d1a3_24f0b6fd","updated":"2015-07-29 05:48:48.000000000","message":"Done","commit_id":"8779e73090a58e0722a1fe16fe6cc5504184b774"}],"tempest/api/volume/admin/test_multi_backend.py":[{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"94a4f29002b4e7f642e6a7b72cdd8b19048c7642","unresolved":false,"context_lines":[{"line_number":62,"context_line":"    def _create_type_and_volume(cls, backend_name_key, with_prefix):"},{"line_number":63,"context_line":"        volume_type \u003d cls.create_type(backend_name_key, with_prefix)"},{"line_number":64,"context_line":"        volume \u003d cls.create_volume_from_type(cls.name_field, volume_type)"},{"line_number":65,"context_line":"        cls.volume_type_id_list.append(volume_type[\u0027id\u0027])"},{"line_number":66,"context_line":"        if with_prefix:"},{"line_number":67,"context_line":"            cls.volume_id_list_with_prefix.append(volume[\u0027id\u0027])"},{"line_number":68,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":47,"id":"7a740942_481ca3c1","line":65,"updated":"2015-12-09 13:11:46.000000000","message":"Hi , \nyou should add to list after creating a volume type , once volume creation fails you wont be able to delete this type from the list.\n\nShould be : \nvolume_type \u003d cls.create_type(backend_name_key, with_prefix)\ncls.volume_type_id_list.append(volume_type[\u0027id\u0027])\nvolume \u003d cls.create_volume_from_type(cls.name_field, volume_type)","commit_id":"4c4889a21b75e7e87444d30365a6218c01bbae1f"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"b3dadb9780ebb7ed761c3b4d3b7493ab22a74ccc","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        type_extra_specs \u003d {spec_key: backend_name_key}"},{"line_number":62,"context_line":"        volume_type \u003d cls.create_volume_type(extra_specs\u003dtype_extra_specs)"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"        cls.create_volume_from_type(cls.name_field, volume_type)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    @classmethod"},{"line_number":67,"context_line":"    def resource_cleanup(cls):"}],"source_content_type":"text/x-python","patch_set":67,"id":"ba5da102_9488a7c8","line":64,"updated":"2016-11-04 17:18:52.000000000","message":"I don\u0027t understand why you had to change this. You don\u0027t need that extra create_volume_from_type utility function.","commit_id":"2d54574e64ee7a0be5a7a81e8e1f5f0035f995d4"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"a0fffa0efacd251d22c880b4008415ee19d89080","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        type_extra_specs \u003d {spec_key: backend_name_key}"},{"line_number":62,"context_line":"        volume_type \u003d cls.create_volume_type(extra_specs\u003dtype_extra_specs)"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"        cls.create_volume_from_type(cls.name_field, volume_type)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    @classmethod"},{"line_number":67,"context_line":"    def resource_cleanup(cls):"}],"source_content_type":"text/x-python","patch_set":67,"id":"9a629dbe_e0677323","line":64,"in_reply_to":"ba5da102_9488a7c8","updated":"2016-11-07 14:42:40.000000000","message":"I think that when this patch was first submit this function wasn\u0027t  merged, then it was merged and I solved the conflict leaving this code which remove some duplicated code by using the create_volume_from_type() function.","commit_id":"2d54574e64ee7a0be5a7a81e8e1f5f0035f995d4"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"d0e386c897b89b130f64dfb8129d9d7661d5d893","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"        cls.create_volume_from_type(cls.name_field, volume_type)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    @classmethod"},{"line_number":67,"context_line":"    def resource_cleanup(cls):"},{"line_number":68,"context_line":"        super(VolumeMultiBackendV2Test, cls).resource_cleanup()"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    @test.idempotent_id(\u0027c1a41f3f-9dad-493e-9f09-3ff197d477cc\u0027)"},{"line_number":71,"context_line":"    def test_backend_name_reporting(self):"}],"source_content_type":"text/x-python","patch_set":67,"id":"ba5da102_b4784b2d","line":68,"range":{"start_line":66,"start_character":0,"end_line":68,"end_character":63},"updated":"2016-11-04 17:30:06.000000000","message":"We can remove this method because this just calls the same method.","commit_id":"2d54574e64ee7a0be5a7a81e8e1f5f0035f995d4"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"a0fffa0efacd251d22c880b4008415ee19d89080","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"        cls.create_volume_from_type(cls.name_field, volume_type)"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    @classmethod"},{"line_number":67,"context_line":"    def resource_cleanup(cls):"},{"line_number":68,"context_line":"        super(VolumeMultiBackendV2Test, cls).resource_cleanup()"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    @test.idempotent_id(\u0027c1a41f3f-9dad-493e-9f09-3ff197d477cc\u0027)"},{"line_number":71,"context_line":"    def test_backend_name_reporting(self):"}],"source_content_type":"text/x-python","patch_set":67,"id":"9a629dbe_ecd04fcc","line":68,"range":{"start_line":66,"start_character":0,"end_line":68,"end_character":63},"in_reply_to":"ba5da102_b4784b2d","updated":"2016-11-07 14:42:40.000000000","message":"Done","commit_id":"2d54574e64ee7a0be5a7a81e8e1f5f0035f995d4"}],"tempest/api/volume/admin/test_volume_migration.py":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"7e3c9c990753249b57109245190446f3179fe87f","unresolved":false,"context_lines":[{"line_number":129,"context_line":"    @test.idempotent_id(\u0027b1a41f3f-9dad-493e-9f09-3ff197d477ce\u0027)"},{"line_number":130,"context_line":"    def test_volume_migration_lvm_to_storwize(self):"},{"line_number":131,"context_line":"        backend_source \u003d \u0027lvm\u0027"},{"line_number":132,"context_line":"        backend_dest \u003d \u0027storwize\u0027"},{"line_number":133,"context_line":"        self._test_volume_migration(backend_source, backend_dest)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    @test.idempotent_id(\u0027c1a41f3f-9dad-493e-9f09-3ff197d473ce\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba3cc151_c2211b4f","line":132,"updated":"2015-06-30 12:45:55.000000000","message":"Its not a good idea to have a test with the name of the backend. You would have to create a test to each BE. You can call the same functions you create for migrate in a more generic test. Then whatever backends the user configure, the test will always migrate the volumes from BE1 -\u003e BE2. If the user configure volume.full_migration_test \u003d True, the test can test both directions.  For example:\n\n@test.idempotent_id(\u0027a1a41f3f-9dad-493e-9f09-3ff197d477cd\u0027)\ndef test_volume_migration(self):\n        self._test_volume_migration(cls.backend_source, cls.backend_dest)\n        if CONF.volume.full_migration_test:\n                  self._test_volume_migration(cls.backend_dest, cls.backend_source)","commit_id":"062649015efd4ef9567fc58715c9288fd2596292"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"dd15a40b140a4daa163285a682c481978c8f0ba1","unresolved":false,"context_lines":[{"line_number":129,"context_line":"    @test.idempotent_id(\u0027b1a41f3f-9dad-493e-9f09-3ff197d477ce\u0027)"},{"line_number":130,"context_line":"    def test_volume_migration_lvm_to_storwize(self):"},{"line_number":131,"context_line":"        backend_source \u003d \u0027lvm\u0027"},{"line_number":132,"context_line":"        backend_dest \u003d \u0027storwize\u0027"},{"line_number":133,"context_line":"        self._test_volume_migration(backend_source, backend_dest)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    @test.idempotent_id(\u0027c1a41f3f-9dad-493e-9f09-3ff197d473ce\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba3cc151_caf2706f","line":132,"in_reply_to":"ba3cc151_c2211b4f","updated":"2015-07-01 01:38:55.000000000","message":"Sweet. It is a better approach. Thank you, Erlon.","commit_id":"062649015efd4ef9567fc58715c9288fd2596292"},{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"a71c1651367edb2b4687347bd59a0dbc845fcd5a","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        super(VolumeMigrationV2Test, cls).skip_checks()"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        if not CONF.volume_feature_enabled.multi_backend:"},{"line_number":47,"context_line":"            raise cls.skipException(\"Cinder multi-backend feature disabled\")"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    @classmethod"},{"line_number":50,"context_line":"    def resource_setup(cls):"}],"source_content_type":"text/x-python","patch_set":21,"id":"3a50d1a3_243e768c","line":47,"updated":"2015-07-22 13:09:17.000000000","message":"Suggest: \"Cinder multi-backend feature is disabled.\"","commit_id":"8779e73090a58e0722a1fe16fe6cc5504184b774"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"f642b3f2cb447268146ba33555553e853ff3f239","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        super(VolumeMigrationV2Test, cls).skip_checks()"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        if not CONF.volume_feature_enabled.multi_backend:"},{"line_number":47,"context_line":"            raise cls.skipException(\"Cinder multi-backend feature disabled\")"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    @classmethod"},{"line_number":50,"context_line":"    def resource_setup(cls):"}],"source_content_type":"text/x-python","patch_set":21,"id":"3a50d1a3_3d59768e","line":47,"in_reply_to":"3a50d1a3_243e768c","updated":"2015-07-29 05:48:48.000000000","message":"Done","commit_id":"8779e73090a58e0722a1fe16fe6cc5504184b774"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"d139dd58d917576ce5db5b920a350b37963dd98e","unresolved":false,"context_lines":[{"line_number":16,"context_line":"configure the local devstack environment with proper back-ends first. With the"},{"line_number":17,"context_line":"current tempest configurations, devstack supports two back-ends most. Assign"},{"line_number":18,"context_line":"each of them to one kind of back-end. Technically, the test case applied to"},{"line_number":19,"context_line":"all kinds of back-end drivers, as long as they support volume migration."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"How to configure the tempest:"},{"line_number":22,"context_line":"* The option backend1_name determines the back-end name of the source."}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_9a6021d1","line":19,"updated":"2015-08-05 14:53:25.000000000","message":"Umm, nothing in tempest is specific to devstack except for that we gate on it in a devstack environment. I do not think it\u0027s appropriate to have this as a docstring since it implies this test is only for devstack.","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"abebbc0b37712a80ea88c3c33e3f1533f77380f1","unresolved":false,"context_lines":[{"line_number":16,"context_line":"configure the local devstack environment with proper back-ends first. With the"},{"line_number":17,"context_line":"current tempest configurations, devstack supports two back-ends most. Assign"},{"line_number":18,"context_line":"each of them to one kind of back-end. Technically, the test case applied to"},{"line_number":19,"context_line":"all kinds of back-end drivers, as long as they support volume migration."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"How to configure the tempest:"},{"line_number":22,"context_line":"* The option backend1_name determines the back-end name of the source."}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_f8835154","line":19,"in_reply_to":"1a4dcd0f_9a6021d1","updated":"2015-08-06 04:58:29.000000000","message":"Thank you for reminding me of it.","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"d139dd58d917576ce5db5b920a350b37963dd98e","unresolved":false,"context_lines":[{"line_number":18,"context_line":"each of them to one kind of back-end. Technically, the test case applied to"},{"line_number":19,"context_line":"all kinds of back-end drivers, as long as they support volume migration."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"How to configure the tempest:"},{"line_number":22,"context_line":"* The option backend1_name determines the back-end name of the source."},{"line_number":23,"context_line":"* The option backend2_name determines the back-end name of the destination."},{"line_number":24,"context_line":"* The option full_migration_test decides if this is a unidirectional"}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_1ae5f1da","line":21,"updated":"2015-08-05 14:53:25.000000000","message":"This is not how you configure tempest, it\u0027s the just the 3 options that are explicitly being used during the test.","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"abebbc0b37712a80ea88c3c33e3f1533f77380f1","unresolved":false,"context_lines":[{"line_number":18,"context_line":"each of them to one kind of back-end. Technically, the test case applied to"},{"line_number":19,"context_line":"all kinds of back-end drivers, as long as they support volume migration."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"How to configure the tempest:"},{"line_number":22,"context_line":"* The option backend1_name determines the back-end name of the source."},{"line_number":23,"context_line":"* The option backend2_name determines the back-end name of the destination."},{"line_number":24,"context_line":"* The option full_migration_test decides if this is a unidirectional"}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_b8794984","line":21,"in_reply_to":"1a4dcd0f_1ae5f1da","updated":"2015-08-06 04:58:29.000000000","message":"Agree will change it.","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"d139dd58d917576ce5db5b920a350b37963dd98e","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        vol_name \u003d data_utils.rand_name(\u0027Volume\u0027)"},{"line_number":72,"context_line":"        params \u003d {\u0027size\u0027: 1, \u0027display_name\u0027: vol_name,"},{"line_number":73,"context_line":"                  \u0027volume_type\u0027: type_id}"},{"line_number":74,"context_line":"        self.volume \u003d self.admin_volume_client.create_volume(**params)"},{"line_number":75,"context_line":"        self.admin_volume_client.wait_for_volume_status("},{"line_number":76,"context_line":"            self.volume[\u0027id\u0027], \u0027available\u0027)"},{"line_number":77,"context_line":"        return self.volume[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_ba1b9dc9","line":74,"updated":"2015-08-05 14:53:25.000000000","message":"why are you using self.volume here. Nothing about this is shared between methods, especially since you do a return at the bottom. Using self here will just cause issues down the road.","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"abebbc0b37712a80ea88c3c33e3f1533f77380f1","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        vol_name \u003d data_utils.rand_name(\u0027Volume\u0027)"},{"line_number":72,"context_line":"        params \u003d {\u0027size\u0027: 1, \u0027display_name\u0027: vol_name,"},{"line_number":73,"context_line":"                  \u0027volume_type\u0027: type_id}"},{"line_number":74,"context_line":"        self.volume \u003d self.admin_volume_client.create_volume(**params)"},{"line_number":75,"context_line":"        self.admin_volume_client.wait_for_volume_status("},{"line_number":76,"context_line":"            self.volume[\u0027id\u0027], \u0027available\u0027)"},{"line_number":77,"context_line":"        return self.volume[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_582fe587","line":74,"in_reply_to":"1a4dcd0f_ba1b9dc9","updated":"2015-08-06 04:58:29.000000000","message":"Y this is something I need to change.","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"d139dd58d917576ce5db5b920a350b37963dd98e","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        volume_id \u003d self._create_volume(type_source)"},{"line_number":83,"context_line":"        volume_source \u003d self.admin_volume_client.show_volume(volume_id)"},{"line_number":84,"context_line":"        self.assertEqual(type_source, volume_source[\u0027volume_type\u0027])"},{"line_number":85,"context_line":"        migration_policy \u003d \u0027on-demand\u0027"},{"line_number":86,"context_line":"        self.admin_volume_client.retype_volume("},{"line_number":87,"context_line":"            volume_id, type_dest, migration_policy\u003dmigration_policy)"},{"line_number":88,"context_line":"        # The migration timeout is 1200 seconds, which is enough to"}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_9a2d81aa","line":85,"updated":"2015-08-05 14:53:25.000000000","message":"Why do we need to set this to a variable? Also what are the other values for the migration policy and why do we only use on-demand? Are there cases where something else needs to be used?","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"abebbc0b37712a80ea88c3c33e3f1533f77380f1","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        volume_id \u003d self._create_volume(type_source)"},{"line_number":83,"context_line":"        volume_source \u003d self.admin_volume_client.show_volume(volume_id)"},{"line_number":84,"context_line":"        self.assertEqual(type_source, volume_source[\u0027volume_type\u0027])"},{"line_number":85,"context_line":"        migration_policy \u003d \u0027on-demand\u0027"},{"line_number":86,"context_line":"        self.admin_volume_client.retype_volume("},{"line_number":87,"context_line":"            volume_id, type_dest, migration_policy\u003dmigration_policy)"},{"line_number":88,"context_line":"        # The migration timeout is 1200 seconds, which is enough to"}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_d82215a3","line":85,"in_reply_to":"1a4dcd0f_9a2d81aa","updated":"2015-08-06 04:58:29.000000000","message":"With the current configuration, this is the only possible to test the REAL migration in terms of retyping. If the migration policy is not \"no-demand\", the volume migration will not be allowed during retyping.","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"d139dd58d917576ce5db5b920a350b37963dd98e","unresolved":false,"context_lines":[{"line_number":86,"context_line":"        self.admin_volume_client.retype_volume("},{"line_number":87,"context_line":"            volume_id, type_dest, migration_policy\u003dmigration_policy)"},{"line_number":88,"context_line":"        # The migration timeout is 1200 seconds, which is enough to"},{"line_number":89,"context_line":"        # migrate the volume with the default 1G."},{"line_number":90,"context_line":"        self.admin_volume_client.wait_for_volume_status("},{"line_number":91,"context_line":"            self.volume[\u0027id\u0027], \u0027available\u0027)"},{"line_number":92,"context_line":"        volume_dest \u003d self.admin_volume_client.show_volume(volume_id)"}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_baa5dd47","line":89,"updated":"2015-08-05 14:53:25.000000000","message":"Umm, isn\u0027t this just using a config value? It\u0027s hardly always going to be 1200 secs (which is way too large a value to run in the gate) it\u0027s also not going to be consistent between all backends and environments. (that\u0027s why the volume timeout is configurable in the first place)","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"d139dd58d917576ce5db5b920a350b37963dd98e","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                         volume_dest[\u0027os-vol-mig-status-attr:migstat\u0027])"},{"line_number":109,"context_line":"        self.assertNotEqual(volume_source[\u0027os-vol-host-attr:host\u0027],"},{"line_number":110,"context_line":"                            volume_dest[\u0027os-vol-host-attr:host\u0027])"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"        self.admin_volume_client.delete_volume(volume_id)"},{"line_number":113,"context_line":"        self.admin_volume_client.wait_for_resource_deletion(volume_id)"},{"line_number":114,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_852d2697","line":111,"updated":"2015-08-05 14:53:25.000000000","message":"Does doing ssh validation to verify the volume has the same contents pre and post migration make sense here? That might make it more of a scenario test if we add that though.","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"abebbc0b37712a80ea88c3c33e3f1533f77380f1","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                         volume_dest[\u0027os-vol-mig-status-attr:migstat\u0027])"},{"line_number":109,"context_line":"        self.assertNotEqual(volume_source[\u0027os-vol-host-attr:host\u0027],"},{"line_number":110,"context_line":"                            volume_dest[\u0027os-vol-host-attr:host\u0027])"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"        self.admin_volume_client.delete_volume(volume_id)"},{"line_number":113,"context_line":"        self.admin_volume_client.wait_for_resource_deletion(volume_id)"},{"line_number":114,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_9867ade8","line":111,"in_reply_to":"1a4dcd0f_852d2697","updated":"2015-08-06 04:58:29.000000000","message":"I am not sure I understand the suggestions here.\nDo you ssh the back-end storage?? It is possible to do it for drivers like storwize, emc, hp, etc, enterprise storage, but I am doubting that this is applicable to LVM or any other shared storage.\nThis method is looking for the general way to test all the drivers.","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"d139dd58d917576ce5db5b920a350b37963dd98e","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        self.volume_types_client.delete_volume_type(type_id_dest)"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    @test.idempotent_id(\u0027a1a41f3f-9dad-493e-9f09-3ff197d477cd\u0027)"},{"line_number":119,"context_line":"    def test_volume_migration(self):"},{"line_number":120,"context_line":"        self._test_volume_migration(self.backend_source, self.backend_dest)"},{"line_number":121,"context_line":"        if self.full_migration_test:"},{"line_number":122,"context_line":"            self._test_volume_migration(self.backend_dest, self.backend_source)"}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_9a04a118","line":119,"updated":"2015-08-05 14:53:25.000000000","message":"I think we need to tag this test as slow, all of your comments imply that this will take a long time to run","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"abebbc0b37712a80ea88c3c33e3f1533f77380f1","unresolved":false,"context_lines":[{"line_number":116,"context_line":"        self.volume_types_client.delete_volume_type(type_id_dest)"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    @test.idempotent_id(\u0027a1a41f3f-9dad-493e-9f09-3ff197d477cd\u0027)"},{"line_number":119,"context_line":"    def test_volume_migration(self):"},{"line_number":120,"context_line":"        self._test_volume_migration(self.backend_source, self.backend_dest)"},{"line_number":121,"context_line":"        if self.full_migration_test:"},{"line_number":122,"context_line":"            self._test_volume_migration(self.backend_dest, self.backend_source)"}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_ffa82022","line":119,"in_reply_to":"1a4dcd0f_9a04a118","updated":"2015-08-06 04:58:29.000000000","message":"Thx so much for the tag.","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"d29a256fec0f80f773818742ea885ad75a5ed33b","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        extra_specs \u003d {spec_key_without_prefix: backend_name}"},{"line_number":52,"context_line":"        type \u003d self.volume_types_client.create_volume_type("},{"line_number":53,"context_line":"            type_name, extra_specs\u003dextra_specs)"},{"line_number":54,"context_line":"        return type[\u0027id\u0027], type_name"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def _create_volume(self, type_id):"},{"line_number":57,"context_line":"        vol_name \u003d data_utils.rand_name(\u0027Volume\u0027)"}],"source_content_type":"text/x-python","patch_set":31,"id":"da20952f_af6cffd0","line":54,"updated":"2015-09-09 20:18:28.000000000","message":"type has no \u0027id\u0027 attribute","commit_id":"f815bcab0c1098ecf6d1639f3fae390bec89e1d6"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"8a1563aa87570b5e75af6792f35e21a351db758e","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    def skip_checks(cls):"},{"line_number":30,"context_line":"        super(VolumeMigrationV2Test, cls).skip_checks()"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        if not CONF.volume_feature_enabled.multi_backend:"},{"line_number":33,"context_line":"            raise cls.skipException(\"Cinder multi-backend feature is \""},{"line_number":34,"context_line":"                                    \"disabled.\")"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"3a29b11f_ba5bc52d","line":32,"updated":"2015-10-20 13:17:39.000000000","message":"Is that all that\u0027s required ?\n\nEvery Cinder volume drivers support volume migration ? If not you need to add a feature flag.","commit_id":"1b33cc88e0e40890fe7e64dff47c69d0d3580347"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"b400c3378f43e24cb7f8b16daae3a85468421c6d","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    def skip_checks(cls):"},{"line_number":30,"context_line":"        super(VolumeMigrationV2Test, cls).skip_checks()"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"        if not CONF.volume_feature_enabled.multi_backend:"},{"line_number":33,"context_line":"            raise cls.skipException(\"Cinder multi-backend feature is \""},{"line_number":34,"context_line":"                                    \"disabled.\")"},{"line_number":35,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"3a29b11f_ae18481f","line":32,"in_reply_to":"3a29b11f_ba5bc52d","updated":"2015-10-21 07:00:03.000000000","message":"Yes, I think so.\n\nIt is true that so many drivers in Cinder may form a great deal of combinations to migrate back and force. We do not know exactly migration from which to which will succeed and migration from which to which will fail.\n\nThis patch is going to test if the migration works from one back-end to another to give the users the general idea about which kind of back-end they will choose.\n\nIf the user is wondering if migration from back-end A to back-end B works, he can use this patch to test to verify if it is real for further decisions.","commit_id":"1b33cc88e0e40890fe7e64dff47c69d0d3580347"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"8a1563aa87570b5e75af6792f35e21a351db758e","unresolved":false,"context_lines":[{"line_number":38,"context_line":"        super(VolumeMigrationV2Test, cls).resource_setup()"},{"line_number":39,"context_line":"        cls.backend_source \u003d CONF.volume.backend1_name"},{"line_number":40,"context_line":"        cls.backend_dest \u003d CONF.volume.backend2_name"},{"line_number":41,"context_line":"        cls.build_timeout \u003d CONF.volume.build_timeout"},{"line_number":42,"context_line":"        cls.build_interval \u003d CONF.volume.build_interval"},{"line_number":43,"context_line":"        cls.admin_volume_client.build_timeout \u003d cls.build_timeout"},{"line_number":44,"context_line":"        cls.admin_volume_client.build_interval \u003d cls.build_interval"}],"source_content_type":"text/x-python","patch_set":39,"id":"3a29b11f_faedbd25","line":41,"updated":"2015-10-20 13:17:39.000000000","message":"Why do you need those 4 \"build_timeout\" variables ?","commit_id":"1b33cc88e0e40890fe7e64dff47c69d0d3580347"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"b400c3378f43e24cb7f8b16daae3a85468421c6d","unresolved":false,"context_lines":[{"line_number":38,"context_line":"        super(VolumeMigrationV2Test, cls).resource_setup()"},{"line_number":39,"context_line":"        cls.backend_source \u003d CONF.volume.backend1_name"},{"line_number":40,"context_line":"        cls.backend_dest \u003d CONF.volume.backend2_name"},{"line_number":41,"context_line":"        cls.build_timeout \u003d CONF.volume.build_timeout"},{"line_number":42,"context_line":"        cls.build_interval \u003d CONF.volume.build_interval"},{"line_number":43,"context_line":"        cls.admin_volume_client.build_timeout \u003d cls.build_timeout"},{"line_number":44,"context_line":"        cls.admin_volume_client.build_interval \u003d cls.build_interval"}],"source_content_type":"text/x-python","patch_set":39,"id":"3a29b11f_ebd39202","line":41,"in_reply_to":"3a29b11f_faedbd25","updated":"2015-10-21 07:00:03.000000000","message":"Right, I forgot to remove them.","commit_id":"1b33cc88e0e40890fe7e64dff47c69d0d3580347"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"8a1563aa87570b5e75af6792f35e21a351db758e","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        cls.admin_volume_client.build_timeout \u003d cls.build_timeout"},{"line_number":44,"context_line":"        cls.admin_volume_client.build_interval \u003d cls.build_interval"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def _create_type(self, backend_name):"},{"line_number":47,"context_line":"        # Volume type creation"},{"line_number":48,"context_line":"        type_name \u003d data_utils.rand_name(\u0027Type\u0027)"},{"line_number":49,"context_line":"        spec_key_without_prefix \u003d \"volume_backend_name\""}],"source_content_type":"text/x-python","patch_set":39,"id":"3a29b11f_5ac6319b","line":46,"updated":"2015-10-20 13:17:39.000000000","message":"Please do not duplicate methods. There\u0027s already a create_type method somewhere.","commit_id":"1b33cc88e0e40890fe7e64dff47c69d0d3580347"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"8a1563aa87570b5e75af6792f35e21a351db758e","unresolved":false,"context_lines":[{"line_number":52,"context_line":"            type_name, extra_specs\u003dextra_specs)[\u0027volume_type\u0027]"},{"line_number":53,"context_line":"        return type[\u0027id\u0027], type_name"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def _create_volume(self, type_id):"},{"line_number":56,"context_line":"        vol_name \u003d data_utils.rand_name(\u0027Volume\u0027)"},{"line_number":57,"context_line":"        params \u003d {\u0027size\u0027: 1, \u0027display_name\u0027: vol_name,"},{"line_number":58,"context_line":"                  \u0027volume_type\u0027: type_id}"}],"source_content_type":"text/x-python","patch_set":39,"id":"3a29b11f_baaf4593","line":55,"updated":"2015-10-20 13:17:39.000000000","message":"Please have a look at _create_type_and_volume from tempest/api/volume/admin/test_multi_backend.py and try to reuse that method.","commit_id":"1b33cc88e0e40890fe7e64dff47c69d0d3580347"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"8a1563aa87570b5e75af6792f35e21a351db758e","unresolved":false,"context_lines":[{"line_number":78,"context_line":"            self.admin_volume_client.show_volume(volume_id)[\u0027volume\u0027])"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        # Check the volume information after the migration."},{"line_number":81,"context_line":"        self.assertEqual(type_dest, volume_dest[\u0027volume_type\u0027])"},{"line_number":82,"context_line":"        self.assertEqual(volume_source[\u0027id\u0027],"},{"line_number":83,"context_line":"                         volume_dest[\u0027id\u0027])"},{"line_number":84,"context_line":"        self.assertEqual(volume_source[\u0027size\u0027],"}],"source_content_type":"text/x-python","patch_set":39,"id":"3a29b11f_dab7a1d6","line":81,"updated":"2015-10-20 13:17:39.000000000","message":"Use a for loop for all these assertequal calls.","commit_id":"1b33cc88e0e40890fe7e64dff47c69d0d3580347"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"8a1563aa87570b5e75af6792f35e21a351db758e","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        self.admin_volume_client.delete_volume(volume_id)"},{"line_number":100,"context_line":"        self.admin_volume_client.wait_for_resource_deletion(volume_id)"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        self.volume_types_client.delete_volume_type(type_id)"},{"line_number":103,"context_line":"        self.volume_types_client.delete_volume_type(type_id_dest)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    @test.idempotent_id(\u0027a1a41f3f-9dad-493e-9f09-3ff197d477cd\u0027)"}],"source_content_type":"text/x-python","patch_set":39,"id":"3a29b11f_7a3c4d2f","line":102,"range":{"start_line":102,"start_character":38,"end_line":102,"end_character":55},"updated":"2015-10-20 13:17:39.000000000","message":"Please use \u0027addCleanup\u0027 for this.","commit_id":"1b33cc88e0e40890fe7e64dff47c69d0d3580347"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"06b2531fa97ba445afa5b44658d3ef45a70ba466","unresolved":false,"context_lines":[{"line_number":22,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class VolumeMigrationV2Test(base.BaseVolumeAdminTest):"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    SOURCE_INDEX \u003d 0"},{"line_number":28,"context_line":"    DESTINATION_INDEX \u003d 1"}],"source_content_type":"text/x-python","patch_set":43,"id":"ba8a016a_3e826e53","line":25,"updated":"2015-11-23 11:50:12.000000000","message":"-1: This is not a migration test, this is a retype with migration test.","commit_id":"9a0980c9edcf94b691f5145edae4f810a5e193fe"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"06b2531fa97ba445afa5b44658d3ef45a70ba466","unresolved":false,"context_lines":[{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    @test.idempotent_id(\u0027a1a41f3f-9dad-493e-9f09-3ff197d477cd\u0027)"},{"line_number":96,"context_line":"    @test.attr(type\u003d\u0027slow\u0027)"},{"line_number":97,"context_line":"    def test_volume_migration(self):"},{"line_number":98,"context_line":"        volume_id \u003d self.volume_id_list[self.SOURCE_INDEX]"},{"line_number":99,"context_line":"        type_dest \u003d self.volume_type_id_list[self.DESTINATION_INDEX]"},{"line_number":100,"context_line":"        self._test_volume_migration(volume_id, type_dest)"}],"source_content_type":"text/x-python","patch_set":43,"id":"ba8a016a_be4bbe2a","line":97,"updated":"2015-11-23 11:50:12.000000000","message":"nit: include in the name `available` to specify the type of retype+migration we are testing.","commit_id":"9a0980c9edcf94b691f5145edae4f810a5e193fe"}],"tempest/api/volume/admin/test_volume_retype_with_migration.py":[{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"4c362833758d3c25f6fe4be3a6eafb63e2d777c5","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class VolumeRetypeWithMigrationV2Test(base.BaseVolumeAdminTest):"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    SOURCE_INDEX \u003d 0"},{"line_number":28,"context_line":"    DESTINATION_INDEX \u003d 1"},{"line_number":29,"context_line":"    KEYS_WITH_NO_CHANGE \u003d (\u0027id\u0027, \u0027size\u0027, \u0027description\u0027,"},{"line_number":30,"context_line":"                           \u0027name\u0027, \u0027user_id\u0027, \u0027os-vol-tenant-attr:tenant_id\u0027)"}],"source_content_type":"text/x-python","patch_set":44,"id":"9a8ffd7b_829e9a52","line":27,"updated":"2015-12-01 17:23:21.000000000","message":"nit: I\u0027m not really sure why you made these instance variables. They aren\u0027t shared across methods they\u0027re just used locally in the test method. Also, it\u0027s just 0 and 1 that are static and don\u0027t change.\n\nPersonally I\u0027d prefer that you just set source_index and dest_index in the test directly","commit_id":"055763728fd5a86d7837ec0486613dd1984628ef"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"5d8ed1267858b30579c9a17a802bcdff13a16638","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class VolumeRetypeWithMigrationV2Test(base.BaseVolumeAdminTest):"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    SOURCE_INDEX \u003d 0"},{"line_number":28,"context_line":"    DESTINATION_INDEX \u003d 1"},{"line_number":29,"context_line":"    KEYS_WITH_NO_CHANGE \u003d (\u0027id\u0027, \u0027size\u0027, \u0027description\u0027,"},{"line_number":30,"context_line":"                           \u0027name\u0027, \u0027user_id\u0027, \u0027os-vol-tenant-attr:tenant_id\u0027)"}],"source_content_type":"text/x-python","patch_set":44,"id":"9a8ffd7b_d103f3b6","line":27,"in_reply_to":"9a8ffd7b_829e9a52","updated":"2015-12-02 04:31:13.000000000","message":"Done","commit_id":"055763728fd5a86d7837ec0486613dd1984628ef"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"4c362833758d3c25f6fe4be3a6eafb63e2d777c5","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":47,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        cls._create_type_and_volume(cls.backend_source)"},{"line_number":50,"context_line":"        cls._create_type_and_volume(cls.backend_dest)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":44,"id":"9a8ffd7b_222faea2","line":49,"updated":"2015-12-01 17:23:21.000000000","message":"So I\u0027m not sure you want to do this here. resource_setup is run as part of setUpClass which makes it a bit more fragile. There is only 1 test method here do we really need to do this in the setUpClass. Moving the volume and type creation to inside the test call will enable you to use addCleanup which is a bit more resilient in failure conditions. It will also makes make the test execution time include the resource creation.\n\nThe primary reason real resources would be created in resource_setup() is to enable sharing them across tests in a class. Since that\u0027s not a concern here (since there is 1 test) there isn\u0027t a reason I can see to do this in resource setup.","commit_id":"055763728fd5a86d7837ec0486613dd1984628ef"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"4c362833758d3c25f6fe4be3a6eafb63e2d777c5","unresolved":false,"context_lines":[{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        volume_id \u003d self.volume_id_list[self.DESTINATION_INDEX]"},{"line_number":103,"context_line":"        type_dest \u003d self.volume_type_id_list[self.SOURCE_INDEX]"},{"line_number":104,"context_line":"        self._test_volume_retype_with_migration(volume_id, type_dest)"}],"source_content_type":"text/x-python","patch_set":44,"id":"9a8ffd7b_6d2127fc","line":104,"updated":"2015-12-01 17:23:21.000000000","message":"Is it really necessary to test that you can go from backend 1 to backend 2 and then another migration from backend 2 to backend 1? This seems like it\u0027s testing the same exact mechanism twice, for very little benefit, at the cost of a much greater runtime","commit_id":"055763728fd5a86d7837ec0486613dd1984628ef"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"5d8ed1267858b30579c9a17a802bcdff13a16638","unresolved":false,"context_lines":[{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        volume_id \u003d self.volume_id_list[self.DESTINATION_INDEX]"},{"line_number":103,"context_line":"        type_dest \u003d self.volume_type_id_list[self.SOURCE_INDEX]"},{"line_number":104,"context_line":"        self._test_volume_retype_with_migration(volume_id, type_dest)"}],"source_content_type":"text/x-python","patch_set":44,"id":"9a8ffd7b_31f59fca","line":104,"in_reply_to":"9a8ffd7b_6d2127fc","updated":"2015-12-02 04:31:13.000000000","message":"The answer is yes.\nIn Cinder, migrating from back-end A to back-end B is not 100% equal to migrating from back-end B to back-end A. Most of the cases, if the volume is migrated successfully from A to B, it is also successful from B to A. However, there are exceptions, we have to check if it works from B to A, even if it already succeeded from A to B.","commit_id":"055763728fd5a86d7837ec0486613dd1984628ef"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"7445219f0c90f037323a22da7d5efe96ec1fb998","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    def resource_setup(cls):"},{"line_number":40,"context_line":"        super(VolumeRetypeWithMigrationV2Test, cls).resource_setup()"},{"line_number":41,"context_line":"        cls.name_field \u003d cls.special_fields[\u0027name_field\u0027]"},{"line_number":42,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":43,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":48,"id":"5a710552_ab7e41e2","line":42,"range":{"start_line":42,"start_character":15,"end_line":42,"end_character":25},"updated":"2015-12-15 09:31:24.000000000","message":"You should use the addCleanup() approach here: create the resources in the test itself (not in resource_setup) and call self.addCleanup(...) after each resource creation.","commit_id":"d137c44029743a952c017dfa10be3a71ae34501f"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"4f629fca4b5043eb66c526f29b0fd57f586ef27d","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    def resource_setup(cls):"},{"line_number":40,"context_line":"        super(VolumeRetypeWithMigrationV2Test, cls).resource_setup()"},{"line_number":41,"context_line":"        cls.name_field \u003d cls.special_fields[\u0027name_field\u0027]"},{"line_number":42,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":43,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":48,"id":"5a710552_f2bac222","line":42,"in_reply_to":"5a710552_ab7e41e2","updated":"2015-12-16 07:22:13.000000000","message":"Done","commit_id":"d137c44029743a952c017dfa10be3a71ae34501f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"99eb3305d96d100194e0281551ea51672c0e6c5c","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        backend_source \u003d CONF.volume.backend1_name"},{"line_number":68,"context_line":"        backend_dest \u003d CONF.volume.backend2_name"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        source_volume_type \u003d self.create_type(backend_source)"},{"line_number":71,"context_line":"        self.addCleanup(self.volume_types_client.delete_volume_type,"},{"line_number":72,"context_line":"                        source_volume_type[\u0027id\u0027])"},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":50,"id":"5a710552_63bd8f67","line":70,"updated":"2015-12-16 14:10:30.000000000","message":"-1: Why not do this in the _create_type_and_volume method you had before?  You create resources there and add the cleanup as well, and return the volume_type and volume. \n\n  source_volume_type, source_volume \u003d self._create_type_and_volume(backend_source)\n  dest_volume_type, dest_volume \u003d self._create_type_and_volume(backend_dest)\n\n  self._test_volume_retype_with_migration(source_volume[\u0027id\u0027], dest_volume_type[\u0027id\u0027])\n  self._test_volume_retype_with_migration(dest_volume[\u0027id\u0027], source_volume_type[\u0027id\u0027]) \n\nAll this resource creation code in the test makes it harder to read and code is redundant.","commit_id":"d57f8f6f05591fe0dab6e592baa848af8d4f430d"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"bd12471f2fd3313f117eda67b6a0f56fe8014c4e","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        backend_source \u003d CONF.volume.backend1_name"},{"line_number":68,"context_line":"        backend_dest \u003d CONF.volume.backend2_name"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        source_volume_type \u003d self.create_type(backend_source)"},{"line_number":71,"context_line":"        self.addCleanup(self.volume_types_client.delete_volume_type,"},{"line_number":72,"context_line":"                        source_volume_type[\u0027id\u0027])"},{"line_number":73,"context_line":""}],"source_content_type":"text/x-python","patch_set":50,"id":"5a710552_56ea9310","line":70,"in_reply_to":"5a710552_63bd8f67","updated":"2015-12-16 14:37:43.000000000","message":"Quite frankly that\u0027s just your opinion on the way the code should be read, and is probably a good rule of thumb for other places. But, test code is a bit different and being explicit is a good thing. In general this approach is much better for tests. Being explicit in what they\u0027re creating like this makes debugging in the failure case much simpler,  especially for people coming looking at it for the first time.(or after a long time) Having to jump around to figure out what test is trying to do (and whether it\u0027s doing it correctly) makes debugging failures much more difficult. \n\nSure, it\u0027s a bit more verbose and similar calls are made in other test methods,  but it\u0027s much easier to read and understand what the test is doing from a single place. That\u0027s what really matters when you need to debug failures. Not that you went to the greatest lengths to deduplicate code with other test methods in a different class.\n\nHelper methods are really only needed if there is a shared setup phase between multiple tests in a single class, which is not the case here. Or if there are common setup bits that are nominally irrelevant to the test method, (like network creation and attachment to a server)  which is also not the case here. For example, a good comment here would be to make a delete helper functions to call add cleanup on that integrated wait and delete in a single place. That would half the add cleanup calls being made.","commit_id":"d57f8f6f05591fe0dab6e592baa848af8d4f430d"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"d0e386c897b89b130f64dfb8129d9d7661d5d893","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class VolumeRetypeWithMigrationV2Test(base.BaseVolumeAdminTest):"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    KEYS_WITH_NO_CHANGE \u003d (\u0027id\u0027, \u0027size\u0027, \u0027description\u0027,"},{"line_number":29,"context_line":"                           \u0027name\u0027, \u0027user_id\u0027, \u0027os-vol-tenant-attr:tenant_id\u0027)"},{"line_number":30,"context_line":"    KEYS_WITH_CHANGE \u003d (\u0027volume_type\u0027, \u0027os-vol-host-attr:host\u0027)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    @classmethod"},{"line_number":33,"context_line":"    def skip_checks(cls):"}],"source_content_type":"text/x-python","patch_set":67,"id":"ba5da102_279fafb5","line":30,"range":{"start_line":28,"start_character":0,"end_line":30,"end_character":63},"updated":"2016-11-04 17:30:06.000000000","message":"nit: it is better to put these values into _test_volume_retype_with_migration because the method only uses them.","commit_id":"2d54574e64ee7a0be5a7a81e8e1f5f0035f995d4"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"a0fffa0efacd251d22c880b4008415ee19d89080","unresolved":false,"context_lines":[{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class VolumeRetypeWithMigrationV2Test(base.BaseVolumeAdminTest):"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    KEYS_WITH_NO_CHANGE \u003d (\u0027id\u0027, \u0027size\u0027, \u0027description\u0027,"},{"line_number":29,"context_line":"                           \u0027name\u0027, \u0027user_id\u0027, \u0027os-vol-tenant-attr:tenant_id\u0027)"},{"line_number":30,"context_line":"    KEYS_WITH_CHANGE \u003d (\u0027volume_type\u0027, \u0027os-vol-host-attr:host\u0027)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    @classmethod"},{"line_number":33,"context_line":"    def skip_checks(cls):"}],"source_content_type":"text/x-python","patch_set":67,"id":"9a629dbe_c3035975","line":30,"range":{"start_line":28,"start_character":0,"end_line":30,"end_character":63},"in_reply_to":"ba5da102_279fafb5","updated":"2016-11-07 14:42:40.000000000","message":"Done","commit_id":"2d54574e64ee7a0be5a7a81e8e1f5f0035f995d4"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"d0e386c897b89b130f64dfb8129d9d7661d5d893","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    def _test_volume_retype_with_migration(self, volume_id, type_dest_id):"},{"line_number":44,"context_line":"        volume_source \u003d ("},{"line_number":45,"context_line":"            self.admin_volume_client.show_volume(volume_id)[\u0027volume\u0027])"},{"line_number":46,"context_line":"        migration_policy \u003d \u0027on-demand\u0027"},{"line_number":47,"context_line":"        self.admin_volume_client.retype_volume("},{"line_number":48,"context_line":"            volume_id, new_type\u003dtype_dest_id,"},{"line_number":49,"context_line":"            migration_policy\u003dmigration_policy)"}],"source_content_type":"text/x-python","patch_set":67,"id":"ba5da102_c74a7355","line":46,"range":{"start_line":46,"start_character":8,"end_line":46,"end_character":38},"updated":"2016-11-04 17:30:06.000000000","message":"nit: enough to specify \"on-demand\" directly on the following retype_volume() call because the argument name is the same.","commit_id":"2d54574e64ee7a0be5a7a81e8e1f5f0035f995d4"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"a0fffa0efacd251d22c880b4008415ee19d89080","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    def _test_volume_retype_with_migration(self, volume_id, type_dest_id):"},{"line_number":44,"context_line":"        volume_source \u003d ("},{"line_number":45,"context_line":"            self.admin_volume_client.show_volume(volume_id)[\u0027volume\u0027])"},{"line_number":46,"context_line":"        migration_policy \u003d \u0027on-demand\u0027"},{"line_number":47,"context_line":"        self.admin_volume_client.retype_volume("},{"line_number":48,"context_line":"            volume_id, new_type\u003dtype_dest_id,"},{"line_number":49,"context_line":"            migration_policy\u003dmigration_policy)"}],"source_content_type":"text/x-python","patch_set":67,"id":"9a629dbe_c32a79e7","line":46,"range":{"start_line":46,"start_character":8,"end_line":46,"end_character":38},"in_reply_to":"ba5da102_c74a7355","updated":"2016-11-07 14:42:40.000000000","message":"Done","commit_id":"2d54574e64ee7a0be5a7a81e8e1f5f0035f995d4"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"63444c9846d3aaa5b86d11c3566cc756b17214b0","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        for volume in cls.volumes:"},{"line_number":49,"context_line":"            # This list should return 2 volumes until the copy and cleanup"},{"line_number":50,"context_line":"            # process is finished."},{"line_number":51,"context_line":"            fetched_list \u003d cls.admin_volume_client.list_volumes("},{"line_number":52,"context_line":"                params\u003d{\u0027all_tenants\u0027: True,"},{"line_number":53,"context_line":"                        \u0027display_name\u0027: volume[\u0027name\u0027]})[\u0027volumes\u0027]"},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":70,"id":"7a77a97e_f561d912","line":51,"updated":"2016-11-21 17:57:45.000000000","message":"That\u0027s so wrong on many level.... First this resource_cleanup method is super compicated. Then list_volume with all_tenants\u003dTrue will completely blow on a \"real cloud\" with thousands of volumes.","commit_id":"2d0d9819bfa59a5c29e64f55b4648cf662dc6a7b"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"a406e88a35da19aff46098953e9d68073dda560b","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        for volume in cls.volumes:"},{"line_number":49,"context_line":"            # This list should return 2 volumes until the copy and cleanup"},{"line_number":50,"context_line":"            # process is finished."},{"line_number":51,"context_line":"            fetched_list \u003d cls.admin_volume_client.list_volumes("},{"line_number":52,"context_line":"                params\u003d{\u0027all_tenants\u0027: True,"},{"line_number":53,"context_line":"                        \u0027display_name\u0027: volume[\u0027name\u0027]})[\u0027volumes\u0027]"},{"line_number":54,"context_line":""}],"source_content_type":"text/x-python","patch_set":70,"id":"7a77a97e_bd7667d6","line":51,"in_reply_to":"7a77a97e_f561d912","updated":"2016-11-21 19:29:31.000000000","message":"That\u0027s  the only solution I could think for the problem I described in the comment above. In the migration process, there\u0027s one volume (the source volume that does not belongs to the tenant anymore, but has the same type we are trying to delete here) that is left behind right after the migration finishes. If we don\u0027t wait Cinder to delete it the test fails when its cleaning up the volume types.\n\n Do you have any other suggestion?","commit_id":"2d0d9819bfa59a5c29e64f55b4648cf662dc6a7b"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"63444c9846d3aaa5b86d11c3566cc756b17214b0","unresolved":false,"context_lines":[{"line_number":90,"context_line":"            self.assertNotEqual(volume_source[key], volume_dest[key])"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    @classmethod"},{"line_number":93,"context_line":"    def _create_volume_from_type(cls, name_field, volume_type\u003dNone):"},{"line_number":94,"context_line":"        type_name \u003d volume_type[\u0027name\u0027] if volume_type else None"},{"line_number":95,"context_line":"        vol_name \u003d data_utils.rand_name(\u0027Volume\u0027)"},{"line_number":96,"context_line":"        params \u003d {name_field: vol_name, \u0027volume_type\u0027: type_name}"}],"source_content_type":"text/x-python","patch_set":70,"id":"7a77a97e_75014939","line":93,"updated":"2016-11-21 17:57:45.000000000","message":"Please delete that method. I already told you on the previous patchset. I don\u0027t want people to blindly copy/paste that method. We made the create_volume() method accept *kwargs argument just so that you can pass extra argument such as \u0027volume_type\u0027. Just use this.","commit_id":"2d0d9819bfa59a5c29e64f55b4648cf662dc6a7b"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"a406e88a35da19aff46098953e9d68073dda560b","unresolved":false,"context_lines":[{"line_number":90,"context_line":"            self.assertNotEqual(volume_source[key], volume_dest[key])"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    @classmethod"},{"line_number":93,"context_line":"    def _create_volume_from_type(cls, name_field, volume_type\u003dNone):"},{"line_number":94,"context_line":"        type_name \u003d volume_type[\u0027name\u0027] if volume_type else None"},{"line_number":95,"context_line":"        vol_name \u003d data_utils.rand_name(\u0027Volume\u0027)"},{"line_number":96,"context_line":"        params \u003d {name_field: vol_name, \u0027volume_type\u0027: type_name}"}],"source_content_type":"text/x-python","patch_set":70,"id":"7a77a97e_a0139a0c","line":93,"in_reply_to":"7a77a97e_75014939","updated":"2016-11-21 19:29:31.000000000","message":"Sorry, missed this one.","commit_id":"2d0d9819bfa59a5c29e64f55b4648cf662dc6a7b"},{"author":{"_account_id":14614,"name":"Kyrylo Romanenko","display_name":"K.Romanenko","email":"romanenko-kv@hotmail.com","username":"kromanenko"},"change_message_id":"472e74e5dd6ec19cf7e860aaafc5048e71c2f938","unresolved":false,"context_lines":[{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    @test.idempotent_id(\u0027a1a41f3f-9dad-493e-9f09-3ff197d477cd\u0027)"},{"line_number":92,"context_line":"    def test_available_volume_retype_with_migration(self):"},{"line_number":93,"context_line":"        # read backend name from a list ."},{"line_number":94,"context_line":"        backend_names \u003d CONF.volume.backend_names"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        backend_src \u003d backend_names[0]"}],"source_content_type":"text/x-python","patch_set":74,"id":"5a74a57a_d398d47a","line":93,"range":{"start_line":93,"start_character":8,"end_line":93,"end_character":41},"updated":"2016-11-28 14:54:41.000000000","message":"nit: docstring with described test steps would look better then just a comment.","commit_id":"d09c782b8b573b2d2d488b73d8aef2ef4c276dba"},{"author":{"_account_id":5689,"name":"Masayuki Igawa","email":"masayuki@igawa.io","username":"igawa"},"change_message_id":"079f2276ad440d9a87ed2db48135d1512b997918","unresolved":false,"context_lines":[{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    @test.idempotent_id(\u0027a1a41f3f-9dad-493e-9f09-3ff197d477cd\u0027)"},{"line_number":92,"context_line":"    def test_available_volume_retype_with_migration(self):"},{"line_number":93,"context_line":"        # read backend name from a list ."},{"line_number":94,"context_line":"        backend_names \u003d CONF.volume.backend_names"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        backend_src \u003d backend_names[0]"}],"source_content_type":"text/x-python","patch_set":74,"id":"5a74a57a_c0c33d37","line":93,"range":{"start_line":93,"start_character":8,"end_line":93,"end_character":41},"in_reply_to":"5a74a57a_d398d47a","updated":"2016-11-30 05:56:14.000000000","message":"+1","commit_id":"d09c782b8b573b2d2d488b73d8aef2ef4c276dba"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"1ef158e976d00e3d1e09d79c515827fd1d02eb0e","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        if not CONF.volume_feature_enabled.multi_backend:"},{"line_number":33,"context_line":"            raise cls.skipException(\"Cinder multi-backend feature disabled.\")"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"        if len(CONF.volume.backend_names) \u003c 2:"},{"line_number":36,"context_line":"            raise cls.skipException(\"Requires at least two different \""},{"line_number":37,"context_line":"                                    \"backend names\")"},{"line_number":38,"context_line":""}],"source_content_type":"text/x-python","patch_set":80,"id":"ba5201f7_d346a9be","line":35,"range":{"start_line":35,"start_character":11,"end_line":35,"end_character":46},"updated":"2016-12-30 14:54:44.000000000","message":"this does not make sure there are 2 diff backend. It should check uniqueness among them. It should be\n\nlen(set(CONF.volume.backend_names))\n\nBut i will not block on this and can be done as followup one.","commit_id":"256911793c75d2abacd72dd9002dbbfd82f50bf6"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"719027a6159c6ab1cd2635e5de2736c73970aa69","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        if not CONF.volume_feature_enabled.multi_backend:"},{"line_number":33,"context_line":"            raise cls.skipException(\"Cinder multi-backend feature disabled.\")"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"        if len(CONF.volume.backend_names) \u003c 2:"},{"line_number":36,"context_line":"            raise cls.skipException(\"Requires at least two different \""},{"line_number":37,"context_line":"                                    \"backend names\")"},{"line_number":38,"context_line":""}],"source_content_type":"text/x-python","patch_set":80,"id":"7a3c09a3_42cc266a","line":35,"range":{"start_line":35,"start_character":11,"end_line":35,"end_character":46},"in_reply_to":"ba5201f7_d346a9be","updated":"2017-01-17 13:27:43.000000000","message":"Done","commit_id":"256911793c75d2abacd72dd9002dbbfd82f50bf6"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"f7e4a29fe38f9c237368ec5755c7d779fcad5adb","unresolved":false,"context_lines":[{"line_number":77,"context_line":"        volume_dest \u003d ("},{"line_number":78,"context_line":"            self.admin_volume_client.show_volume(volume_id)[\u0027volume\u0027])"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        # Check the volume information after the migration."},{"line_number":81,"context_line":"        self.assertEqual(\u0027success\u0027,"},{"line_number":82,"context_line":"                         volume_dest[\u0027os-vol-mig-status-attr:migstat\u0027])"},{"line_number":83,"context_line":"        self.assertEqual(\u0027success\u0027, volume_dest[\u0027migration_status\u0027])"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        for key in keys_with_no_change:"},{"line_number":86,"context_line":"            self.assertEqual(volume_source[key], volume_dest[key])"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        for key in keys_with_change:"},{"line_number":89,"context_line":"            self.assertNotEqual(volume_source[key], volume_dest[key])"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    @test.idempotent_id(\u0027a1a41f3f-9dad-493e-9f09-3ff197d477cd\u0027)"},{"line_number":92,"context_line":"    def test_available_volume_retype_with_migration(self):"}],"source_content_type":"text/x-python","patch_set":80,"id":"ba5201f7_ee874610","line":89,"range":{"start_line":80,"start_character":0,"end_line":89,"end_character":69},"updated":"2016-12-30 15:07:59.000000000","message":"Not sure about time taken for migration but before all these checks we should wait for retype as done in https://review.openstack.org/#/c/326681/17/tempest/common/waiters.py : wait_for_volume_retype","commit_id":"256911793c75d2abacd72dd9002dbbfd82f50bf6"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"719027a6159c6ab1cd2635e5de2736c73970aa69","unresolved":false,"context_lines":[{"line_number":77,"context_line":"        volume_dest \u003d ("},{"line_number":78,"context_line":"            self.admin_volume_client.show_volume(volume_id)[\u0027volume\u0027])"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"        # Check the volume information after the migration."},{"line_number":81,"context_line":"        self.assertEqual(\u0027success\u0027,"},{"line_number":82,"context_line":"                         volume_dest[\u0027os-vol-mig-status-attr:migstat\u0027])"},{"line_number":83,"context_line":"        self.assertEqual(\u0027success\u0027, volume_dest[\u0027migration_status\u0027])"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        for key in keys_with_no_change:"},{"line_number":86,"context_line":"            self.assertEqual(volume_source[key], volume_dest[key])"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        for key in keys_with_change:"},{"line_number":89,"context_line":"            self.assertNotEqual(volume_source[key], volume_dest[key])"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    @test.idempotent_id(\u0027a1a41f3f-9dad-493e-9f09-3ff197d477cd\u0027)"},{"line_number":92,"context_line":"    def test_available_volume_retype_with_migration(self):"}],"source_content_type":"text/x-python","patch_set":80,"id":"7a3c09a3_c2c81625","line":89,"range":{"start_line":80,"start_character":0,"end_line":89,"end_character":69},"in_reply_to":"ba5201f7_ee874610","updated":"2017-01-17 13:27:43.000000000","message":"Hmm, that\u0027s a good idea. Ill use this approach tough the way it is its also waiting to the volume until it gets available.","commit_id":"256911793c75d2abacd72dd9002dbbfd82f50bf6"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"1ef158e976d00e3d1e09d79c515827fd1d02eb0e","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    @test.idempotent_id(\u0027a1a41f3f-9dad-493e-9f09-3ff197d477cd\u0027)"},{"line_number":92,"context_line":"    def test_available_volume_retype_with_migration(self):"},{"line_number":93,"context_line":"        # read backend name from a list ."},{"line_number":94,"context_line":"        backend_names \u003d CONF.volume.backend_names"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        backend_src \u003d backend_names[0]"},{"line_number":97,"context_line":"        backend_dst \u003d backend_names[1]"}],"source_content_type":"text/x-python","patch_set":80,"id":"ba5201f7_53ed598a","line":94,"range":{"start_line":94,"start_character":0,"end_line":94,"end_character":49},"updated":"2016-12-30 14:54:44.000000000","message":"as in skip check we need to get uniqueness of backends so here too we fetch the unique backend.\nOtherwise tests fails if same backend name gets configured in list and hard to debug the mis-configuration from failure.","commit_id":"256911793c75d2abacd72dd9002dbbfd82f50bf6"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"719027a6159c6ab1cd2635e5de2736c73970aa69","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    @test.idempotent_id(\u0027a1a41f3f-9dad-493e-9f09-3ff197d477cd\u0027)"},{"line_number":92,"context_line":"    def test_available_volume_retype_with_migration(self):"},{"line_number":93,"context_line":"        # read backend name from a list ."},{"line_number":94,"context_line":"        backend_names \u003d CONF.volume.backend_names"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        backend_src \u003d backend_names[0]"},{"line_number":97,"context_line":"        backend_dst \u003d backend_names[1]"}],"source_content_type":"text/x-python","patch_set":80,"id":"7a3c09a3_a2014a08","line":94,"range":{"start_line":94,"start_character":0,"end_line":94,"end_character":49},"in_reply_to":"ba5201f7_53ed598a","updated":"2017-01-17 13:27:43.000000000","message":"Actually we only need to check in the skip right? If the check does not pass we don\u0027t get here.","commit_id":"256911793c75d2abacd72dd9002dbbfd82f50bf6"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8e38da374e42af2b9abca2f22f5c991ac7539d91","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    def resource_cleanup(cls):"},{"line_number":41,"context_line":"        # When retyping a volume, Cinder creates an internal volume in the"},{"line_number":42,"context_line":"        # target backend. The volume in the source backend is deleted after"},{"line_number":43,"context_line":"        # the migration, so we need to wait Cinder delete this volume before"},{"line_number":44,"context_line":"        # deleting the types we\u0027ve created."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        volume_ids \u003d [volume[\u0027id\u0027] for volume in cls.volumes]"}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_cfbb6b74","line":43,"range":{"start_line":43,"start_character":44,"end_line":43,"end_character":57},"updated":"2017-01-19 11:01:08.000000000","message":"nit: for Cinder to delete","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"759bbfc9c4d030c1dc7fe94d0099114b4662969a","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    def resource_cleanup(cls):"},{"line_number":41,"context_line":"        # When retyping a volume, Cinder creates an internal volume in the"},{"line_number":42,"context_line":"        # target backend. The volume in the source backend is deleted after"},{"line_number":43,"context_line":"        # the migration, so we need to wait Cinder delete this volume before"},{"line_number":44,"context_line":"        # deleting the types we\u0027ve created."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        volume_ids \u003d [volume[\u0027id\u0027] for volume in cls.volumes]"}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_f562baac","line":43,"range":{"start_line":43,"start_character":44,"end_line":43,"end_character":57},"in_reply_to":"5a3905b3_cfbb6b74","updated":"2017-01-19 16:48:06.000000000","message":"Done","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8e38da374e42af2b9abca2f22f5c991ac7539d91","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        # the migration, so we need to wait Cinder delete this volume before"},{"line_number":44,"context_line":"        # deleting the types we\u0027ve created."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        volume_ids \u003d [volume[\u0027id\u0027] for volume in cls.volumes]"},{"line_number":47,"context_line":"        for volume in cls.volumes:"},{"line_number":48,"context_line":"            # This list should return 2 volumes until the copy and cleanup"},{"line_number":49,"context_line":"            # process is finished."}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_2fbb0f74","line":46,"updated":"2017-01-19 11:01:08.000000000","message":"-1: This is unnecessary","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"759bbfc9c4d030c1dc7fe94d0099114b4662969a","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        # the migration, so we need to wait Cinder delete this volume before"},{"line_number":44,"context_line":"        # deleting the types we\u0027ve created."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        volume_ids \u003d [volume[\u0027id\u0027] for volume in cls.volumes]"},{"line_number":47,"context_line":"        for volume in cls.volumes:"},{"line_number":48,"context_line":"            # This list should return 2 volumes until the copy and cleanup"},{"line_number":49,"context_line":"            # process is finished."}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_90e7c43f","line":46,"in_reply_to":"5a3905b3_2fbb0f74","updated":"2017-01-19 16:48:06.000000000","message":"At this point I don\u0027t have good way to know what was the volume I\u0027v created. Though list will actually have 1 volume, and I do can get it with cls.volumes[0], I think its safer just iterate over the list.","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9b9b80cf3c94953f8c974d1b9e0d35678902f277","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        # the migration, so we need to wait Cinder delete this volume before"},{"line_number":44,"context_line":"        # deleting the types we\u0027ve created."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        volume_ids \u003d [volume[\u0027id\u0027] for volume in cls.volumes]"},{"line_number":47,"context_line":"        for volume in cls.volumes:"},{"line_number":48,"context_line":"            # This list should return 2 volumes until the copy and cleanup"},{"line_number":49,"context_line":"            # process is finished."}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_ce37d6e1","line":46,"in_reply_to":"5a3905b3_90e7c43f","updated":"2017-01-19 17:00:24.000000000","message":"First, it doesn\u0027t matter, you have created both, second, you are fetching the volumes\u0027 information using the name, which is randomly generated, so you\u0027ll only get the volumes you want to check (originial plus the one being deleted after the migration).\n\nSecond, if you do the volume creation in a setup method like I mentioned below you will store the src_volume in a variable and you\u0027ll be able to query it from here directly instead of needing a loop.","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8e38da374e42af2b9abca2f22f5c991ac7539d91","unresolved":false,"context_lines":[{"line_number":52,"context_line":"                        \u0027display_name\u0027: volume[\u0027name\u0027]})[\u0027volumes\u0027]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"            for fetched_vol in fetched_list:"},{"line_number":55,"context_line":"                if fetched_vol[\u0027id\u0027] not in volume_ids:"},{"line_number":56,"context_line":"                    # This is the Cinder internal volume"},{"line_number":57,"context_line":"                    LOG.debug(\u0027Waiting for internal volume %s deletion\u0027,"},{"line_number":58,"context_line":"                              volume[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_afa7ff4d","line":55,"updated":"2017-01-19 11:01:08.000000000","message":"-1: We don\u0027t need to check against all volume_ids we created, it should be the one we are fetching:\n\n if fetched_vol[\u0027id\u0027] !\u003d volume[\u0027id\u0027]:","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"759bbfc9c4d030c1dc7fe94d0099114b4662969a","unresolved":false,"context_lines":[{"line_number":52,"context_line":"                        \u0027display_name\u0027: volume[\u0027name\u0027]})[\u0027volumes\u0027]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"            for fetched_vol in fetched_list:"},{"line_number":55,"context_line":"                if fetched_vol[\u0027id\u0027] not in volume_ids:"},{"line_number":56,"context_line":"                    # This is the Cinder internal volume"},{"line_number":57,"context_line":"                    LOG.debug(\u0027Waiting for internal volume %s deletion\u0027,"},{"line_number":58,"context_line":"                              volume[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_f0d48848","line":55,"in_reply_to":"5a3905b3_afa7ff4d","updated":"2017-01-19 16:48:06.000000000","message":"Same as above.","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9b9b80cf3c94953f8c974d1b9e0d35678902f277","unresolved":false,"context_lines":[{"line_number":52,"context_line":"                        \u0027display_name\u0027: volume[\u0027name\u0027]})[\u0027volumes\u0027]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"            for fetched_vol in fetched_list:"},{"line_number":55,"context_line":"                if fetched_vol[\u0027id\u0027] not in volume_ids:"},{"line_number":56,"context_line":"                    # This is the Cinder internal volume"},{"line_number":57,"context_line":"                    LOG.debug(\u0027Waiting for internal volume %s deletion\u0027,"},{"line_number":58,"context_line":"                              volume[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_ae41aa3b","line":55,"in_reply_to":"5a3905b3_f0d48848","updated":"2017-01-19 17:00:24.000000000","message":"You shouldn\u0027t need this, the code I wrote should work.","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8e38da374e42af2b9abca2f22f5c991ac7539d91","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                    LOG.debug(\u0027Waiting for internal volume %s deletion\u0027,"},{"line_number":58,"context_line":"                              volume[\u0027id\u0027])"},{"line_number":59,"context_line":"                    cls.admin_volume_client.wait_for_resource_deletion("},{"line_number":60,"context_line":"                        fetched_vol[\u0027id\u0027])"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        super(VolumeRetypeWithMigrationV2Test, cls).resource_cleanup()"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_ef674723","line":60,"updated":"2017-01-19 11:01:08.000000000","message":"nit: We should break out of the loop after we\u0027ve found the right volume. It\u0027s not a big thing since worst case we\u0027ll loop once more, but I think it\u0027s the sensible thing to do.","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"759bbfc9c4d030c1dc7fe94d0099114b4662969a","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                    LOG.debug(\u0027Waiting for internal volume %s deletion\u0027,"},{"line_number":58,"context_line":"                              volume[\u0027id\u0027])"},{"line_number":59,"context_line":"                    cls.admin_volume_client.wait_for_resource_deletion("},{"line_number":60,"context_line":"                        fetched_vol[\u0027id\u0027])"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        super(VolumeRetypeWithMigrationV2Test, cls).resource_cleanup()"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_30cff0b1","line":60,"in_reply_to":"5a3905b3_ef674723","updated":"2017-01-19 16:48:06.000000000","message":"Done","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8e38da374e42af2b9abca2f22f5c991ac7539d91","unresolved":false,"context_lines":[{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        volume_source \u003d ("},{"line_number":71,"context_line":"            self.admin_volume_client.show_volume(volume_id)[\u0027volume\u0027])"},{"line_number":72,"context_line":"        self.admin_volume_client.retype_volume("},{"line_number":73,"context_line":"            volume_id, new_type\u003dtype_dest_id,"},{"line_number":74,"context_line":"            migration_policy\u003d\u0027on-demand\u0027)"},{"line_number":75,"context_line":"        waiters.wait_for_volume_retype(self.volumes_client, volume_id,"}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_af7e1fac","line":72,"updated":"2017-01-19 11:01:08.000000000","message":"?: Why do we do the retype with the admin? I think we should retype with the same user as we created the volume to check that the user can do the retype.","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9b9b80cf3c94953f8c974d1b9e0d35678902f277","unresolved":false,"context_lines":[{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        volume_source \u003d ("},{"line_number":71,"context_line":"            self.admin_volume_client.show_volume(volume_id)[\u0027volume\u0027])"},{"line_number":72,"context_line":"        self.admin_volume_client.retype_volume("},{"line_number":73,"context_line":"            volume_id, new_type\u003dtype_dest_id,"},{"line_number":74,"context_line":"            migration_policy\u003d\u0027on-demand\u0027)"},{"line_number":75,"context_line":"        waiters.wait_for_volume_retype(self.volumes_client, volume_id,"}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_8eaa2ed3","line":72,"in_reply_to":"5a3905b3_13d3cb39","updated":"2017-01-19 17:00:24.000000000","message":"Wow, that\u0027s a serious bug!","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"759bbfc9c4d030c1dc7fe94d0099114b4662969a","unresolved":false,"context_lines":[{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        volume_source \u003d ("},{"line_number":71,"context_line":"            self.admin_volume_client.show_volume(volume_id)[\u0027volume\u0027])"},{"line_number":72,"context_line":"        self.admin_volume_client.retype_volume("},{"line_number":73,"context_line":"            volume_id, new_type\u003dtype_dest_id,"},{"line_number":74,"context_line":"            migration_policy\u003d\u0027on-demand\u0027)"},{"line_number":75,"context_line":"        waiters.wait_for_volume_retype(self.volumes_client, volume_id,"}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_13d3cb39","line":72,"in_reply_to":"5a3905b3_af7e1fac","updated":"2017-01-19 16:48:06.000000000","message":"Good point! This was legacy from the first test so, I didn\u0027t noticed. I changed it to run as user, and it pointed to a bug on Cinder[1].\n\n[1] https://bugs.launchpad.net/cinder/+bug/1657806","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8e38da374e42af2b9abca2f22f5c991ac7539d91","unresolved":false,"context_lines":[{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    @test.idempotent_id(\u0027a1a41f3f-9dad-493e-9f09-3ff197d477cd\u0027)"},{"line_number":92,"context_line":"    def test_available_volume_retype_with_migration(self):"},{"line_number":93,"context_line":"        # read backend name from a list ."},{"line_number":94,"context_line":"        backend_names \u003d CONF.volume.backend_names"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        backend_src \u003d backend_names[0]"},{"line_number":97,"context_line":"        backend_dst \u003d backend_names[1]"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        extra_specs_src \u003d {\"volume_backend_name\": backend_src}"},{"line_number":100,"context_line":"        extra_specs_dst \u003d {\"volume_backend_name\": backend_dst}"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        src_volume_type \u003d self.create_volume_type(extra_specs\u003dextra_specs_src)"},{"line_number":103,"context_line":"        dst_volume_type \u003d self.create_volume_type(extra_specs\u003dextra_specs_dst)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        src_volume \u003d self.create_volume(volume_type\u003dsrc_volume_type[\u0027name\u0027])"},{"line_number":106,"context_line":"        self._test_volume_retype_with_migration(src_volume[\u0027id\u0027],"},{"line_number":107,"context_line":"                                                dst_volume_type[\u0027name\u0027])"}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_cf30cb0a","line":105,"range":{"start_line":93,"start_character":0,"end_line":105,"end_character":76},"updated":"2017-01-19 11:01:08.000000000","message":"nit: Shouldn\u0027t we be doing this in the resource_setup method?","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"9b9b80cf3c94953f8c974d1b9e0d35678902f277","unresolved":false,"context_lines":[{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    @test.idempotent_id(\u0027a1a41f3f-9dad-493e-9f09-3ff197d477cd\u0027)"},{"line_number":92,"context_line":"    def test_available_volume_retype_with_migration(self):"},{"line_number":93,"context_line":"        # read backend name from a list ."},{"line_number":94,"context_line":"        backend_names \u003d CONF.volume.backend_names"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        backend_src \u003d backend_names[0]"},{"line_number":97,"context_line":"        backend_dst \u003d backend_names[1]"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        extra_specs_src \u003d {\"volume_backend_name\": backend_src}"},{"line_number":100,"context_line":"        extra_specs_dst \u003d {\"volume_backend_name\": backend_dst}"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        src_volume_type \u003d self.create_volume_type(extra_specs\u003dextra_specs_src)"},{"line_number":103,"context_line":"        dst_volume_type \u003d self.create_volume_type(extra_specs\u003dextra_specs_dst)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        src_volume \u003d self.create_volume(volume_type\u003dsrc_volume_type[\u0027name\u0027])"},{"line_number":106,"context_line":"        self._test_volume_retype_with_migration(src_volume[\u0027id\u0027],"},{"line_number":107,"context_line":"                                                dst_volume_type[\u0027name\u0027])"}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_0ee55e36","line":105,"range":{"start_line":93,"start_character":0,"end_line":105,"end_character":76},"in_reply_to":"5a3905b3_9395fb41","updated":"2017-01-19 17:00:24.000000000","message":"It\u0027s fine, that\u0027s why I only mentioned as a nit, because it\u0027s cleaner to separate the setup from the actual test.","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"759bbfc9c4d030c1dc7fe94d0099114b4662969a","unresolved":false,"context_lines":[{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    @test.idempotent_id(\u0027a1a41f3f-9dad-493e-9f09-3ff197d477cd\u0027)"},{"line_number":92,"context_line":"    def test_available_volume_retype_with_migration(self):"},{"line_number":93,"context_line":"        # read backend name from a list ."},{"line_number":94,"context_line":"        backend_names \u003d CONF.volume.backend_names"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        backend_src \u003d backend_names[0]"},{"line_number":97,"context_line":"        backend_dst \u003d backend_names[1]"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        extra_specs_src \u003d {\"volume_backend_name\": backend_src}"},{"line_number":100,"context_line":"        extra_specs_dst \u003d {\"volume_backend_name\": backend_dst}"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"        src_volume_type \u003d self.create_volume_type(extra_specs\u003dextra_specs_src)"},{"line_number":103,"context_line":"        dst_volume_type \u003d self.create_volume_type(extra_specs\u003dextra_specs_dst)"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        src_volume \u003d self.create_volume(volume_type\u003dsrc_volume_type[\u0027name\u0027])"},{"line_number":106,"context_line":"        self._test_volume_retype_with_migration(src_volume[\u0027id\u0027],"},{"line_number":107,"context_line":"                                                dst_volume_type[\u0027name\u0027])"}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_9395fb41","line":105,"range":{"start_line":93,"start_character":0,"end_line":105,"end_character":76},"in_reply_to":"5a3905b3_cf30cb0a","updated":"2017-01-19 16:48:06.000000000","message":"I believe that\u0027s fine to do it here as this does not repeat over other tests.","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8e38da374e42af2b9abca2f22f5c991ac7539d91","unresolved":false,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        src_volume \u003d self.create_volume(volume_type\u003dsrc_volume_type[\u0027name\u0027])"},{"line_number":106,"context_line":"        self._test_volume_retype_with_migration(src_volume[\u0027id\u0027],"},{"line_number":107,"context_line":"                                                dst_volume_type[\u0027name\u0027])"}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_2f406f56","line":107,"updated":"2017-01-19 11:01:08.000000000","message":"-1: This should be doing it also in the other direction according to the commit message.\n\n self._test_volume_retype_with_migration(src_volume[\u0027id\u0027],\n                                         src_volume_type[\u0027name\u0027])","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"759bbfc9c4d030c1dc7fe94d0099114b4662969a","unresolved":false,"context_lines":[{"line_number":104,"context_line":""},{"line_number":105,"context_line":"        src_volume \u003d self.create_volume(volume_type\u003dsrc_volume_type[\u0027name\u0027])"},{"line_number":106,"context_line":"        self._test_volume_retype_with_migration(src_volume[\u0027id\u0027],"},{"line_number":107,"context_line":"                                                dst_volume_type[\u0027name\u0027])"}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_338d2732","line":107,"in_reply_to":"5a3905b3_2f406f56","updated":"2017-01-19 16:48:06.000000000","message":"Updated the commit message.","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8c3e96a1627bf1bf6e4767cf02c592b68ecaffb5","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        # before deleting the types we\u0027ve created."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        volume_ids \u003d [volume[\u0027id\u0027] for volume in cls.volumes]"},{"line_number":47,"context_line":"        for volume in cls.volumes:"},{"line_number":48,"context_line":"            # This list should return 2 volumes until the copy and cleanup"},{"line_number":49,"context_line":"            # process is finished."},{"line_number":50,"context_line":"            fetched_list \u003d cls.volumes_client.list_volumes("},{"line_number":51,"context_line":"                params\u003d{\u0027all_tenants\u0027: True,"},{"line_number":52,"context_line":"                        \u0027display_name\u0027: volume[\u0027name\u0027]})[\u0027volumes\u0027]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"            for fetched_vol in fetched_list:"},{"line_number":55,"context_line":"                if fetched_vol[\u0027id\u0027] not in volume_ids:"},{"line_number":56,"context_line":"                    # This is the Cinder internal volume"},{"line_number":57,"context_line":"                    LOG.debug(\u0027Waiting for internal volume %s deletion\u0027,"},{"line_number":58,"context_line":"                              volume[\u0027id\u0027])"},{"line_number":59,"context_line":"                    cls.volumes_client.wait_for_resource_deletion("},{"line_number":60,"context_line":"                        fetched_vol[\u0027id\u0027])"},{"line_number":61,"context_line":"                    break"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"        super(VolumeRetypeWithMigrationV2Test, cls).resource_cleanup()"}],"source_content_type":"text/x-python","patch_set":87,"id":"5a3905b3_0ed9de21","line":60,"range":{"start_line":47,"start_character":0,"end_line":60,"end_character":42},"updated":"2017-01-19 17:04:35.000000000","message":"-1: I believe this is more complicated than it is necessary.","commit_id":"1b65b2c88d95445328269f3c134b9ecb189a82f8"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"695f443645778ce55ee560facd67044351cc5083","unresolved":false,"context_lines":[{"line_number":44,"context_line":"        # before deleting the types we\u0027ve created."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        volume_ids \u003d [volume[\u0027id\u0027] for volume in cls.volumes]"},{"line_number":47,"context_line":"        for volume in cls.volumes:"},{"line_number":48,"context_line":"            # This list should return 2 volumes until the copy and cleanup"},{"line_number":49,"context_line":"            # process is finished."},{"line_number":50,"context_line":"            fetched_list \u003d cls.volumes_client.list_volumes("},{"line_number":51,"context_line":"                params\u003d{\u0027all_tenants\u0027: True,"},{"line_number":52,"context_line":"                        \u0027display_name\u0027: volume[\u0027name\u0027]})[\u0027volumes\u0027]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"            for fetched_vol in fetched_list:"},{"line_number":55,"context_line":"                if fetched_vol[\u0027id\u0027] not in volume_ids:"},{"line_number":56,"context_line":"                    # This is the Cinder internal volume"},{"line_number":57,"context_line":"                    LOG.debug(\u0027Waiting for internal volume %s deletion\u0027,"},{"line_number":58,"context_line":"                              volume[\u0027id\u0027])"},{"line_number":59,"context_line":"                    cls.volumes_client.wait_for_resource_deletion("},{"line_number":60,"context_line":"                        fetched_vol[\u0027id\u0027])"},{"line_number":61,"context_line":"                    break"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"        super(VolumeRetypeWithMigrationV2Test, cls).resource_cleanup()"}],"source_content_type":"text/x-python","patch_set":87,"id":"5a3905b3_ff7efeac","line":60,"range":{"start_line":47,"start_character":0,"end_line":60,"end_character":42},"in_reply_to":"5a3905b3_0ed9de21","updated":"2017-01-19 18:06:15.000000000","message":"Done","commit_id":"1b65b2c88d95445328269f3c134b9ecb189a82f8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8c3e96a1627bf1bf6e4767cf02c592b68ecaffb5","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        keys_with_change \u003d (\u0027volume_type\u0027, \u0027os-vol-host-attr:host\u0027)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        volume_source \u003d self.admin_volume_client.show_volume(volume_id)[\u0027volume\u0027]"},{"line_number":72,"context_line":"        self.volumes_client.retype_volume(volume_id, new_type\u003dtype_dest_id,"},{"line_number":73,"context_line":"                                          migration_policy\u003d\u0027on-demand\u0027)"},{"line_number":74,"context_line":"        waiters.wait_for_volume_retype(self.volumes_client, volume_id,"},{"line_number":75,"context_line":"                                       type_dest_id)"}],"source_content_type":"text/x-python","patch_set":87,"id":"5a3905b3_aeca8a4d","line":72,"updated":"2017-01-19 17:04:35.000000000","message":"-1: If the bug you opened [1] is correct then you should do this as an admin and add a TODO here to fix it once the bug is closed.\n\n[1] https://bugs.launchpad.net/cinder/+bug/1657806","commit_id":"1b65b2c88d95445328269f3c134b9ecb189a82f8"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"695f443645778ce55ee560facd67044351cc5083","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        keys_with_change \u003d (\u0027volume_type\u0027, \u0027os-vol-host-attr:host\u0027)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        volume_source \u003d self.admin_volume_client.show_volume(volume_id)[\u0027volume\u0027]"},{"line_number":72,"context_line":"        self.volumes_client.retype_volume(volume_id, new_type\u003dtype_dest_id,"},{"line_number":73,"context_line":"                                          migration_policy\u003d\u0027on-demand\u0027)"},{"line_number":74,"context_line":"        waiters.wait_for_volume_retype(self.volumes_client, volume_id,"},{"line_number":75,"context_line":"                                       type_dest_id)"}],"source_content_type":"text/x-python","patch_set":87,"id":"5a3905b3_bf7476cf","line":72,"in_reply_to":"5a3905b3_aeca8a4d","updated":"2017-01-19 18:06:15.000000000","message":"Well, submit a fix and add this patch as dependent of it. That should work as well.","commit_id":"1b65b2c88d95445328269f3c134b9ecb189a82f8"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b767c5f67d7074f606707956c0250452c522db58","unresolved":false,"context_lines":[{"line_number":64,"context_line":"            params\u003d{\u0027all_tenants\u0027: True,"},{"line_number":65,"context_line":"                    \u0027display_name\u0027: cls.src_vol[\u0027name\u0027]})[\u0027volumes\u0027]"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        for fetched_vol in fetched_list:"},{"line_number":68,"context_line":"            if fetched_vol[\u0027id\u0027] !\u003d cls.src_vol[\u0027id\u0027]:"},{"line_number":69,"context_line":"                # This is the Cinder internal volume"},{"line_number":70,"context_line":"                LOG.debug(\u0027Waiting for internal volume %s deletion\u0027,"}],"source_content_type":"text/x-python","patch_set":88,"id":"5a3905b3_3fbfa5fc","line":67,"updated":"2017-01-19 18:23:00.000000000","message":"nit: This could be done with an if checking if we have 2 volumes and then just checking which one is the one to wait.","commit_id":"2104f82e06cf558b2ad2033a3dbf21b938e4db8d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d12670442ea9f83e264f77d86a14d753557a08cd","unresolved":false,"context_lines":[{"line_number":64,"context_line":"            params\u003d{\u0027all_tenants\u0027: True,"},{"line_number":65,"context_line":"                    \u0027display_name\u0027: cls.src_vol[\u0027name\u0027]})[\u0027volumes\u0027]"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        for fetched_vol in fetched_list:"},{"line_number":68,"context_line":"            if fetched_vol[\u0027id\u0027] !\u003d cls.src_vol[\u0027id\u0027]:"},{"line_number":69,"context_line":"                # This is the Cinder internal volume"},{"line_number":70,"context_line":"                LOG.debug(\u0027Waiting for internal volume %s deletion\u0027,"}],"source_content_type":"text/x-python","patch_set":89,"id":"5a3905b3_9c307eea","line":67,"updated":"2017-01-20 10:50:34.000000000","message":"nit: Same nit as in previous patch","commit_id":"f10466078442bfc9cea2ef362606e417c1cfac28"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"fcae30d76883702190ce64e0d980e9d01cc79cf3","unresolved":false,"context_lines":[{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        # This list should return 2 volumes until the copy and cleanup"},{"line_number":62,"context_line":"        # process is finished."},{"line_number":63,"context_line":"        fetched_list \u003d cls.volumes_client.list_volumes("},{"line_number":64,"context_line":"            params\u003d{\u0027all_tenants\u0027: True,"},{"line_number":65,"context_line":"                    \u0027display_name\u0027: cls.src_vol[\u0027name\u0027]})[\u0027volumes\u0027]"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        for fetched_vol in fetched_list:"},{"line_number":68,"context_line":"            if fetched_vol[\u0027id\u0027] !\u003d cls.src_vol[\u0027id\u0027]:"}],"source_content_type":"text/x-python","patch_set":90,"id":"5a3905b3_8fd4b547","line":65,"range":{"start_line":63,"start_character":0,"end_line":65,"end_character":68},"updated":"2017-01-20 14:26:34.000000000","message":"I do not think non-admin can get the all tenants volume with \u0027all_tenants\u0027 ?\n\nYou need to use admin client here","commit_id":"bf2c55a4bec2f7f0e01dbeb1fb8b9dad8ceb5d51"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"fcae30d76883702190ce64e0d980e9d01cc79cf3","unresolved":false,"context_lines":[{"line_number":69,"context_line":"                # This is the Cinder internal volume"},{"line_number":70,"context_line":"                LOG.debug(\u0027Waiting for internal volume %s deletion\u0027,"},{"line_number":71,"context_line":"                          fetched_vol[\u0027id\u0027])"},{"line_number":72,"context_line":"                cls.volumes_client.wait_for_resource_deletion("},{"line_number":73,"context_line":"                    fetched_vol[\u0027id\u0027])"},{"line_number":74,"context_line":"                break"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        super(VolumeRetypeWithMigrationV2Test, cls).resource_cleanup()"}],"source_content_type":"text/x-python","patch_set":90,"id":"5a3905b3_4fbbad0e","line":73,"range":{"start_line":72,"start_character":0,"end_line":73,"end_character":38},"updated":"2017-01-20 14:26:34.000000000","message":"should be ok as test wait for retype to finish but explicit wait for volume deletion is no harm.","commit_id":"bf2c55a4bec2f7f0e01dbeb1fb8b9dad8ceb5d51"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"a9501b11b802a6a89cce57a601232af99feacdaa","unresolved":false,"context_lines":[{"line_number":36,"context_line":"            raise cls.skipException(\"Requires at least two different \""},{"line_number":37,"context_line":"                                    \"backend names\")"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    @classmethod"},{"line_number":40,"context_line":"    def resource_setup(cls):"},{"line_number":41,"context_line":"        super(VolumeRetypeWithMigrationV2Test, cls).resource_setup()"},{"line_number":42,"context_line":"        # read backend name from a list."},{"line_number":43,"context_line":"        cls.backend_src \u003d CONF.volume.backend_names[0]"},{"line_number":44,"context_line":"        cls.backend_dst \u003d CONF.volume.backend_names[1]"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"        extra_specs_src \u003d {\"volume_backend_name\": cls.backend_src}"},{"line_number":47,"context_line":"        extra_specs_dst \u003d {\"volume_backend_name\": cls.backend_dst}"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        cls.src_vol_type \u003d cls.create_volume_type(extra_specs\u003dextra_specs_src)"},{"line_number":50,"context_line":"        cls.dst_vol_type \u003d cls.create_volume_type(extra_specs\u003dextra_specs_dst)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        cls.src_vol \u003d cls.create_volume(volume_type\u003dcls.src_vol_type[\u0027name\u0027])"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    @classmethod"},{"line_number":55,"context_line":"    def resource_cleanup(cls):"},{"line_number":56,"context_line":"        # When retyping a volume, Cinder creates an internal volume in the"},{"line_number":57,"context_line":"        # target backend. The volume in the source backend is deleted after"},{"line_number":58,"context_line":"        # the migration, so we need to wait for Cinder delete this volume"},{"line_number":59,"context_line":"        # before deleting the types we\u0027ve created."},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        # This list should return 2 volumes until the copy and cleanup"},{"line_number":62,"context_line":"        # process is finished."},{"line_number":63,"context_line":"        fetched_list \u003d cls.admin_volume_client.list_volumes("},{"line_number":64,"context_line":"            params\u003d{\u0027all_tenants\u0027: True,"},{"line_number":65,"context_line":"                    \u0027display_name\u0027: cls.src_vol[\u0027name\u0027]})[\u0027volumes\u0027]"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        for fetched_vol in fetched_list:"},{"line_number":68,"context_line":"            if fetched_vol[\u0027id\u0027] !\u003d cls.src_vol[\u0027id\u0027]:"},{"line_number":69,"context_line":"                # This is the Cinder internal volume"},{"line_number":70,"context_line":"                LOG.debug(\u0027Waiting for internal volume %s deletion\u0027,"},{"line_number":71,"context_line":"                          fetched_vol[\u0027id\u0027])"},{"line_number":72,"context_line":"                cls.admin_volume_client.wait_for_resource_deletion("},{"line_number":73,"context_line":"                    fetched_vol[\u0027id\u0027])"},{"line_number":74,"context_line":"                break"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"        super(VolumeRetypeWithMigrationV2Test, cls).resource_cleanup()"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    @test.idempotent_id(\u0027a1a41f3f-9dad-493e-9f09-3ff197d477cd\u0027)"},{"line_number":79,"context_line":"    def test_available_volume_retype_with_migration(self):"}],"source_content_type":"text/x-python","patch_set":91,"id":"5a3905b3_842c389a","line":76,"range":{"start_line":39,"start_character":0,"end_line":76,"end_character":70},"updated":"2017-01-23 18:09:34.000000000","message":"nit: I guess this test class will always contain current test only without another tests because of the test class name.\nSo this resource_setup/cleanup could be merged into a test itself.\nBut current patch is already good for me, because that is kind of cleanup. And the test itself seems good for me.","commit_id":"890f59ac7c1be8d7bbe39555be4435d09935359f"}],"tempest/api/volume/base.py":[{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"1ab5a900fbb58506fe12443294388a66b8726b76","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                # The qos_specs may have already been deleted which is OK."},{"line_number":239,"context_line":"                pass"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"    @classmethod"},{"line_number":242,"context_line":"    def create_volume_with_type(self, backend_name_key, name_field,"},{"line_number":243,"context_line":"                                with_prefix\u003dFalse):"},{"line_number":244,"context_line":"        # Volume/Type creation"}],"source_content_type":"text/x-python","patch_set":42,"id":"3a29b11f_c4175c43","line":241,"updated":"2015-10-22 13:09:32.000000000","message":"Why are you using a class method ?","commit_id":"af7aa33ca31d34d4dbf38e1d6e0b4d5424e8e021"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"1ab5a900fbb58506fe12443294388a66b8726b76","unresolved":false,"context_lines":[{"line_number":239,"context_line":"                pass"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"    @classmethod"},{"line_number":242,"context_line":"    def create_volume_with_type(self, backend_name_key, name_field,"},{"line_number":243,"context_line":"                                with_prefix\u003dFalse):"},{"line_number":244,"context_line":"        # Volume/Type creation"},{"line_number":245,"context_line":"        type_name \u003d data_utils.rand_name(\u0027Type\u0027)"}],"source_content_type":"text/x-python","patch_set":42,"id":"3a29b11f_44702ca4","line":242,"updated":"2015-10-22 13:09:32.000000000","message":"We usually name this first arg \"cls\" when using classmethod.\n\nAlso rename this method \"create_type_and_volume\". To reflect the fact that you are not only creating a typed volume but also a new volume type.","commit_id":"af7aa33ca31d34d4dbf38e1d6e0b4d5424e8e021"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"4805eee5c93ec4988879a6ee15fad3086972378d","unresolved":false,"context_lines":[{"line_number":239,"context_line":"                pass"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"    @classmethod"},{"line_number":242,"context_line":"    def create_volume_with_type(self, backend_name_key, name_field,"},{"line_number":243,"context_line":"                                with_prefix\u003dFalse):"},{"line_number":244,"context_line":"        # Volume/Type creation"},{"line_number":245,"context_line":"        type_name \u003d data_utils.rand_name(\u0027Type\u0027)"}],"source_content_type":"text/x-python","patch_set":42,"id":"1a26ad4f_3d7938fd","line":242,"in_reply_to":"3a29b11f_44702ca4","updated":"2015-10-28 02:42:07.000000000","message":"Thank you, Jordan.\nI saw all the methods here are decorated with @classmethod, considering it was a sort of standardized format or something. \nI will revise it.","commit_id":"af7aa33ca31d34d4dbf38e1d6e0b4d5424e8e021"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"06b2531fa97ba445afa5b44658d3ef45a70ba466","unresolved":false,"context_lines":[{"line_number":249,"context_line":"        if with_prefix:"},{"line_number":250,"context_line":"            extra_specs \u003d {spec_key_with_prefix: backend_name_key}"},{"line_number":251,"context_line":"        else:"},{"line_number":252,"context_line":"            extra_specs \u003d {spec_key_without_prefix: backend_name_key}"},{"line_number":253,"context_line":"        type \u003d cls.volume_types_client.create_volume_type("},{"line_number":254,"context_line":"            type_name, extra_specs\u003dextra_specs)[\u0027volume_type\u0027]"},{"line_number":255,"context_line":""}],"source_content_type":"text/x-python","patch_set":43,"id":"ba8a016a_0320759a","line":252,"updated":"2015-11-23 11:50:12.000000000","message":"nit: I know you are just moving the code, but this looks weird to me, seems more reasonable:\n\n    if with_prefix:\n        spec_key \u003d \"capabilities:volume_backend_name\"\n    else:\n        spec_key \u003d \"volume_backend_name\"\n    extra_specs \u003d {spec_key: backend_name_key}","commit_id":"9a0980c9edcf94b691f5145edae4f810a5e193fe"},{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"ac9cf6a47f4742e6bb8ae0becb3029ccc0ace0cf","unresolved":false,"context_lines":[{"line_number":237,"context_line":"            except lib_exc.NotFound:"},{"line_number":238,"context_line":"                # The qos_specs may have already been deleted which is OK."},{"line_number":239,"context_line":"                pass"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"    @classmethod"},{"line_number":242,"context_line":"    def create_type_and_volume(cls, backend_name_key, name_field,"},{"line_number":243,"context_line":"                               with_prefix\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":45,"id":"9a8ffd7b_cf1b47c9","line":240,"updated":"2015-12-02 08:32:08.000000000","message":"I like the idea of moving the type create to base but :\n#1 please do not use the same function for type and volume  , think about that users will ask to create a type without a volume  and attaching latert encrypted type.\n\n#2 you should care of cleanup for types , please note that the order is important.\nclean volumes, \nclean type encrypted\nclean volume type\n\nAnd volume type should be deleted by admin permission .\n\nLet me know if you need help ..\n\nBenny","commit_id":"49ede0f8acf581e6899b0c674fb112089b49bb91"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"b088d10cc059baa4dafa5fd31c955eb8dc751951","unresolved":false,"context_lines":[{"line_number":237,"context_line":"            except lib_exc.NotFound:"},{"line_number":238,"context_line":"                # The qos_specs may have already been deleted which is OK."},{"line_number":239,"context_line":"                pass"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":"    @classmethod"},{"line_number":242,"context_line":"    def create_type_and_volume(cls, backend_name_key, name_field,"},{"line_number":243,"context_line":"                               with_prefix\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":45,"id":"7a740942_d8a54547","line":240,"in_reply_to":"9a8ffd7b_cf1b47c9","updated":"2015-12-07 13:53:13.000000000","message":"I understand all the comments except the encrypted type.\nI do not know how my change is related to the encrypted type .","commit_id":"49ede0f8acf581e6899b0c674fb112089b49bb91"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b94d219451eb83e9ce6b5cd98e30b91d3bfa3f5a","unresolved":false,"context_lines":[{"line_number":254,"context_line":"    def create_volume_from_type(cls, name_field, volume_type\u003dNone):"},{"line_number":255,"context_line":"        type_name \u003d None"},{"line_number":256,"context_line":"        if volume_type:"},{"line_number":257,"context_line":"            type_name \u003d volume_type[\u0027name\u0027]"},{"line_number":258,"context_line":"        vol_name \u003d data_utils.rand_name(\u0027Volume\u0027)"},{"line_number":259,"context_line":"        params \u003d {name_field: vol_name, \u0027volume_type\u0027: type_name}"},{"line_number":260,"context_line":"        volume \u003d cls.admin_volume_client.create_volume("}],"source_content_type":"text/x-python","patch_set":47,"id":"7a740942_a80d07ea","line":257,"updated":"2015-12-09 13:04:31.000000000","message":"nit: type_name \u003d volume_type[\u0027name\u0027] if volume_type else None","commit_id":"4c4889a21b75e7e87444d30365a6218c01bbae1f"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"2b189c51cbd8066938365a2495dc444cc99dc476","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            spec_key \u003d \"volume_backend_name\""},{"line_number":246,"context_line":"        extra_specs \u003d {spec_key: backend_name_key}"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"        return cls.volume_types_client.create_volume_type("},{"line_number":249,"context_line":"            type_name, extra_specs\u003dextra_specs)[\u0027volume_type\u0027]"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":54,"id":"dab17558_fbda7616","line":248,"updated":"2016-05-12 21:48:32.000000000","message":"This blows up for me that create_volume_type only takes one argument.\n\nhttps://github.com/openstack/tempest/blob/dcc559792320d6cd087f658a3c49a88104493dc5/tempest/services/volume/base/admin/base_types_client.py#L69","commit_id":"572d482f6ce080d53d11db2e096eb8b36dd378d3"},{"author":{"_account_id":7350,"name":"Jordan Pittier","email":"jordan.pittier@gmail.com","username":"JordanP"},"change_message_id":"b3dadb9780ebb7ed761c3b4d3b7493ab22a74ccc","unresolved":false,"context_lines":[{"line_number":280,"context_line":"                vol_type)"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"    @classmethod"},{"line_number":283,"context_line":"    def create_volume_from_type(cls, name_field, volume_type\u003dNone):"},{"line_number":284,"context_line":"        type_name \u003d volume_type[\u0027name\u0027] if volume_type else None"},{"line_number":285,"context_line":"        vol_name \u003d data_utils.rand_name(\u0027Volume\u0027)"},{"line_number":286,"context_line":"        params \u003d {name_field: vol_name, \u0027volume_type\u0027: type_name}"}],"source_content_type":"text/x-python","patch_set":67,"id":"ba5da102_14c65716","line":283,"updated":"2016-11-04 17:18:52.000000000","message":"You don\u0027t need that function. Just use cls.create_volume(**params) in your tests directly.","commit_id":"2d54574e64ee7a0be5a7a81e8e1f5f0035f995d4"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"a0fffa0efacd251d22c880b4008415ee19d89080","unresolved":false,"context_lines":[{"line_number":280,"context_line":"                vol_type)"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"    @classmethod"},{"line_number":283,"context_line":"    def create_volume_from_type(cls, name_field, volume_type\u003dNone):"},{"line_number":284,"context_line":"        type_name \u003d volume_type[\u0027name\u0027] if volume_type else None"},{"line_number":285,"context_line":"        vol_name \u003d data_utils.rand_name(\u0027Volume\u0027)"},{"line_number":286,"context_line":"        params \u003d {name_field: vol_name, \u0027volume_type\u0027: type_name}"}],"source_content_type":"text/x-python","patch_set":67,"id":"9a629dbe_a34bc5af","line":283,"in_reply_to":"ba5da102_14c65716","updated":"2016-11-07 14:42:40.000000000","message":"IMO this is cleaner here, or in and private function inside the class but if is a blocker I can change.","commit_id":"2d54574e64ee7a0be5a7a81e8e1f5f0035f995d4"}],"tempest/config.py":[{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"d139dd58d917576ce5db5b920a350b37963dd98e","unresolved":false,"context_lines":[{"line_number":632,"context_line":"    cfg.IntOpt(\u0027build_interval\u0027,"},{"line_number":633,"context_line":"               default\u003d1,"},{"line_number":634,"context_line":"               help\u003d\u0027Time in seconds between volume availability checks. \u0027"},{"line_number":635,"context_line":"                    \u0027The suggested value for volume migration is 10.\u0027),"},{"line_number":636,"context_line":"    cfg.IntOpt(\u0027build_timeout\u0027,"},{"line_number":637,"context_line":"               default\u003d300,"},{"line_number":638,"context_line":"               help\u003d\u0027Timeout in seconds to wait for a volume to become \u0027"}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_5abcd9db","line":635,"updated":"2015-08-05 14:53:25.000000000","message":"So I really don\u0027t think this is necessary. If it\u0027s a suggested value just make it the default. It\u0027s really weird to say we\u0027re going to set this to something but you really want to use this other value instead.","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"d139dd58d917576ce5db5b920a350b37963dd98e","unresolved":false,"context_lines":[{"line_number":637,"context_line":"               default\u003d300,"},{"line_number":638,"context_line":"               help\u003d\u0027Timeout in seconds to wait for a volume to become \u0027"},{"line_number":639,"context_line":"                    \u0027available. The suggested value for volume migration \u0027"},{"line_number":640,"context_line":"                    \u0027is 1200.\u0027),"},{"line_number":641,"context_line":"    cfg.StrOpt(\u0027catalog_type\u0027,"},{"line_number":642,"context_line":"               default\u003d\u0027volume\u0027,"},{"line_number":643,"context_line":"               help\u003d\"Catalog type of the Volume Service\"),"}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_ba715d46","line":640,"updated":"2015-08-05 14:53:25.000000000","message":"Umm, 1200 seconds is a huge value. Like 700 seconds more than the per test timeout fixture. That does not seem like a value we want to default or suggest. If the migration operations are really that slow you need to tag the test as slow.","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"abebbc0b37712a80ea88c3c33e3f1533f77380f1","unresolved":false,"context_lines":[{"line_number":637,"context_line":"               default\u003d300,"},{"line_number":638,"context_line":"               help\u003d\u0027Timeout in seconds to wait for a volume to become \u0027"},{"line_number":639,"context_line":"                    \u0027available. The suggested value for volume migration \u0027"},{"line_number":640,"context_line":"                    \u0027is 1200.\u0027),"},{"line_number":641,"context_line":"    cfg.StrOpt(\u0027catalog_type\u0027,"},{"line_number":642,"context_line":"               default\u003d\u0027volume\u0027,"},{"line_number":643,"context_line":"               help\u003d\"Catalog type of the Volume Service\"),"}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_3801b9af","line":640,"in_reply_to":"1a4dcd0f_ba715d46","updated":"2015-08-06 04:58:29.000000000","message":"It is hard to predict how long the long is going to take. For 1G volume, it varies from 3 minutes to 10+ mins depending on the network.\n\nFor the migration test, I hope the migration can be done in a complete way and then check if everything is what we expect to decide it is successful or not. \n\nWould tagging this test slow help?","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"d139dd58d917576ce5db5b920a350b37963dd98e","unresolved":false,"context_lines":[{"line_number":658,"context_line":"    cfg.StrOpt(\u0027backend2_name\u0027,"},{"line_number":659,"context_line":"               default\u003d\u0027BACKEND_2\u0027,"},{"line_number":660,"context_line":"               help\u003d\"Name of the backend2 (must be declared in cinder.conf)\"),"},{"line_number":661,"context_line":"    cfg.BoolOpt(\u0027full_migration_test\u0027,"},{"line_number":662,"context_line":"                default\u003dTrue,"},{"line_number":663,"context_line":"                help\u003d\"Indication of whether volume migration will be tested \""},{"line_number":664,"context_line":"                     \"bidirectionaly. If it is True, volume migration will be \""}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_df7e9b18","line":661,"updated":"2015-08-05 14:53:25.000000000","message":"Is this just a flag to enable or disable tests? Or is this something that only certain versions of cinder support? It looks like this is just a flag to enable additional testing. Under what conditions do you set this false? I\u0027m not sure this is really a candidate for an additional config flag.","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"ca1521d0c0b32adc0137374618ea83e6524f14a8","unresolved":false,"context_lines":[{"line_number":658,"context_line":"    cfg.StrOpt(\u0027backend2_name\u0027,"},{"line_number":659,"context_line":"               default\u003d\u0027BACKEND_2\u0027,"},{"line_number":660,"context_line":"               help\u003d\"Name of the backend2 (must be declared in cinder.conf)\"),"},{"line_number":661,"context_line":"    cfg.BoolOpt(\u0027full_migration_test\u0027,"},{"line_number":662,"context_line":"                default\u003dTrue,"},{"line_number":663,"context_line":"                help\u003d\"Indication of whether volume migration will be tested \""},{"line_number":664,"context_line":"                     \"bidirectionaly. If it is True, volume migration will be \""}],"source_content_type":"text/x-python","patch_set":28,"id":"1a4dcd0f_e4272c61","line":661,"in_reply_to":"1a4dcd0f_df7e9b18","updated":"2015-08-12 06:31:47.000000000","message":"I will give a new name to it. The test is like this:\n\nWe have got backend A and backend B, if this flag is true, the test will check the migration from A to B and then from B to A.\nIf this flag is false, the migration test from A to B only.\nMaybe a name like bidirectional_migration will sound better.","commit_id":"dfba052b50481db43a0d0fef5a52cf4f73eb1004"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"d29a256fec0f80f773818742ea885ad75a5ed33b","unresolved":false,"context_lines":[{"line_number":635,"context_line":"    cfg.IntOpt(\u0027build_timeout\u0027,"},{"line_number":636,"context_line":"               default\u003d300,"},{"line_number":637,"context_line":"               help\u003d\u0027Timeout in seconds to wait for a volume to become \u0027"},{"line_number":638,"context_line":"                    \u0027available.\u0027),"},{"line_number":639,"context_line":"    cfg.StrOpt(\u0027catalog_type\u0027,"},{"line_number":640,"context_line":"               default\u003d\u0027volume\u0027,"},{"line_number":641,"context_line":"               help\u003d\"Catalog type of the Volume Service\"),"}],"source_content_type":"text/x-python","patch_set":31,"id":"da20952f_0febab13","side":"PARENT","line":638,"updated":"2015-09-09 20:18:28.000000000","message":"It\u0027s unclear why these are removed. you\u0027ll need these defined to prevent a runtime error.","commit_id":"4ec06d749a02bfd54e8d8a3c8a12f43262f7a306"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"cf9dba0bfe2a7e6166b5ec2919a558cab3d21c85","unresolved":false,"context_lines":[{"line_number":635,"context_line":"    cfg.IntOpt(\u0027build_timeout\u0027,"},{"line_number":636,"context_line":"               default\u003d300,"},{"line_number":637,"context_line":"               help\u003d\u0027Timeout in seconds to wait for a volume to become \u0027"},{"line_number":638,"context_line":"                    \u0027available.\u0027),"},{"line_number":639,"context_line":"    cfg.StrOpt(\u0027catalog_type\u0027,"},{"line_number":640,"context_line":"               default\u003d\u0027volume\u0027,"},{"line_number":641,"context_line":"               help\u003d\"Catalog type of the Volume Service\"),"}],"source_content_type":"text/x-python","patch_set":31,"id":"9a1a9d01_96794696","side":"PARENT","line":638,"in_reply_to":"da20952f_0febab13","updated":"2015-09-25 07:10:18.000000000","message":"I agree. I need to get them back.","commit_id":"4ec06d749a02bfd54e8d8a3c8a12f43262f7a306"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"d29a256fec0f80f773818742ea885ad75a5ed33b","unresolved":false,"context_lines":[{"line_number":649,"context_line":"    cfg.StrOpt(\u0027backend2_name\u0027,"},{"line_number":650,"context_line":"               default\u003d\u0027BACKEND_2\u0027,"},{"line_number":651,"context_line":"               help\u003d\"Name of the backend2 (must be declared in cinder.conf)\"),"},{"line_number":652,"context_line":"    cfg.BoolOpt(\u0027bidirectionally_migration_test\u0027,"},{"line_number":653,"context_line":"                default\u003dTrue,"},{"line_number":654,"context_line":"                help\u003d\"Indication of whether volume migration will be tested \""},{"line_number":655,"context_line":"                     \"bidirectionally. If it is True, volume migration will be \""}],"source_content_type":"text/x-python","patch_set":31,"id":"da20952f_2fbd4f02","line":652,"updated":"2015-09-09 20:18:28.000000000","message":"If the backends are not the same a bidirection test gives us more code coverage and a better sense of correctness.  Is it ever valuable to /not/ test in both directions?  I would remove this and make bidirectional migration tests the default.","commit_id":"f815bcab0c1098ecf6d1639f3fae390bec89e1d6"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"1e89e34c4e6d234bbe0f6bfae77a228cd77864af","unresolved":false,"context_lines":[{"line_number":649,"context_line":"    cfg.StrOpt(\u0027backend2_name\u0027,"},{"line_number":650,"context_line":"               default\u003d\u0027BACKEND_2\u0027,"},{"line_number":651,"context_line":"               help\u003d\"Name of the backend2 (must be declared in cinder.conf)\"),"},{"line_number":652,"context_line":"    cfg.BoolOpt(\u0027bidirectionally_migration_test\u0027,"},{"line_number":653,"context_line":"                default\u003dTrue,"},{"line_number":654,"context_line":"                help\u003d\"Indication of whether volume migration will be tested \""},{"line_number":655,"context_line":"                     \"bidirectionally. If it is True, volume migration will be \""}],"source_content_type":"text/x-python","patch_set":31,"id":"fa1b9901_6c34bcfa","line":652,"updated":"2015-08-24 14:57:43.000000000","message":"This would make more sense as \u0027bidirectional_migration_test\u0027 .","commit_id":"f815bcab0c1098ecf6d1639f3fae390bec89e1d6"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"cf9dba0bfe2a7e6166b5ec2919a558cab3d21c85","unresolved":false,"context_lines":[{"line_number":649,"context_line":"    cfg.StrOpt(\u0027backend2_name\u0027,"},{"line_number":650,"context_line":"               default\u003d\u0027BACKEND_2\u0027,"},{"line_number":651,"context_line":"               help\u003d\"Name of the backend2 (must be declared in cinder.conf)\"),"},{"line_number":652,"context_line":"    cfg.BoolOpt(\u0027bidirectionally_migration_test\u0027,"},{"line_number":653,"context_line":"                default\u003dTrue,"},{"line_number":654,"context_line":"                help\u003d\"Indication of whether volume migration will be tested \""},{"line_number":655,"context_line":"                     \"bidirectionally. If it is True, volume migration will be \""}],"source_content_type":"text/x-python","patch_set":31,"id":"9a1a9d01_76d03254","line":652,"in_reply_to":"da20952f_2fbd4f02","updated":"2015-09-25 07:10:18.000000000","message":"Let me get it simpler. I like to keep the bidirectional as a default behavior.","commit_id":"f815bcab0c1098ecf6d1639f3fae390bec89e1d6"}],"tempest/scenario/test_volume_migrate_attached.py":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"1ef158e976d00e3d1e09d79c515827fd1d02eb0e","unresolved":false,"context_lines":[{"line_number":34,"context_line":"     * Check written content of migrated volume"},{"line_number":35,"context_line":"    \"\"\""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    # Boot from volume scenario is quite slow, and needs extra"},{"line_number":38,"context_line":"    # breathing room to get through deletes in the time allotted."},{"line_number":39,"context_line":"    TIMEOUT_SCALING_FACTOR \u003d 2"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":42,"context_line":""}],"source_content_type":"text/x-python","patch_set":80,"id":"ba5201f7_73b09d8c","side":"PARENT","line":39,"range":{"start_line":37,"start_character":0,"end_line":39,"end_character":30},"updated":"2016-12-30 14:54:44.000000000","message":"did not find reason for this change in commit msg, can we do this as separate patch with proper reasoning w.r.t. to Notes.","commit_id":"1a8d58698315229b41aa6818d7d40d5578b0d030"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"719027a6159c6ab1cd2635e5de2736c73970aa69","unresolved":false,"context_lines":[{"line_number":34,"context_line":"     * Check written content of migrated volume"},{"line_number":35,"context_line":"    \"\"\""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    # Boot from volume scenario is quite slow, and needs extra"},{"line_number":38,"context_line":"    # breathing room to get through deletes in the time allotted."},{"line_number":39,"context_line":"    TIMEOUT_SCALING_FACTOR \u003d 2"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":42,"context_line":""}],"source_content_type":"text/x-python","patch_set":80,"id":"7a3c09a3_c5758098","side":"PARENT","line":39,"range":{"start_line":37,"start_character":0,"end_line":39,"end_character":30},"in_reply_to":"ba5201f7_73b09d8c","updated":"2017-01-17 13:27:43.000000000","message":"This came from the other patch in the patch series.","commit_id":"1a8d58698315229b41aa6818d7d40d5578b0d030"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"8e38da374e42af2b9abca2f22f5c991ac7539d91","unresolved":false,"context_lines":[{"line_number":34,"context_line":"     * Check written content of migrated volume"},{"line_number":35,"context_line":"    \"\"\""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    credentials \u003d [\u0027primary\u0027, \u0027admin\u0027]"},{"line_number":38,"context_line":"    @classmethod"},{"line_number":39,"context_line":"    def setup_clients(cls):"},{"line_number":40,"context_line":"        super(TestVolumeMigrateRetypeAttached, cls).setup_clients()"}],"source_content_type":"text/x-python","patch_set":84,"id":"5a3905b3_af037f0e","line":37,"range":{"start_line":37,"start_character":31,"end_line":37,"end_character":36},"updated":"2017-01-19 11:01:08.000000000","message":"?: Do we really need admin credentials","commit_id":"b11f5823702b536e1db47387b081c8ee8ed12c56"}]}
