)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":8648,"name":"Radomir Dopieralski","email":"openstack@dopieralski.pl","username":"thesheep"},"change_message_id":"52e36dac0bf384d3fbad8e78fcd8c7b7081a76bb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ad79f1ab_b040e8aa","updated":"2025-12-02 12:59:02.000000000","message":"Is this still relevant?","commit_id":"ac329a4e174178fc25e9312b3259a773b7cbfa96"}],"openstack_dashboard/test/selenium/integration/test_instances.py":[{"author":{"_account_id":8648,"name":"Radomir Dopieralski","email":"openstack@dopieralski.pl","username":"thesheep"},"change_message_id":"3222d5f8962c36af90912ecf5c2d7969f5b82417","unresolved":true,"context_lines":[{"line_number":129,"context_line":"def is_instance_in_active_state(openstack, instance_name):"},{"line_number":130,"context_line":"    is_active \u003d False"},{"line_number":131,"context_line":"    for attempt in range(120):"},{"line_number":132,"context_line":"        if (openstack.compute.find_server(instance_name).status \u003d\u003d \"ACTIVE\"):"},{"line_number":133,"context_line":"            is_active \u003d True"},{"line_number":134,"context_line":"            break"},{"line_number":135,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"406c9a7b_bc9acb58","line":132,"updated":"2025-05-19 15:02:40.000000000","message":"do we want to catch the error in case the instance doesn\u0027t exist yet?","commit_id":"ac329a4e174178fc25e9312b3259a773b7cbfa96"},{"author":{"_account_id":35133,"name":"Jan Jasek","email":"jjasek@redhat.com","username":"janjasek"},"change_message_id":"dc2c88558794b6df484d2e3b4016d8c0b6fd855e","unresolved":true,"context_lines":[{"line_number":129,"context_line":"def is_instance_in_active_state(openstack, instance_name):"},{"line_number":130,"context_line":"    is_active \u003d False"},{"line_number":131,"context_line":"    for attempt in range(120):"},{"line_number":132,"context_line":"        if (openstack.compute.find_server(instance_name).status \u003d\u003d \"ACTIVE\"):"},{"line_number":133,"context_line":"            is_active \u003d True"},{"line_number":134,"context_line":"            break"},{"line_number":135,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"4b9e2cfe_7a00f126","line":132,"in_reply_to":"406c9a7b_bc9acb58","updated":"2025-05-19 16:50:15.000000000","message":"I am not exactly sure what you mean by that.\nBut if you look at the tests where it is used (for example lines 188-191) then you can see that  we fill the instance form, click the button Launch instance, and then because we are not able to catch the popup message we use EC.invisibility wait (line 188-189) for waiting until the page is reloaded. At this moment the request is done and the creation of an instance is existing in some state (building, spawning, anything). We never had a random failure because the instance was not created yet. The page refresh is enough time for instance to be created.\n\nUntil now we only checked if the instance is existing, no matter what the state is (line 190) and if yes, then we were okay with that and passed the tests. \n\nBut based on this patch.\n(https://review.opendev.org/c/openstack/horizon/+/946269/comments/29d6747c_20193aa5)\n and issues that I found there I think the current check is not enough and we should also check the status (wait for active state). \nThe issue is not on our side, it is an issue in OpenstackSDK:\nhttps://review.opendev.org/c/openstack/openstacksdk/+/950305\nSo practically it is not something that should be covered by our integration tests and we would not catch it using standard popup window checking anyway, because this issue appears when the creation of Instance is finished, not when the request is sent from Horizon.\n\nBut anyway… When we do not check the popup windows because we are not able to handle it using Selenium I think we should check the status on the server side. So I added an additional assert for checking if the status of the instance is really active.","commit_id":"ac329a4e174178fc25e9312b3259a773b7cbfa96"},{"author":{"_account_id":35133,"name":"Jan Jasek","email":"jjasek@redhat.com","username":"janjasek"},"change_message_id":"0417023dbba67d259db71de0c7b72fdd207fc6b6","unresolved":true,"context_lines":[{"line_number":129,"context_line":"def is_instance_in_active_state(openstack, instance_name):"},{"line_number":130,"context_line":"    is_active \u003d False"},{"line_number":131,"context_line":"    for attempt in range(120):"},{"line_number":132,"context_line":"        if (openstack.compute.find_server(instance_name).status \u003d\u003d \"ACTIVE\"):"},{"line_number":133,"context_line":"            is_active \u003d True"},{"line_number":134,"context_line":"            break"},{"line_number":135,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"2ded2712_bfb90c23","line":132,"in_reply_to":"4b9e2cfe_7a00f126","updated":"2025-05-19 22:28:41.000000000","message":"@openstack@dopieralski.pl,\nOkay, It is driving me crazy and I am still thinking about it :D. If I was not reviewing the SDK patch so deeply, everything looks well, tests passed and there is a chance it would be merged and it will break instance creation.\nOtherwise it was not issued on our side, it was issued in the OpenStack SDK. And communication from Horizon side was correct so our Integration tests should pass and our tests should not care what is happening later then during instance creation. And if we merge this mine patch for expanding checking instances then it can bring more instabilities/issues or catching issues on nova side.\nSo I think this patch should not be merged. But I would like to discuss what is the difference between popups for Instances compared to other services and if I have time I am willing to spend tim to try to fix it. (because although it would not catch this issue anyway… It drives me crazy :D)","commit_id":"ac329a4e174178fc25e9312b3259a773b7cbfa96"}]}