)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":27329,"name":"Federico Ressi","email":"fressi@redhat.com","username":"fressi_redhat"},"change_message_id":"d559ae04a6ed45d8f782feefe47162b0d787aacb","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fixed cleanup long duration"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"wait_for_status() is looking for loadbalancer objects with ACTIVE status, even when they were already DELETED."},{"line_number":10,"context_line":"It caused current tests to finish after ~20 minutes."},{"line_number":11,"context_line":"This patch reduces the total duration to ~7 minutes."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5f7c97a3_60f3bb9f","line":9,"range":{"start_line":9,"start_character":66,"end_line":9,"end_character":110},"updated":"2018-05-23 11:57:49.000000000","message":"Please break lines at 80 column in commit message too to make code review more comfortable.","commit_id":"ca2405c83ce948680258af124100064d839cd171"},{"author":{"_account_id":27329,"name":"Federico Ressi","email":"fressi@redhat.com","username":"fressi_redhat"},"change_message_id":"a6a3c4681c4b086e78731d01bc02f480e07b8240","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fixed cleanup long duration"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"wait_for_status() is looking for loadbalancer objects with an "},{"line_number":10,"context_line":"ACTIVE status, even when they were already DELETED."},{"line_number":11,"context_line":"It caused current tests to finish after ~20 minutes."},{"line_number":12,"context_line":"This patch reduces the total duration to ~7 minutes."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5f7c97a3_89a2b799","line":9,"range":{"start_line":9,"start_character":61,"end_line":9,"end_character":62},"updated":"2018-05-24 06:45:46.000000000","message":"Trailing white-space characters should be removed.","commit_id":"2899f934efe3150ecbf100731c7af95c63225ff9"}],"octavia_tempest_plugin/tests/waiters.py":[{"author":{"_account_id":27329,"name":"Federico Ressi","email":"fressi@redhat.com","username":"fressi_redhat"},"change_message_id":"d559ae04a6ed45d8f782feefe47162b0d787aacb","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                                                        message\u003dmessage)"},{"line_number":77,"context_line":"            raise exceptions.UnexpectedResponseCode(message)"},{"line_number":78,"context_line":"        elif object_details[status_key] \u003d\u003d const.DELETED:"},{"line_number":79,"context_line":"            LOG.info(\u0027{name}\\\u0027s status updated to DELETED.\u0027.format("},{"line_number":80,"context_line":"                name\u003dshow_client.__name__))"},{"line_number":81,"context_line":"            return object_details"},{"line_number":82,"context_line":"        elif int(time.time()) - start \u003e\u003d check_timeout:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_e034ab4c","line":79,"range":{"start_line":79,"start_character":21,"end_line":79,"end_character":59},"updated":"2018-05-23 11:57:49.000000000","message":"Use double quoted strings when you want to use \u0027 char in strings. In this way you don\u0027t need to use \\\u0027 escaped character.","commit_id":"ca2405c83ce948680258af124100064d839cd171"},{"author":{"_account_id":27821,"name":"Noam Manos","email":"nmanos@redhat.com","username":"nmanos"},"change_message_id":"9192c24021693305b6285af4db5bdbccaf77b6f3","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                                                        message\u003dmessage)"},{"line_number":77,"context_line":"            raise exceptions.UnexpectedResponseCode(message)"},{"line_number":78,"context_line":"        elif object_details[status_key] \u003d\u003d const.DELETED:"},{"line_number":79,"context_line":"            LOG.info(\u0027{name}\\\u0027s status updated to DELETED.\u0027.format("},{"line_number":80,"context_line":"                name\u003dshow_client.__name__))"},{"line_number":81,"context_line":"            return object_details"},{"line_number":82,"context_line":"        elif int(time.time()) - start \u003e\u003d check_timeout:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_a00d533f","line":79,"range":{"start_line":79,"start_character":21,"end_line":79,"end_character":59},"in_reply_to":"5f7c97a3_e034ab4c","updated":"2018-05-23 13:19:46.000000000","message":"Please look at line 165 - it has the same format. If I change this format, we\u0027ll have to change all similar formats, like in line 165.","commit_id":"ca2405c83ce948680258af124100064d839cd171"},{"author":{"_account_id":6469,"name":"Carlos Gonçalves","display_name":"Carlos Goncalves","email":"cgoncalves@redhat.com","username":"cgoncalves"},"change_message_id":"151fc4f6da2fea627e115b5122ce0b3770ec1ff6","unresolved":false,"context_lines":[{"line_number":63,"context_line":"            object_details \u003d response"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        if object_details[status_key] \u003d\u003d status:"},{"line_number":66,"context_line":"            LOG.info(\u0027{name}\\\u0027s status updated to {status}.\u0027.format("},{"line_number":67,"context_line":"                name\u003dshow_client.__name__, status\u003dstatus))"},{"line_number":68,"context_line":"            return object_details"},{"line_number":69,"context_line":"        elif object_details[status_key] \u003d\u003d \u0027ERROR\u0027:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_983c1594","line":66,"range":{"start_line":66,"start_character":22,"end_line":66,"end_character":38},"updated":"2018-05-29 09:18:54.000000000","message":"Would be nice if you could also change this to reflect my comment below.","commit_id":"2899f934efe3150ecbf100731c7af95c63225ff9"},{"author":{"_account_id":27821,"name":"Noam Manos","email":"nmanos@redhat.com","username":"nmanos"},"change_message_id":"8a21d5eca77a1d2043585ef96dac407ea1378969","unresolved":false,"context_lines":[{"line_number":63,"context_line":"            object_details \u003d response"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        if object_details[status_key] \u003d\u003d status:"},{"line_number":66,"context_line":"            LOG.info(\u0027{name}\\\u0027s status updated to {status}.\u0027.format("},{"line_number":67,"context_line":"                name\u003dshow_client.__name__, status\u003dstatus))"},{"line_number":68,"context_line":"            return object_details"},{"line_number":69,"context_line":"        elif object_details[status_key] \u003d\u003d \u0027ERROR\u0027:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_557643a6","line":66,"range":{"start_line":66,"start_character":22,"end_line":66,"end_character":38},"in_reply_to":"5f7c97a3_983c1594","updated":"2018-05-29 12:38:24.000000000","message":"See Stacktrace where it happens:\n\nwait_for_status, waiters.py:80\n_cleanup_obj, base_client.py:371\nresource_cleanup, test.py:558\ntearDownClass, test.py:196\n\nVariable value at that point are:\n\nstatus \u003d ACTIVE\nstatus_key \u003d provisioning_status\nprovisioning_status \u003d DELETED\n\n\nLog trace:\n\n2018-05-29 08:21:21.195 25245 INFO octavia_tempest_plugin.services.load_balancer.v2.base_client [-] Starting cleanup for loadbalancer deee2dad-f8a9-41af-bed1-ebbd6bc7e6f4...\n2018-05-29 08:21:21.196 25245 INFO octavia_tempest_plugin.services.load_balancer.v2.base_client [-] Waiting for loadbalancer deee2dad-f8a9-41af-bed1-ebbd6bc7e6f4 to be ACTIVE...\n2018-05-29 08:21:21.197 25245 INFO octavia_tempest_plugin.tests.waiters [-] Waiting for _show_object status to update to ACTIVE\n2018-05-29 08:21:21.292 25245 INFO tempest.lib.common.rest_client [req-12882a1b-dc95-4681-9c30-44a1377b6540 ] Request (LoadBalancerScenarioTest:tearDownClass): 200 GET http://10.0.0.110:9876/v2.0/lbaas/loadbalancers/deee2dad-f8a9-41af-bed1-ebbd6bc7e6f4 0.092s\n2018-05-29 08:21:21.296 25245 DEBUG tempest.lib.common.rest_client [req-12882a1b-dc95-4681-9c30-44a1377b6540 ] Request - Headers: {\u0027Content-Type\u0027: \u0027application/json\u0027, \u0027Accept\u0027: \u0027application/json\u0027, \u0027X-Auth-Token\u0027: \u0027\u003comitted\u003e\u0027}\n        Body: None\n    Response - Headers: {\u0027status\u0027: \u0027200\u0027, u\u0027content-length\u0027: \u0027893\u0027, \u0027content-location\u0027: \u0027http://10.0.0.110:9876/v2.0/lbaas/loadbalancers/deee2dad-f8a9-41af-bed1-ebbd6bc7e6f4\u0027, u\u0027date\u0027: \u0027Tue, 29 May 2018 12:21:34 GMT\u0027, u\u0027content-type\u0027: \u0027application/json\u0027, u\u0027x-openstack-request-id\u0027: \u0027req-12882a1b-dc95-4681-9c30-44a1377b6540\u0027, u\u0027server\u0027: \u0027WSGIServer/0.1 Python/2.7.5\u0027}\n        Body: {\"loadbalancer\": {\"provider\": \"octavia\", \"description\": \"newnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnewnew\", \"admin_state_up\": true, \"pools\": [], \"created_at\": \"2018-05-29T11:59:19\", \"provisioning_status\": \"DELETED\", \"updated_at\": \"2018-05-29T12:00:55\", \"vip_qos_policy_id\": null, \"vip_network_id\": \"e858cc06-983e-4fd7-b407-74a2ca4eeb9f\", \"listeners\": [], \"vip_port_id\": \"abb7f8fc-9d38-4bbc-b86a-9828eb393191\", \"vip_address\": \"10.1.1.32\", \"vip_subnet_id\": \"211c9d59-153a-402f-b7d5-765759917639\", \"flavor\": \"\", \"project_id\": \"3c79aec2ffdb44c98485f902e52d7243\", \"id\": \"deee2dad-f8a9-41af-bed1-ebbd6bc7e6f4\", \"operating_status\": \"ONLINE\", \"name\": \"tempest-lb_member_lb1-update-1881186674\"}} _log_request_full /home/stack/.venv/lib/python2.7/site-packages/tempest/lib/common/rest_client.py:434\n2018-05-29 08:21:21.299 25245 INFO octavia_tempest_plugin.tests.waiters [-] _show_object\u0027s status updated to DELETED.\n2018-05-29 08:21:21.301 25245 INFO octavia_tempest_plugin.services.load_balancer.v2.base_client [-] Cleaning up loadbalancer deee2dad-f8a9-41af-bed1-ebbd6bc7e6f4...\n2018-05-29 08:21:21.381 25245 INFO tempest.lib.common.rest_client [req-0fc1f119-ff15-4e21-a95a-31ef837f372e ] Request (LoadBalancerScenarioTest:tearDownClass): 204 DELETE http://10.0.0.110:9876/v2.0/lbaas/loadbalancers/deee2dad-f8a9-41af-bed1-ebbd6bc7e6f4 0.077s\n2018-05-29 08:21:21.383 25245 DEBUG tempest.lib.common.rest_client [req-0fc1f119-ff15-4e21-a95a-31ef837f372e ] Request - Headers: {\u0027Content-Type\u0027: \u0027application/json\u0027, \u0027Accept\u0027: \u0027application/json\u0027, \u0027X-Auth-Token\u0027: \u0027\u003comitted\u003e\u0027}\n        Body: None\n    Response - Headers: {\u0027status\u0027: \u0027204\u0027, u\u0027content-length\u0027: \u00270\u0027, \u0027content-location\u0027: \u0027http://10.0.0.110:9876/v2.0/lbaas/loadbalancers/deee2dad-f8a9-41af-bed1-ebbd6bc7e6f4\u0027, u\u0027date\u0027: \u0027Tue, 29 May 2018 12:21:34 GMT\u0027, u\u0027x-openstack-request-id\u0027: \u0027req-0fc1f119-ff15-4e21-a95a-31ef837f372e\u0027, u\u0027server\u0027: \u0027WSGIServer/0.1 Python/2.7.5\u0027}\n        Body:  _log_request_full /home/stack/.venv/lib/python2.7/site-packages/tempest/lib/common/rest_client.py:434\n2018-05-29 08:21:21.385 25245 INFO octavia_tempest_plugin.services.load_balancer.v2.base_client [-] Cleanup complete for loadbalancer deee2dad-f8a9-41af-bed1-ebbd6bc7e6f4...","commit_id":"2899f934efe3150ecbf100731c7af95c63225ff9"},{"author":{"_account_id":6469,"name":"Carlos Gonçalves","display_name":"Carlos Goncalves","email":"cgoncalves@redhat.com","username":"cgoncalves"},"change_message_id":"151fc4f6da2fea627e115b5122ce0b3770ec1ff6","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                                                        message\u003dmessage)"},{"line_number":77,"context_line":"            raise exceptions.UnexpectedResponseCode(message)"},{"line_number":78,"context_line":"        elif object_details[status_key] \u003d\u003d const.DELETED:"},{"line_number":79,"context_line":"            LOG.info(\u0027{name}\\\u0027s status updated to DELETED.\u0027.format("},{"line_number":80,"context_line":"                name\u003dshow_client.__name__))"},{"line_number":81,"context_line":"            return object_details"},{"line_number":82,"context_line":"        elif int(time.time()) - start \u003e\u003d check_timeout:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_7837b9b1","line":79,"range":{"start_line":79,"start_character":22,"end_line":79,"end_character":38},"updated":"2018-05-29 09:18:54.000000000","message":"Status key could be provisioning_status or operating_status. While \"DELETED\" is only valid for provisioning_status, make it more explicit in log by following other similar messages: \"{name} {field} updated to...\"","commit_id":"2899f934efe3150ecbf100731c7af95c63225ff9"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"c74afa04644d33047df253cadbfa3ff4150ca302","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                message \u003d \u0027({caller}) {message}\u0027.format(caller\u003dcaller,"},{"line_number":77,"context_line":"                                                        message\u003dmessage)"},{"line_number":78,"context_line":"            raise exceptions.UnexpectedResponseCode(message)"},{"line_number":79,"context_line":"        elif object_details[status_key] \u003d\u003d const.DELETED:"},{"line_number":80,"context_line":"            LOG.info(\u0027{name} {field} updated to DELETED.\u0027.format("},{"line_number":81,"context_line":"                name\u003dshow_client.__name__, field\u003dstatus_key))"},{"line_number":82,"context_line":"            return object_details"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_f5481988","line":79,"updated":"2018-06-06 08:54:53.000000000","message":"This means that if I am waiting for \"ACTIVE\" status, and something causes the object to be deleted, then this function will return successfully.\n\nReally what you would want to do is just change line 70 above to do the check like this:\n\nelif object_details[status_key] in (\u0027ERROR\u0027, \u0027DELETED\u0027)\n\nwhich would make sense, because those are both final states that nothing is coming back from, so we may as well short-circuit.","commit_id":"6b373f0718b38da0fe5851c318f877ca5587b057"},{"author":{"_account_id":10273,"name":"Adam Harwell","email":"flux.adam@gmail.com","username":"rm_you"},"change_message_id":"6c726a058efc42b866d44fb207acfd1330fa1f78","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                message \u003d \u0027({caller}) {message}\u0027.format(caller\u003dcaller,"},{"line_number":77,"context_line":"                                                        message\u003dmessage)"},{"line_number":78,"context_line":"            raise exceptions.UnexpectedResponseCode(message)"},{"line_number":79,"context_line":"        elif object_details[status_key] \u003d\u003d const.DELETED:"},{"line_number":80,"context_line":"            LOG.info(\u0027{name} {field} updated to DELETED.\u0027.format("},{"line_number":81,"context_line":"                name\u003dshow_client.__name__, field\u003dstatus_key))"},{"line_number":82,"context_line":"            return object_details"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_f91eb40b","line":79,"in_reply_to":"5f7c97a3_3dfd173b","updated":"2018-06-19 17:10:39.000000000","message":"Actually -- revising my review. I noticed something else.\nIf the item is DELETED, we will never even make it here.\nIf we\u0027re *expecting* DELETED and it is, it will return on line 57. If we\u0027re expecting something else (ACTIVE/whatever) and it is DELETED, it will actually 404 on line 59 and the exception isn\u0027t caught so it will be raised.\nHaving an elif for DELETED here is unreachable.","commit_id":"6b373f0718b38da0fe5851c318f877ca5587b057"},{"author":{"_account_id":27821,"name":"Noam Manos","email":"nmanos@redhat.com","username":"nmanos"},"change_message_id":"a849a1e243f6b4060a4ab6edbb44096c7dc31cf6","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                message \u003d \u0027({caller}) {message}\u0027.format(caller\u003dcaller,"},{"line_number":77,"context_line":"                                                        message\u003dmessage)"},{"line_number":78,"context_line":"            raise exceptions.UnexpectedResponseCode(message)"},{"line_number":79,"context_line":"        elif object_details[status_key] \u003d\u003d const.DELETED:"},{"line_number":80,"context_line":"            LOG.info(\u0027{name} {field} updated to DELETED.\u0027.format("},{"line_number":81,"context_line":"                name\u003dshow_client.__name__, field\u003dstatus_key))"},{"line_number":82,"context_line":"            return object_details"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_120338c0","line":79,"in_reply_to":"5f7c97a3_4f1b487f","updated":"2018-06-20 09:53:12.000000000","message":"Even worst - \nWithout the patch: test_load_balancer passed in 2160 seconds.\n\nWith the patch: test_load_balancer passed in 232 seconds.\n\nAlmost 10 times faster.","commit_id":"6b373f0718b38da0fe5851c318f877ca5587b057"},{"author":{"_account_id":27821,"name":"Noam Manos","email":"nmanos@redhat.com","username":"nmanos"},"change_message_id":"53518da1296a905c8d3c48a494dceacfe006a526","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                message \u003d \u0027({caller}) {message}\u0027.format(caller\u003dcaller,"},{"line_number":77,"context_line":"                                                        message\u003dmessage)"},{"line_number":78,"context_line":"            raise exceptions.UnexpectedResponseCode(message)"},{"line_number":79,"context_line":"        elif object_details[status_key] \u003d\u003d const.DELETED:"},{"line_number":80,"context_line":"            LOG.info(\u0027{name} {field} updated to DELETED.\u0027.format("},{"line_number":81,"context_line":"                name\u003dshow_client.__name__, field\u003dstatus_key))"},{"line_number":82,"context_line":"            return object_details"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_3dfd173b","line":79,"in_reply_to":"5f7c97a3_f5481988","updated":"2018-06-06 09:34:23.000000000","message":"I see your point. \nHowever, in case of ERROR status - you\u0027ll need to raise exceptions.UnexpectedResponseCode(message). Whereas in DELETED status - it\u0027s an expected behavior (no need to raise exception).\n\nSo anyhow, if fixing line 70 to deal with both cases, we\u0027ll need to add a nested if statement - to raise exception for an ERROR status. \n\nIs that the preferred resolution ?","commit_id":"6b373f0718b38da0fe5851c318f877ca5587b057"},{"author":{"_account_id":27821,"name":"Noam Manos","email":"nmanos@redhat.com","username":"nmanos"},"change_message_id":"8436e57e00d6585a40174028c03d162ae721c361","unresolved":false,"context_lines":[{"line_number":76,"context_line":"                message \u003d \u0027({caller}) {message}\u0027.format(caller\u003dcaller,"},{"line_number":77,"context_line":"                                                        message\u003dmessage)"},{"line_number":78,"context_line":"            raise exceptions.UnexpectedResponseCode(message)"},{"line_number":79,"context_line":"        elif object_details[status_key] \u003d\u003d const.DELETED:"},{"line_number":80,"context_line":"            LOG.info(\u0027{name} {field} updated to DELETED.\u0027.format("},{"line_number":81,"context_line":"                name\u003dshow_client.__name__, field\u003dstatus_key))"},{"line_number":82,"context_line":"            return object_details"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f7c97a3_4f1b487f","line":79,"in_reply_to":"5f7c97a3_f91eb40b","updated":"2018-06-20 08:58:57.000000000","message":"Adam, please see debug stacktrace and variables values at my previous comment - in Patch Set 3.\n\nI\u0027ve just verified it again on my environment (OSP13 with Octavia): When running test_load_balancer.py - it is getting into object_details[status_key] \u003d const.DELETED.\nBut at this while loop, the condition in line 53 is false, for many iterations, and thus not returning (in line 57).\n\nOn my environment, if not using this patch, it will return eventually, but instead of finishing after ~5 minutes, it will finish after ~20 minutes!","commit_id":"6b373f0718b38da0fe5851c318f877ca5587b057"}]}