)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"21ed8c854cc634671e7bda5089d1063e679a043f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"52869cce_29bd4bad","updated":"2022-03-20 13:55:27.000000000","message":"Suggestion inline.","commit_id":"6ff033954448822dcc9673e6b79ce1f45d557d6f"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"29a632ca4a59c5a3406220f809bfda067cfda2f4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7ad8d6ac_c00caa64","updated":"2022-02-11 09:02:34.000000000","message":"recheck","commit_id":"6ff033954448822dcc9673e6b79ce1f45d557d6f"}],"cinder_tempest_plugin/api/volume/test_create_from_image.py":[{"author":{"_account_id":11075,"name":"Benny Kopilov","email":"bkopilov@redhat.com","username":"bkopilov"},"change_message_id":"5698f1b8de128a54b1b9c944df00854106400e1b","unresolved":true,"context_lines":[{"line_number":129,"context_line":"        letters \u003d string.ascii_lowercase"},{"line_number":130,"context_line":"        prefix \u003d \u0027\u0027.join(random.choice(letters) for i in range(6))"},{"line_number":131,"context_line":"        volume_type_name \u003d ("},{"line_number":132,"context_line":"            f\u0027{prefix}-vol-type-for-6e9266ff-a917-4dd5-aa4a-c36e59e7a2a6\u0027)"},{"line_number":133,"context_line":"        description \u003d (\u0027Generic volume_type for test \u0027"},{"line_number":134,"context_line":"                       \u00276e9266ff-a917-4dd5-aa4a-c36e59e7a2a6\u0027)"},{"line_number":135,"context_line":"        proto \u003d CONF.volume.storage_protocol"}],"source_content_type":"text/x-python","patch_set":1,"id":"dee35be2_f318ecae","line":132,"updated":"2022-03-10 14:54:13.000000000","message":"The f\u0027 flag is not support by python 2.7 , not sure if its a good choose here .\nWe need to assume that this test would run on older versions ....","commit_id":"6ff033954448822dcc9673e6b79ce1f45d557d6f"},{"author":{"_account_id":33612,"name":"yuval","email":"yuval@lightbitslabs.com","username":"yuval"},"change_message_id":"c66a468d5cc5a3c1c883138953f479404e40245f","unresolved":true,"context_lines":[{"line_number":129,"context_line":"        letters \u003d string.ascii_lowercase"},{"line_number":130,"context_line":"        prefix \u003d \u0027\u0027.join(random.choice(letters) for i in range(6))"},{"line_number":131,"context_line":"        volume_type_name \u003d ("},{"line_number":132,"context_line":"            f\u0027{prefix}-vol-type-for-6e9266ff-a917-4dd5-aa4a-c36e59e7a2a6\u0027)"},{"line_number":133,"context_line":"        description \u003d (\u0027Generic volume_type for test \u0027"},{"line_number":134,"context_line":"                       \u00276e9266ff-a917-4dd5-aa4a-c36e59e7a2a6\u0027)"},{"line_number":135,"context_line":"        proto \u003d CONF.volume.storage_protocol"}],"source_content_type":"text/x-python","patch_set":1,"id":"72f6d7bc_9046ecbd","line":132,"in_reply_to":"b8e26f88_b56dea90","updated":"2022-03-20 15:09:36.000000000","message":"I can change the prefix creation to use time.time()","commit_id":"6ff033954448822dcc9673e6b79ce1f45d557d6f"},{"author":{"_account_id":10459,"name":"Luigi Toscano","email":"ltoscano@redhat.com","username":"ltoscano"},"change_message_id":"df2a9aa86498e7ba23f20f090a3090afea121a2a","unresolved":true,"context_lines":[{"line_number":129,"context_line":"        letters \u003d string.ascii_lowercase"},{"line_number":130,"context_line":"        prefix \u003d \u0027\u0027.join(random.choice(letters) for i in range(6))"},{"line_number":131,"context_line":"        volume_type_name \u003d ("},{"line_number":132,"context_line":"            f\u0027{prefix}-vol-type-for-6e9266ff-a917-4dd5-aa4a-c36e59e7a2a6\u0027)"},{"line_number":133,"context_line":"        description \u003d (\u0027Generic volume_type for test \u0027"},{"line_number":134,"context_line":"                       \u00276e9266ff-a917-4dd5-aa4a-c36e59e7a2a6\u0027)"},{"line_number":135,"context_line":"        proto \u003d CONF.volume.storage_protocol"}],"source_content_type":"text/x-python","patch_set":1,"id":"fcc21ebe_058b4de1","line":132,"in_reply_to":"dee35be2_f318ecae","updated":"2022-03-18 15:19:44.000000000","message":"The master branches of tempest and its plugins support OpenStack since Ussuri, which means Python 3 only. So the change is fine.\n\nMy question is instead: why is the UUID hardcoded? Instead of adding a prefix, can\u0027t we just change it? Of course the description (next line) would need to be changed too in that case, as well as the name of the image (line 150) an the name of the volume  (line 159). \n\nNow, I should have asked this when the test was added instead of simply approving it :) https://review.opendev.org/c/openstack/cinder-tempest-plugin/+/737380 but well.\nBrian, do you think we can dynamically generate a new UUID here to be used instead of the hardcoded one?","commit_id":"6ff033954448822dcc9673e6b79ce1f45d557d6f"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"21ed8c854cc634671e7bda5089d1063e679a043f","unresolved":true,"context_lines":[{"line_number":129,"context_line":"        letters \u003d string.ascii_lowercase"},{"line_number":130,"context_line":"        prefix \u003d \u0027\u0027.join(random.choice(letters) for i in range(6))"},{"line_number":131,"context_line":"        volume_type_name \u003d ("},{"line_number":132,"context_line":"            f\u0027{prefix}-vol-type-for-6e9266ff-a917-4dd5-aa4a-c36e59e7a2a6\u0027)"},{"line_number":133,"context_line":"        description \u003d (\u0027Generic volume_type for test \u0027"},{"line_number":134,"context_line":"                       \u00276e9266ff-a917-4dd5-aa4a-c36e59e7a2a6\u0027)"},{"line_number":135,"context_line":"        proto \u003d CONF.volume.storage_protocol"}],"source_content_type":"text/x-python","patch_set":1,"id":"b8e26f88_b56dea90","line":132,"in_reply_to":"fcc21ebe_058b4de1","updated":"2022-03-20 13:55:27.000000000","message":"My intention in the original patch was to get the test\u0027s idempotent_id into the name of the volume type so that it would be easier to determine where it came from when troubleshooting test failures.  I used the id because while it\u0027s ok to change test names, the id is supposed to stay constant.\n\nThis id is used in the names of all the volume types and volumes created by this test (except for a volume in the image-volume-cache, whose name I believe will be that of the image_id, which we don\u0027t have any control over from the cinder side), so it should be possible to write a maintenance script to go through and clean these things out (although, if something is left in the image-volume-cache, that script will fail).\n\nI\u0027m kind of surprised that we\u0027re seeing this issue, because this test should create one volume type and at most two volumes (the second only if the image-volume-cache is enabled).  So it\u0027s not like there\u0027s a huge list of volumes that must be deleted before the volume type can be deleted during class cleanup. \n\nDeleting a volume type is a simple database operation, so the only reason it should fail is if there are existing volumes of that type.  While this patch will workaround the previously-existing-volume-type problem, it doesn\u0027t address the other cruft that could be left behind:\n\n- the volume created by the test (should be deleted during test teardown)\n- the image created by the test (should be deleted during test teardown)\n- any other volumes of the test type (would be created if the image-volume-cache is enabled; should be deleted during class teardown)\n\nI would be curious to know if any of this other stuff is piling up on your CI system.\n\nMy suggestion is that instead of introducing a random name into the volume_type, you instead use time.time() to give you seconds since the epoch to create the prefix, then use the prefix for all of the items created by this test.  Glance and Cinder both allow names on the order of 255 chars, so name length shouldn\u0027t be a problem.  I\u0027d be curious to find out, for example, if images are accumulating in Glance.  Also, this would give you a way to have some kind of scripted cleanup for your CI.","commit_id":"6ff033954448822dcc9673e6b79ce1f45d557d6f"}]}