)]}'
{"tempest/api/compute/base.py":[{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":205,"context_line":"    @classmethod"},{"line_number":206,"context_line":"    def create_test_server(cls, **kwargs):"},{"line_number":207,"context_line":"        \"\"\"Wrapper utility that calls the common create test server and"},{"line_number":208,"context_line":"           returns a test server."},{"line_number":209,"context_line":"        \"\"\""},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"        body \u003d server_ssh.create_test_server(cls, **kwargs)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_83cf5855","line":208,"updated":"2015-02-25 15:55:06.000000000","message":"I think we should have a comment here about the purpose of this wrapper i.e. to minimize the impact on the test code which uses this function already.","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":5803,"name":"Attila Fazekas","email":"afazekas@redhat.com","username":"afazekas"},"change_message_id":"819f94bddbacf44e02c2e4eec9e6b23dd3b86152","unresolved":false,"context_lines":[{"line_number":212,"context_line":"        the impact on the code of the tests already using this"},{"line_number":213,"context_line":"        function."},{"line_number":214,"context_line":"        \"\"\""},{"line_number":215,"context_line":"        body \u003d compute.create_test_server(cls, validatable,"},{"line_number":216,"context_line":"                                          validation_resources, **kwargs)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        return body"}],"source_content_type":"text/x-python","patch_set":32,"id":"dac4157f_373fe1d7","line":215,"updated":"2015-04-22 10:55:34.000000000","message":"You call the method with cls, the method uses attributes from cls. Why did you moved it from here ?","commit_id":"f215950dc4ba78c0a6055c3ad1913d8a6567a72d"},{"author":{"_account_id":10016,"name":"Joseph Lanoux","email":"joseph.lanoux@hpe.com","username":"lanoux"},"change_message_id":"55cc3237f402dc04d0dbab6168c9883b71d07d87","unresolved":false,"context_lines":[{"line_number":212,"context_line":"        the impact on the code of the tests already using this"},{"line_number":213,"context_line":"        function."},{"line_number":214,"context_line":"        \"\"\""},{"line_number":215,"context_line":"        body \u003d compute.create_test_server(cls, validatable,"},{"line_number":216,"context_line":"                                          validation_resources, **kwargs)"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"        return body"}],"source_content_type":"text/x-python","patch_set":32,"id":"7ad32947_50dd1b13","line":215,"in_reply_to":"dac4157f_373fe1d7","updated":"2015-04-28 08:20:16.000000000","message":"I only kept this wrapper here for simplicity. In this way, the impact on all the tests in reduced. I then call the method with cls as the new compute method needs some of the clients.","commit_id":"f215950dc4ba78c0a6055c3ad1913d8a6567a72d"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"22ed4d0e703a674432be91987e9b299acb186b18","unresolved":false,"context_lines":[{"line_number":191,"context_line":"                              server_group_id)"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    @classmethod"},{"line_number":194,"context_line":"    def create_test_server(cls, validatable\u003dFalse, **kwargs):"},{"line_number":195,"context_line":"        \"\"\""},{"line_number":196,"context_line":"        Wrapper utility that calls the common create test server and"},{"line_number":197,"context_line":"        returns a test server. The purpose of this wrapper is to minimize"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_fbcf8526","line":194,"updated":"2015-06-11 08:42:16.000000000","message":"I do not much prefer the wrapper of common create server function. All test should use the commonly defined function which should have all options needed by tests.\n\nI think that was the goal of having common function. Please let me know if you have same plan and planning to do that in further patches?","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":10016,"name":"Joseph Lanoux","email":"joseph.lanoux@hpe.com","username":"lanoux"},"change_message_id":"3de35dd9c166449bef20ae903052af275a924e10","unresolved":false,"context_lines":[{"line_number":191,"context_line":"                              server_group_id)"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    @classmethod"},{"line_number":194,"context_line":"    def create_test_server(cls, validatable\u003dFalse, **kwargs):"},{"line_number":195,"context_line":"        \"\"\""},{"line_number":196,"context_line":"        Wrapper utility that calls the common create test server and"},{"line_number":197,"context_line":"        returns a test server. The purpose of this wrapper is to minimize"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_86c27883","line":194,"in_reply_to":"3afb71cf_fbcf8526","updated":"2015-06-11 09:58:21.000000000","message":"Exactly. After discussion with Andrea, we decided that as a first step, having a wrapper was the best solution as it implies less code change.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":7227,"name":"Timur Nurlygayanov","email":"tnurlygayanov@mirantis.com","username":"tnurlygayanov"},"change_message_id":"e67b049e7dbfec8d174ae7cce4bcb3ef9e2f2277","unresolved":false,"context_lines":[{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    @classmethod"},{"line_number":194,"context_line":"    def create_test_server(cls, validatable\u003dFalse, **kwargs):"},{"line_number":195,"context_line":"        \"\"\""},{"line_number":196,"context_line":"        Wrapper utility that calls the common create test server and"},{"line_number":197,"context_line":"        returns a test server. The purpose of this wrapper is to minimize"},{"line_number":198,"context_line":"        the impact on the code of the tests already using this"}],"source_content_type":"text/x-python","patch_set":44,"id":"fa32b979_72ccb689","line":195,"updated":"2015-06-15 13:48:47.000000000","message":"minor issue: could you please start the doc string from the first line, like described in code guide:\n\nhttp://docs.openstack.org/developer/hacking/#docstrings","commit_id":"011c1a5642d3fde04074bced78126325640a0ee2"}],"tempest/api/compute/servers/test_create_server.py":[{"author":{"_account_id":15637,"name":"Dmitry Ratushnyy","email":"d.ratushnyy@gmail.com","username":"dratushnyy"},"change_message_id":"98e04af6eb56964eeae92d45c29783a0ba040466","unresolved":false,"context_lines":[{"line_number":47,"context_line":"            cls.set_validation_resources(security_group\u003dTrue,"},{"line_number":48,"context_line":"                                         security_group_rules\u003dTrue)"},{"line_number":49,"context_line":"            validatable \u003d True"},{"line_number":50,"context_line":"        else:"},{"line_number":51,"context_line":"            validatable \u003d False"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        super(ServersTestJSON, cls).resource_setup()"}],"source_content_type":"text/x-python","patch_set":26,"id":"da9b358b_df3b40ea","line":50,"updated":"2015-03-31 15:36:45.000000000","message":"unnecessary else - you can put validatable \u003d False before if","commit_id":"4942c17c16ca3ada73c9e39a51b301cbd1ce1d14"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"b4ce91a7b94c707ecada616be9219fb4d241dbdd","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        cls.network_client \u003d cls.os.network_client"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    @classmethod"},{"line_number":45,"context_line":"    def resource_setup(cls):"},{"line_number":46,"context_line":"        if CONF.compute.run_ssh:"},{"line_number":47,"context_line":"            cls.set_validation_resources()"},{"line_number":48,"context_line":"            validatable \u003d True"},{"line_number":49,"context_line":"        else:"},{"line_number":50,"context_line":"            validatable \u003d False"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        super(ServersTestJSON, cls).resource_setup()"},{"line_number":53,"context_line":"        cls.meta \u003d {\u0027hello\u0027: \u0027world\u0027}"}],"source_content_type":"text/x-python","patch_set":35,"id":"9a0c5dc1_af4e38bf","line":50,"range":{"start_line":45,"start_character":0,"end_line":50,"end_character":31},"updated":"2015-05-12 20:29:41.000000000","message":"This is not correct. The test should decide know whether it wants validatable servers or not. A global switch in validation resources (https://review.openstack.org/#/c/182041/) can be used to turn validation on/off for the whole test run.","commit_id":"7241132f06cddcf7fd2b11f36a77448508dde446"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"b4ce91a7b94c707ecada616be9219fb4d241dbdd","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        disk_config \u003d cls.disk_config"},{"line_number":61,"context_line":"        cls.server_initial \u003d cls.create_test_server("},{"line_number":62,"context_line":"            validatable,"},{"line_number":63,"context_line":"            validation_resources\u003dcls.validation_resources,"},{"line_number":64,"context_line":"            wait_until\u003d\u0027ACTIVE\u0027,"},{"line_number":65,"context_line":"            name\u003dcls.name,"},{"line_number":66,"context_line":"            meta\u003dcls.meta,"}],"source_content_type":"text/x-python","patch_set":35,"id":"9a0c5dc1_a293eff9","line":63,"range":{"start_line":63,"start_character":12,"end_line":63,"end_character":57},"updated":"2015-05-12 20:29:41.000000000","message":"This should be set by wrapper function and not exposed to tests. We always want to pass validation resources in.","commit_id":"7241132f06cddcf7fd2b11f36a77448508dde446"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"b4ce91a7b94c707ecada616be9219fb4d241dbdd","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        cls.server_initial \u003d cls.create_test_server("},{"line_number":62,"context_line":"            validatable,"},{"line_number":63,"context_line":"            validation_resources\u003dcls.validation_resources,"},{"line_number":64,"context_line":"            wait_until\u003d\u0027ACTIVE\u0027,"},{"line_number":65,"context_line":"            name\u003dcls.name,"},{"line_number":66,"context_line":"            meta\u003dcls.meta,"},{"line_number":67,"context_line":"            accessIPv4\u003dcls.accessIPv4,"}],"source_content_type":"text/x-python","patch_set":35,"id":"9a0c5dc1_05cf65b1","line":64,"updated":"2015-05-12 20:29:41.000000000","message":"This seems like an unrelated change. Why adding the wait_until\u003d\u0027ACTIVE\u0027 if it was not used before?","commit_id":"7241132f06cddcf7fd2b11f36a77448508dde446"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"cc984b187ed1941d5d390db94adf172ac1565ba3","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    @classmethod"},{"line_number":45,"context_line":"    def resource_setup(cls):"},{"line_number":46,"context_line":"        cls.set_validation_resources()"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"        super(ServersTestJSON, cls).resource_setup()"},{"line_number":49,"context_line":"        cls.meta \u003d {\u0027hello\u0027: \u0027world\u0027}"},{"line_number":50,"context_line":"        cls.accessIPv4 \u003d \u00271.1.1.1\u0027"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_9243f912","line":47,"updated":"2015-06-11 14:46:28.000000000","message":"NIT: additional line","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"22ed4d0e703a674432be91987e9b299acb186b18","unresolved":false,"context_lines":[{"line_number":55,"context_line":"                       \u0027contents\u0027: base64.b64encode(file_contents)}]"},{"line_number":56,"context_line":"        disk_config \u003d cls.disk_config"},{"line_number":57,"context_line":"        cls.server_initial \u003d cls.create_test_server("},{"line_number":58,"context_line":"            validatable\u003dTrue,"},{"line_number":59,"context_line":"            wait_until\u003d\u0027ACTIVE\u0027,"},{"line_number":60,"context_line":"            name\u003dcls.name,"},{"line_number":61,"context_line":"            meta\u003dcls.meta,"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_fb330560","line":58,"updated":"2015-06-11 08:42:16.000000000","message":"why validatable True in this, it will start logging error as no sec grp, key_name","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"78de8a6cea65bc613b5f66339e84126800063b6a","unresolved":false,"context_lines":[{"line_number":55,"context_line":"                       \u0027contents\u0027: base64.b64encode(file_contents)}]"},{"line_number":56,"context_line":"        disk_config \u003d cls.disk_config"},{"line_number":57,"context_line":"        cls.server_initial \u003d cls.create_test_server("},{"line_number":58,"context_line":"            validatable\u003dTrue,"},{"line_number":59,"context_line":"            wait_until\u003d\u0027ACTIVE\u0027,"},{"line_number":60,"context_line":"            name\u003dcls.name,"},{"line_number":61,"context_line":"            meta\u003dcls.meta,"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_f9e2ead3","line":58,"in_reply_to":"3afb71cf_a6ea14d9","updated":"2015-06-11 10:56:20.000000000","message":"Yes, But my concern is we do not create any validation resource in this test (L46) then why validatable\u003dTrue. \n\nWhen run_validation (config) will be true then it will unnecessarily log the msg from compute.py","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"cc984b187ed1941d5d390db94adf172ac1565ba3","unresolved":false,"context_lines":[{"line_number":55,"context_line":"                       \u0027contents\u0027: base64.b64encode(file_contents)}]"},{"line_number":56,"context_line":"        disk_config \u003d cls.disk_config"},{"line_number":57,"context_line":"        cls.server_initial \u003d cls.create_test_server("},{"line_number":58,"context_line":"            validatable\u003dTrue,"},{"line_number":59,"context_line":"            wait_until\u003d\u0027ACTIVE\u0027,"},{"line_number":60,"context_line":"            name\u003dcls.name,"},{"line_number":61,"context_line":"            meta\u003dcls.meta,"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_4d78112c","line":58,"in_reply_to":"3afb71cf_f9e2ead3","updated":"2015-06-11 14:46:28.000000000","message":"There are three steps involved in doing server validation:\n- run_validation configuration option must be set to True\n- set_validation_resources is invoked\n- the server is created as validatable\n\nIf run_validation is False, set_validation_resources will not create the resources, and create_test_server will ignore validatable.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":10016,"name":"Joseph Lanoux","email":"joseph.lanoux@hpe.com","username":"lanoux"},"change_message_id":"3de35dd9c166449bef20ae903052af275a924e10","unresolved":false,"context_lines":[{"line_number":55,"context_line":"                       \u0027contents\u0027: base64.b64encode(file_contents)}]"},{"line_number":56,"context_line":"        disk_config \u003d cls.disk_config"},{"line_number":57,"context_line":"        cls.server_initial \u003d cls.create_test_server("},{"line_number":58,"context_line":"            validatable\u003dTrue,"},{"line_number":59,"context_line":"            wait_until\u003d\u0027ACTIVE\u0027,"},{"line_number":60,"context_line":"            name\u003dcls.name,"},{"line_number":61,"context_line":"            meta\u003dcls.meta,"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_a6ea14d9","line":58,"in_reply_to":"3afb71cf_fb330560","updated":"2015-06-11 09:58:21.000000000","message":"The big switch for ssh validation is the run_validation parameter. It is used to create the resources. The validatable parameter is used as a small switch for each test. The ssh validation is conditional to those 2 parameters in the new compute.py method.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"b2d09912cead60a7d4cad01dcea0759d4773e988","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    def resource_setup(cls):"},{"line_number":46,"context_line":"        cls.set_validation_resources()"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"        if CONF.validation.run_validation:"},{"line_number":49,"context_line":"            validatable \u003d True"},{"line_number":50,"context_line":"        else:"},{"line_number":51,"context_line":"            validatable \u003d False"}],"source_content_type":"text/x-python","patch_set":40,"id":"3afb71cf_ad39c5f4","line":48,"updated":"2015-06-11 14:55:28.000000000","message":"Why this? In my view, tests asks for a validatable server. If run_validation is off it will not get one.\n\nThere is still the need for checking run_validation later in the tests for now, once the function will return (or not return) a RemoteClient, that should be used to decide whether we can attempt validation or not.\nSome tests may decide to skip completely if run_validation is not true.","commit_id":"75add7a43d0451c8bb5727b045dc38f81609dec1"}],"tempest/common/compute.py":[{"author":{"_account_id":15637,"name":"Dmitry Ratushnyy","email":"d.ratushnyy@gmail.com","username":"dratushnyy"},"change_message_id":"e882ea876e4013bef7d13cb4ddcf879c21c831cc","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        if \u0027security_groups\u0027 in kwargs:"},{"line_number":59,"context_line":"            kwargs[\u0027security_groups\u0027].append("},{"line_number":60,"context_line":"                {\u0027name\u0027: validation_resources[\u0027security_group\u0027][\u0027name\u0027]})"},{"line_number":61,"context_line":"        if \u0027security_groups\u0027 not in kwargs:"},{"line_number":62,"context_line":"            try:"},{"line_number":63,"context_line":"                kwargs[\u0027security_groups\u0027] \u003d \\"},{"line_number":64,"context_line":"                    [{\u0027name\u0027: validation_resources[\u0027security_group\u0027][\u0027name\u0027]}]"}],"source_content_type":"text/x-python","patch_set":23,"id":"da9b358b_62ba99e3","line":61,"updated":"2015-03-30 09:05:50.000000000","message":"use else instead","commit_id":"401a4a215460ca13a13a29fbe4567128c59c160a"},{"author":{"_account_id":15637,"name":"Dmitry Ratushnyy","email":"d.ratushnyy@gmail.com","username":"dratushnyy"},"change_message_id":"e882ea876e4013bef7d13cb4ddcf879c21c831cc","unresolved":false,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    # As a first implementation, multiple pingable or sshable servers will not"},{"line_number":100,"context_line":"    # be supported"},{"line_number":101,"context_line":"    if validatable:"},{"line_number":102,"context_line":"        if len(servers) \u003e 1:"},{"line_number":103,"context_line":"            LOG.warn(\"Multiple pingable or sshable servers not supported \\"},{"line_number":104,"context_line":"                this stage.\")"}],"source_content_type":"text/x-python","patch_set":23,"id":"da9b358b_62a179a9","line":101,"updated":"2015-03-30 09:05:50.000000000","message":"Duplicate of condition on line 57","commit_id":"401a4a215460ca13a13a29fbe4567128c59c160a"},{"author":{"_account_id":15637,"name":"Dmitry Ratushnyy","email":"d.ratushnyy@gmail.com","username":"dratushnyy"},"change_message_id":"e5af0fb08460d2d7a0cf1df91cab83463c18653e","unresolved":false,"context_lines":[{"line_number":46,"context_line":"    if \u0027name\u0027 in kwargs:"},{"line_number":47,"context_line":"        name \u003d kwargs.pop(\u0027name\u0027)"},{"line_number":48,"context_line":"    else:"},{"line_number":49,"context_line":"        name \u003d data_utils.rand_name(__name__ + \"-instance\")"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    flavor \u003d kwargs.get(\u0027flavor\u0027, CONF.compute.flavor_ref)"},{"line_number":52,"context_line":"    image_id \u003d kwargs.get(\u0027image_id\u0027, CONF.compute.image_ref)"}],"source_content_type":"text/x-python","patch_set":24,"id":"da9b358b_0624fc0a","line":49,"updated":"2015-03-30 12:09:02.000000000","message":"name \u003d kwargs.get(\u0027name\u0027, data_utils.rand_name(__name__ + \"-instance\"))","commit_id":"af03232bac4d485bc7aa473656bad74294debac7"},{"author":{"_account_id":15637,"name":"Dmitry Ratushnyy","email":"d.ratushnyy@gmail.com","username":"dratushnyy"},"change_message_id":"e5af0fb08460d2d7a0cf1df91cab83463c18653e","unresolved":false,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    # As a first implementation, multiple pingable or sshable servers will not"},{"line_number":100,"context_line":"    # be supported"},{"line_number":101,"context_line":"    if validatable:"},{"line_number":102,"context_line":"        if len(servers) \u003e 1:"},{"line_number":103,"context_line":"            LOG.warn(\"Multiple pingable or sshable servers not supported \\"},{"line_number":104,"context_line":"                this stage.\")"}],"source_content_type":"text/x-python","patch_set":24,"id":"da9b358b_86bb6cc4","line":101,"updated":"2015-03-30 12:09:02.000000000","message":"duplicate condition: firs appearance in line 57","commit_id":"af03232bac4d485bc7aa473656bad74294debac7"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"99a9761526b22876c3badc2c8961990556b31389","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright (c) 2015 Hewlett-Packard Development Company, L.P."},{"line_number":2,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\");"},{"line_number":3,"context_line":"#    you may not use this file except in compliance with the License."},{"line_number":4,"context_line":"#    You may obtain a copy of the License at"}],"source_content_type":"text/x-python","patch_set":27,"id":"baa041b7_fb41930c","line":1,"updated":"2015-04-02 22:36:06.000000000","message":"NIT: most files have \"All Rights Reserved.\" on the second line and then a blank line.","commit_id":"1f60453122f13479d1c8e185be4cc6f13530b9ea"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"99a9761526b22876c3badc2c8961990556b31389","unresolved":false,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"def create_test_server(cls, validatable\u003dFalse, validation_resources\u003dNone,"},{"line_number":35,"context_line":"                       **kwargs):"},{"line_number":36,"context_line":"    \"\"\""},{"line_number":37,"context_line":"    Common wrapper utility returning a test server that can be pingable or"},{"line_number":38,"context_line":"    sshable."},{"line_number":39,"context_line":"    validatable: Whether the server will be pingable or sshable."}],"source_content_type":"text/x-python","patch_set":27,"id":"baa041b7_7b56a3d3","line":36,"updated":"2015-04-02 22:36:06.000000000","message":"NIT: docstring formatting (http://docs.openstack.org/developer/hacking/#docstrings)","commit_id":"1f60453122f13479d1c8e185be4cc6f13530b9ea"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"99a9761526b22876c3badc2c8961990556b31389","unresolved":false,"context_lines":[{"line_number":57,"context_line":"    if validatable:"},{"line_number":58,"context_line":"        if \u0027security_groups\u0027 in kwargs:"},{"line_number":59,"context_line":"            kwargs[\u0027security_groups\u0027].append("},{"line_number":60,"context_line":"                {\u0027name\u0027: validation_resources[\u0027security_group\u0027][\u0027name\u0027]})"},{"line_number":61,"context_line":"        else:"},{"line_number":62,"context_line":"            try:"},{"line_number":63,"context_line":"                kwargs[\u0027security_groups\u0027] \u003d \\"}],"source_content_type":"text/x-python","patch_set":27,"id":"baa041b7_1be1d7d0","line":60,"updated":"2015-04-02 22:36:06.000000000","message":"\u0027security_group\u0027 might not be in validation_resources, so you should handle that case","commit_id":"1f60453122f13479d1c8e185be4cc6f13530b9ea"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"99a9761526b22876c3badc2c8961990556b31389","unresolved":false,"context_lines":[{"line_number":62,"context_line":"            try:"},{"line_number":63,"context_line":"                kwargs[\u0027security_groups\u0027] \u003d \\"},{"line_number":64,"context_line":"                    [{\u0027name\u0027: validation_resources[\u0027security_group\u0027][\u0027name\u0027]}]"},{"line_number":65,"context_line":"            except exceptions.NotFound:"},{"line_number":66,"context_line":"                LOG.warn(\"No key provided.\")"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        if \u0027key_name\u0027 not in kwargs:"}],"source_content_type":"text/x-python","patch_set":27,"id":"baa041b7_1ba557b4","line":65,"updated":"2015-04-02 22:36:06.000000000","message":"NotFound is a 404 reply to an API call - I don\u0027t see the API call in the try/except.\n\n\u0027security_group\u0027 might not be in validation_resources, so you should handle that case","commit_id":"1f60453122f13479d1c8e185be4cc6f13530b9ea"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"99a9761526b22876c3badc2c8961990556b31389","unresolved":false,"context_lines":[{"line_number":63,"context_line":"                kwargs[\u0027security_groups\u0027] \u003d \\"},{"line_number":64,"context_line":"                    [{\u0027name\u0027: validation_resources[\u0027security_group\u0027][\u0027name\u0027]}]"},{"line_number":65,"context_line":"            except exceptions.NotFound:"},{"line_number":66,"context_line":"                LOG.warn(\"No key provided.\")"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        if \u0027key_name\u0027 not in kwargs:"},{"line_number":69,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":27,"id":"baa041b7_bbd2ebfe","line":66,"updated":"2015-04-02 22:36:06.000000000","message":"\"No security group provided\". \nWhy a warning? I think DEBUG would enough here.","commit_id":"1f60453122f13479d1c8e185be4cc6f13530b9ea"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"99a9761526b22876c3badc2c8961990556b31389","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        if \u0027key_name\u0027 not in kwargs:"},{"line_number":69,"context_line":"            try:"},{"line_number":70,"context_line":"                kwargs[\u0027key_name\u0027] \u003d validation_resources[\u0027keypair\u0027][\u0027name\u0027]"},{"line_number":71,"context_line":"            except exceptions.NotFound:"},{"line_number":72,"context_line":"                LOG.warn(\"No security group provided.\")"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    body \u003d cls.servers_client.create_server(name, image_id, flavor, **kwargs)"}],"source_content_type":"text/x-python","patch_set":27,"id":"baa041b7_bedc19e5","line":71,"updated":"2015-04-02 22:36:06.000000000","message":"Ditto as L65","commit_id":"1f60453122f13479d1c8e185be4cc6f13530b9ea"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"99a9761526b22876c3badc2c8961990556b31389","unresolved":false,"context_lines":[{"line_number":69,"context_line":"            try:"},{"line_number":70,"context_line":"                kwargs[\u0027key_name\u0027] \u003d validation_resources[\u0027keypair\u0027][\u0027name\u0027]"},{"line_number":71,"context_line":"            except exceptions.NotFound:"},{"line_number":72,"context_line":"                LOG.warn(\"No security group provided.\")"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    body \u003d cls.servers_client.create_server(name, image_id, flavor, **kwargs)"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"baa041b7_fed181ce","line":72,"updated":"2015-04-02 22:36:06.000000000","message":"\"No key provided\".\nDitto as L66","commit_id":"1f60453122f13479d1c8e185be4cc6f13530b9ea"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"99a9761526b22876c3badc2c8961990556b31389","unresolved":false,"context_lines":[{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    # As a first implementation, multiple pingable or sshable servers will not"},{"line_number":100,"context_line":"    # be supported"},{"line_number":101,"context_line":"    if validatable:"},{"line_number":102,"context_line":"        if len(servers) \u003e 1:"},{"line_number":103,"context_line":"            LOG.warn(\"Multiple pingable or sshable servers not supported \\"},{"line_number":104,"context_line":"                this stage.\")"}],"source_content_type":"text/x-python","patch_set":27,"id":"baa041b7_bbfaebbb","line":101,"updated":"2015-04-02 22:36:06.000000000","message":"I think it\u0027s fair to limit this to one server for now, but it might be better to check this at the beginning, and fail, rather than logging a warning, as the result won\u0027t be what\u0027s expected.","commit_id":"1f60453122f13479d1c8e185be4cc6f13530b9ea"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"b4ce91a7b94c707ecada616be9219fb4d241dbdd","unresolved":false,"context_lines":[{"line_number":25,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"def _associate_floating_ip(cls, floating_ip, server):"},{"line_number":29,"context_line":"    \"\"\"Associate a floating IP to the test server.\"\"\""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    cls.floating_ips_client.associate_floating_ip_to_server("}],"source_content_type":"text/x-python","patch_set":35,"id":"9a0c5dc1_e520d14b","line":28,"updated":"2015-05-12 20:29:41.000000000","message":"I\u0027m not convinced a one-liner warrants a helper method here.\n\nIf you want to keep it, I think it should then accept a client rather then \"cls\"","commit_id":"7241132f06cddcf7fd2b11f36a77448508dde446"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"b4ce91a7b94c707ecada616be9219fb4d241dbdd","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        floating_ip\u003dfloating_ip[\u0027ip\u0027], server_id\u003dserver[\u0027id\u0027])"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"def create_test_server(cls, validatable\u003dFalse, validation_resources\u003dNone,"},{"line_number":36,"context_line":"                       **kwargs):"},{"line_number":37,"context_line":"    \"\"\"Common wrapper utility returning a test server."},{"line_number":38,"context_line":""}],"source_content_type":"text/x-python","patch_set":35,"id":"9a0c5dc1_20cff7b1","line":35,"updated":"2015-05-12 20:29:41.000000000","message":"Passing \"cls\" like this as a parameter does not seem ideal to me, as it relies on a non-specified set of expectations on the \"cls\" object.\n\nAlso it imposes un-necessary restrictions on the set of credentials with which the server is created.\n\nLooking through the code I see you use the following from \"cls\":\n- a client manager and its clients \u003d\u003e it would be good to pass it as a parameter\n- the get_tenant_network() function \u003d\u003e I think it would be fair to require here the tenant_network as an optional parameter. We can keep the call to set_networks_kwargs in here, and let the call to get_tenant_network to be done by the caller\n- the servers list \u003d\u003e this should be returned (which is the case already), and the caller should care about adding the servers to whatever mechanism is used to for cleanup (which will be different for instance if servers are created during the class setup  - resource cleanup - or during a test - addCleanup).\n\ndef create_test_server(clients, validatable\u003dFalse, validation_resources\u003dNone, tenant_network\u003dNone, **kwargs)","commit_id":"7241132f06cddcf7fd2b11f36a77448508dde446"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"b4ce91a7b94c707ecada616be9219fb4d241dbdd","unresolved":false,"context_lines":[{"line_number":111,"context_line":"        if CONF.validation.connect_method \u003d\u003d \u0027floating\u0027:"},{"line_number":112,"context_line":"            _associate_floating_ip("},{"line_number":113,"context_line":"                cls,"},{"line_number":114,"context_line":"                floating_ip\u003dvalidation_resources[\u0027floating_ip\u0027],"},{"line_number":115,"context_line":"                server\u003dservers[0])"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    cls.servers.extend(servers)"}],"source_content_type":"text/x-python","patch_set":35,"id":"9a0c5dc1_e092ff56","line":114,"updated":"2015-05-12 20:29:41.000000000","message":"You should probably handle the case on no floating_ip defined, else you would fail here, not return the created server and leak it.\nIt would probably be better to check this before creating the server.","commit_id":"7241132f06cddcf7fd2b11f36a77448508dde446"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"5cdee4a2f665e9a3d5cde7d3dd9ebb605d5314f1","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            try:"},{"line_number":75,"context_line":"                kwargs[\u0027key_name\u0027] \u003d validation_resources[\u0027keypair\u0027][\u0027name\u0027]"},{"line_number":76,"context_line":"            except KeyError:"},{"line_number":77,"context_line":"                LOG.debug(\"No key provided.\")"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    body \u003d clients.servers_client.create_server(name, image_id, flavor,"},{"line_number":80,"context_line":"                                                **kwargs)"}],"source_content_type":"text/x-python","patch_set":38,"id":"7a016987_8c86a711","line":77,"updated":"2015-05-17 18:26:01.000000000","message":"I think it would be nice to add another check here:\n\n if CONF.validation.connect_method \u003d\u003d \u0027floating\u0027\n   if wait_until not in kwargs:\n      kwargs[\u0027wait_until\u0027] \u003d \u0027ACTIVE\u0027\n\nSo that we wait for the server to be ACTVE before attaching a floating IP to it.","commit_id":"9f39aacd6a8aa6b759b0f975a758b3b201d2d0d3"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"5cdee4a2f665e9a3d5cde7d3dd9ebb605d5314f1","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                floating_ip\u003dvalidation_resources[\u0027floating_ip\u0027],"},{"line_number":110,"context_line":"                server_id\u003dservers[0])"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    return [body, servers]"}],"source_content_type":"text/x-python","patch_set":38,"id":"7a016987_2c1c5311","line":112,"updated":"2015-05-17 18:26:01.000000000","message":"Looking at the code in base.py, you probably wanted to return a tuple here rather than a list here.","commit_id":"9f39aacd6a8aa6b759b0f975a758b3b201d2d0d3"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"22ed4d0e703a674432be91987e9b299acb186b18","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    This method is a common wrapper returning a test server that can be"},{"line_number":33,"context_line":"    pingable or sshable."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    :param clients: Client manager and its clients."},{"line_number":36,"context_line":"    :param validatble: Whether the server will be pingable or sshable."},{"line_number":37,"context_line":"    :param validation_resources: Resources created for the connection to the"},{"line_number":38,"context_line":"    server. Include a keypair, a security group and an IP."}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_58dfbfd5","line":35,"updated":"2015-06-11 08:42:16.000000000","message":"This is confusing for me. Can we just make this something-\n-param client_manager: Client manager which provides OpenStack tempest clients.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":10016,"name":"Joseph Lanoux","email":"joseph.lanoux@hpe.com","username":"lanoux"},"change_message_id":"3de35dd9c166449bef20ae903052af275a924e10","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    This method is a common wrapper returning a test server that can be"},{"line_number":33,"context_line":"    pingable or sshable."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    :param clients: Client manager and its clients."},{"line_number":36,"context_line":"    :param validatble: Whether the server will be pingable or sshable."},{"line_number":37,"context_line":"    :param validation_resources: Resources created for the connection to the"},{"line_number":38,"context_line":"    server. Include a keypair, a security group and an IP."}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_612e46a6","line":35,"in_reply_to":"3afb71cf_58dfbfd5","updated":"2015-06-11 09:58:21.000000000","message":"Ok.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"22ed4d0e703a674432be91987e9b299acb186b18","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    pingable or sshable."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    :param clients: Client manager and its clients."},{"line_number":36,"context_line":"    :param validatble: Whether the server will be pingable or sshable."},{"line_number":37,"context_line":"    :param validation_resources: Resources created for the connection to the"},{"line_number":38,"context_line":"    server. Include a keypair, a security group and an IP."},{"line_number":39,"context_line":"    :returns a server"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_b8d473eb","line":36,"updated":"2015-06-11 08:42:16.000000000","message":"Hope this you will remove after your TODO mentioned below.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"cc984b187ed1941d5d390db94adf172ac1565ba3","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    pingable or sshable."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    :param clients: Client manager and its clients."},{"line_number":36,"context_line":"    :param validatble: Whether the server will be pingable or sshable."},{"line_number":37,"context_line":"    :param validation_resources: Resources created for the connection to the"},{"line_number":38,"context_line":"    server. Include a keypair, a security group and an IP."},{"line_number":39,"context_line":"    :returns a server"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_0ded79ef","line":36,"in_reply_to":"3afb71cf_3c175046","updated":"2015-06-11 14:46:28.000000000","message":"There is a spelling error in validatable (validatble)","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"78de8a6cea65bc613b5f66339e84126800063b6a","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    pingable or sshable."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    :param clients: Client manager and its clients."},{"line_number":36,"context_line":"    :param validatble: Whether the server will be pingable or sshable."},{"line_number":37,"context_line":"    :param validation_resources: Resources created for the connection to the"},{"line_number":38,"context_line":"    server. Include a keypair, a security group and an IP."},{"line_number":39,"context_line":"    :returns a server"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_3c175046","line":36,"in_reply_to":"3afb71cf_812382bc","updated":"2015-06-11 10:56:20.000000000","message":"ahh, i got it. Thanks. I misunderstood that.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":10016,"name":"Joseph Lanoux","email":"joseph.lanoux@hpe.com","username":"lanoux"},"change_message_id":"3de35dd9c166449bef20ae903052af275a924e10","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    pingable or sshable."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    :param clients: Client manager and its clients."},{"line_number":36,"context_line":"    :param validatble: Whether the server will be pingable or sshable."},{"line_number":37,"context_line":"    :param validation_resources: Resources created for the connection to the"},{"line_number":38,"context_line":"    server. Include a keypair, a security group and an IP."},{"line_number":39,"context_line":"    :returns a server"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_812382bc","line":36,"in_reply_to":"3afb71cf_b8d473eb","updated":"2015-06-11 09:58:21.000000000","message":"Validatable will not be removed as this is the boolean for ssh validation for each tests. The TODO below is for wait_for.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"22ed4d0e703a674432be91987e9b299acb186b18","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    :param validatble: Whether the server will be pingable or sshable."},{"line_number":37,"context_line":"    :param validation_resources: Resources created for the connection to the"},{"line_number":38,"context_line":"    server. Include a keypair, a security group and an IP."},{"line_number":39,"context_line":"    :returns a server"},{"line_number":40,"context_line":"    \"\"\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    # TODO(jlanoux) add support of wait_until PINGABLE/SSHABLE"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_78fb9b47","line":39,"updated":"2015-06-11 08:42:16.000000000","message":"It returns tuple","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":10016,"name":"Joseph Lanoux","email":"joseph.lanoux@hpe.com","username":"lanoux"},"change_message_id":"3de35dd9c166449bef20ae903052af275a924e10","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    :param validatble: Whether the server will be pingable or sshable."},{"line_number":37,"context_line":"    :param validation_resources: Resources created for the connection to the"},{"line_number":38,"context_line":"    server. Include a keypair, a security group and an IP."},{"line_number":39,"context_line":"    :returns a server"},{"line_number":40,"context_line":"    \"\"\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    # TODO(jlanoux) add support of wait_until PINGABLE/SSHABLE"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_e110f65a","line":39,"in_reply_to":"3afb71cf_78fb9b47","updated":"2015-06-11 09:58:21.000000000","message":"Ok.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":7020,"name":"Joe Hakim Rahme","email":"jhakimra@redhat.com","username":"rahmu"},"change_message_id":"2675798aa5e48b9277c5239a7a61b8c0456bfbc4","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    flavor \u003d kwargs.get(\u0027flavor\u0027, CONF.compute.flavor_ref)"},{"line_number":50,"context_line":"    image_id \u003d kwargs.get(\u0027image_id\u0027, CONF.compute.image_ref)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    kwargs \u003d fixed_network.set_networks_kwarg("},{"line_number":53,"context_line":"        tenant_network, kwargs) or {}"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    if CONF.validation.run_validation and validatable:"},{"line_number":56,"context_line":"        # As a first implementation, multiple pingable or sshable servers will"}],"source_content_type":"text/x-python","patch_set":39,"id":"5afe65bd_2c46d723","line":53,"range":{"start_line":52,"start_character":0,"end_line":53,"end_character":37},"updated":"2015-06-02 10:22:37.000000000","message":"Not sure if `or {}` is needed here, since it looks like set_networks_kwarg can only return a dict. Or maybe am I missing something?","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":1192,"name":"David Kranz","email":"dkranz@redhat.com","username":"david-kranz"},"change_message_id":"83d15c744952d3bcca4bd0ab324d35f3ba1c727d","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    image_id \u003d kwargs.get(\u0027image_id\u0027, CONF.compute.image_ref)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    kwargs \u003d fixed_network.set_networks_kwarg("},{"line_number":53,"context_line":"        tenant_network, kwargs) or {}"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    if CONF.validation.run_validation and validatable:"},{"line_number":56,"context_line":"        # As a first implementation, multiple pingable or sshable servers will"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_276cd49b","line":53,"in_reply_to":"5afe65bd_2c46d723","updated":"2015-06-04 19:22:43.000000000","message":"This was just copied from the original.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":7428,"name":"Yaroslav Lobankov","email":"lobankovyv@gmail.com","username":"ylobankov"},"change_message_id":"34dd704ac1b42aa8040b32581e64ac56d7d353aa","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        # As a first implementation, multiple pingable or sshable servers will"},{"line_number":57,"context_line":"        # not be supported"},{"line_number":58,"context_line":"        if \u0027min_count\u0027 in kwargs or \u0027max_count\u0027 in kwargs:"},{"line_number":59,"context_line":"            msg \u003d (\"Multiple pingable or sshable servers not supported at\\"},{"line_number":60,"context_line":"                   this stage.\")"},{"line_number":61,"context_line":"            raise ValueError(msg)"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"7a016987_86219b4f","line":59,"updated":"2015-05-29 16:37:13.000000000","message":"Do you really need backslash here (after at) or it is here accidentally?","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":10016,"name":"Joseph Lanoux","email":"joseph.lanoux@hpe.com","username":"lanoux"},"change_message_id":"5620f3f51219f7534e22dc9cfb41585da72cf50a","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        # As a first implementation, multiple pingable or sshable servers will"},{"line_number":57,"context_line":"        # not be supported"},{"line_number":58,"context_line":"        if \u0027min_count\u0027 in kwargs or \u0027max_count\u0027 in kwargs:"},{"line_number":59,"context_line":"            msg \u003d (\"Multiple pingable or sshable servers not supported at\\"},{"line_number":60,"context_line":"                   this stage.\")"},{"line_number":61,"context_line":"            raise ValueError(msg)"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"7a016987_299970c4","line":59,"in_reply_to":"7a016987_86219b4f","updated":"2015-06-01 13:02:52.000000000","message":"Yes. But it seems that quotes at every line is the best practice. I\u0027ll update that.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":7020,"name":"Joe Hakim Rahme","email":"jhakimra@redhat.com","username":"rahmu"},"change_message_id":"2675798aa5e48b9277c5239a7a61b8c0456bfbc4","unresolved":false,"context_lines":[{"line_number":65,"context_line":"                {\u0027name\u0027: validation_resources[\u0027security_group\u0027][\u0027name\u0027]})"},{"line_number":66,"context_line":"        else:"},{"line_number":67,"context_line":"            try:"},{"line_number":68,"context_line":"                kwargs[\u0027security_groups\u0027] \u003d \\"},{"line_number":69,"context_line":"                    [{\u0027name\u0027: validation_resources[\u0027security_group\u0027][\u0027name\u0027]}]"},{"line_number":70,"context_line":"            except KeyError:"},{"line_number":71,"context_line":"                LOG.debug(\"No security group provided.\")"}],"source_content_type":"text/x-python","patch_set":39,"id":"5afe65bd_ec6f9f90","line":68,"updated":"2015-06-02 10:22:37.000000000","message":"Small style nitpick: wouldn\u0027t it be nicer to avoid the backslash by opening the bracket and returning to the line? Something like\n\n```\nkwargs[\u0027security_groups\u0027] \u003d [\n    {\u0027name\u0027: ...}]\n```","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":10016,"name":"Joseph Lanoux","email":"joseph.lanoux@hpe.com","username":"lanoux"},"change_message_id":"92dc298968d5291e82535a85ccc70a03ed9ddf19","unresolved":false,"context_lines":[{"line_number":65,"context_line":"                {\u0027name\u0027: validation_resources[\u0027security_group\u0027][\u0027name\u0027]})"},{"line_number":66,"context_line":"        else:"},{"line_number":67,"context_line":"            try:"},{"line_number":68,"context_line":"                kwargs[\u0027security_groups\u0027] \u003d \\"},{"line_number":69,"context_line":"                    [{\u0027name\u0027: validation_resources[\u0027security_group\u0027][\u0027name\u0027]}]"},{"line_number":70,"context_line":"            except KeyError:"},{"line_number":71,"context_line":"                LOG.debug(\"No security group provided.\")"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_bf7c3921","line":68,"in_reply_to":"5afe65bd_ec6f9f90","updated":"2015-06-05 08:04:59.000000000","message":"Yes, why not?","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"22ed4d0e703a674432be91987e9b299acb186b18","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            try:"},{"line_number":68,"context_line":"                kwargs[\u0027security_groups\u0027] \u003d \\"},{"line_number":69,"context_line":"                    [{\u0027name\u0027: validation_resources[\u0027security_group\u0027][\u0027name\u0027]}]"},{"line_number":70,"context_line":"            except KeyError:"},{"line_number":71,"context_line":"                LOG.debug(\"No security group provided.\")"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        if \u0027key_name\u0027 not in kwargs:"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_78f03bc9","line":70,"updated":"2015-06-11 08:42:16.000000000","message":"Do not we need to create one if it is not available in provided resources as per spec?","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":10016,"name":"Joseph Lanoux","email":"joseph.lanoux@hpe.com","username":"lanoux"},"change_message_id":"3de35dd9c166449bef20ae903052af275a924e10","unresolved":false,"context_lines":[{"line_number":67,"context_line":"            try:"},{"line_number":68,"context_line":"                kwargs[\u0027security_groups\u0027] \u003d \\"},{"line_number":69,"context_line":"                    [{\u0027name\u0027: validation_resources[\u0027security_group\u0027][\u0027name\u0027]}]"},{"line_number":70,"context_line":"            except KeyError:"},{"line_number":71,"context_line":"                LOG.debug(\"No security group provided.\")"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        if \u0027key_name\u0027 not in kwargs:"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_0c2c6997","line":70,"in_reply_to":"3afb71cf_78f03bc9","updated":"2015-06-11 09:58:21.000000000","message":"It\u0027s done by validation_resources.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"22ed4d0e703a674432be91987e9b299acb186b18","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        if \u0027key_name\u0027 not in kwargs:"},{"line_number":74,"context_line":"            try:"},{"line_number":75,"context_line":"                kwargs[\u0027key_name\u0027] \u003d validation_resources[\u0027keypair\u0027][\u0027name\u0027]"},{"line_number":76,"context_line":"            except KeyError:"},{"line_number":77,"context_line":"                LOG.debug(\"No key provided.\")"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        if CONF.validation.connect_method \u003d\u003d \u0027floating\u0027:"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_9893f75a","line":76,"updated":"2015-06-11 08:42:16.000000000","message":"ditto","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":10016,"name":"Joseph Lanoux","email":"joseph.lanoux@hpe.com","username":"lanoux"},"change_message_id":"3de35dd9c166449bef20ae903052af275a924e10","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        if \u0027key_name\u0027 not in kwargs:"},{"line_number":74,"context_line":"            try:"},{"line_number":75,"context_line":"                kwargs[\u0027key_name\u0027] \u003d validation_resources[\u0027keypair\u0027][\u0027name\u0027]"},{"line_number":76,"context_line":"            except KeyError:"},{"line_number":77,"context_line":"                LOG.debug(\"No key provided.\")"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        if CONF.validation.connect_method \u003d\u003d \u0027floating\u0027:"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_cc21b19f","line":76,"in_reply_to":"3afb71cf_9893f75a","updated":"2015-06-11 09:58:21.000000000","message":"It\u0027s done by validation_resources.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":1192,"name":"David Kranz","email":"dkranz@redhat.com","username":"david-kranz"},"change_message_id":"83d15c744952d3bcca4bd0ab324d35f3ba1c727d","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            except KeyError:"},{"line_number":77,"context_line":"                LOG.debug(\"No key provided.\")"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        if CONF.validation.connect_method \u003d\u003d \u0027floating\u0027:"},{"line_number":80,"context_line":"            if \u0027wait_until\u0027 not in kwargs:"},{"line_number":81,"context_line":"                kwargs[\u0027wait_until\u0027] \u003d \u0027ACTIVE\u0027"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_c7d74022","line":79,"updated":"2015-06-04 19:22:43.000000000","message":"Can you please explain this? I\u0027m not sure why we force a wait for ACTIVE for floating but not fixed. Also, what if the caller specified a wait_until but it is not ACTIVE?","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":10016,"name":"Joseph Lanoux","email":"joseph.lanoux@hpe.com","username":"lanoux"},"change_message_id":"92dc298968d5291e82535a85ccc70a03ed9ddf19","unresolved":false,"context_lines":[{"line_number":76,"context_line":"            except KeyError:"},{"line_number":77,"context_line":"                LOG.debug(\"No key provided.\")"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        if CONF.validation.connect_method \u003d\u003d \u0027floating\u0027:"},{"line_number":80,"context_line":"            if \u0027wait_until\u0027 not in kwargs:"},{"line_number":81,"context_line":"                kwargs[\u0027wait_until\u0027] \u003d \u0027ACTIVE\u0027"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_ffb5117e","line":79,"in_reply_to":"3afb71cf_c7d74022","updated":"2015-06-05 08:04:59.000000000","message":"What happens in slower cases is that the floating IP will try to be attached before the server is connected to the network and it will therefore fail. This doesn\u0027t occur for fixed IP.\n\nIIRC, the caller is not considering transient states before a server becomes ACTIVE. And I don\u0027t think a test would wait for something else if coming through create_server.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"22ed4d0e703a674432be91987e9b299acb186b18","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                kwargs[\u0027wait_until\u0027] \u003d \u0027ACTIVE\u0027"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    body \u003d clients.servers_client.create_server(name, image_id, flavor,"},{"line_number":84,"context_line":"                                                **kwargs)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    # handle the case of multiple servers"},{"line_number":87,"context_line":"    servers \u003d [body]"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_5bc21954","line":84,"updated":"2015-06-11 08:42:16.000000000","message":"Is we are making it independent function then, i think it should take care of resource cleanup also.\n\nThis file can have clear_servers() and add that in addCleanup.\n\nI am ok if you plan to do that in follow up patch?","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":10016,"name":"Joseph Lanoux","email":"joseph.lanoux@hpe.com","username":"lanoux"},"change_message_id":"3de35dd9c166449bef20ae903052af275a924e10","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                kwargs[\u0027wait_until\u0027] \u003d \u0027ACTIVE\u0027"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    body \u003d clients.servers_client.create_server(name, image_id, flavor,"},{"line_number":84,"context_line":"                                                **kwargs)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    # handle the case of multiple servers"},{"line_number":87,"context_line":"    servers \u003d [body]"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_ac443549","line":84,"in_reply_to":"3afb71cf_5bc21954","updated":"2015-06-11 09:58:21.000000000","message":"The tooling is not available in this method. The returned tuple is used to include the created servers in the cleanup.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"78de8a6cea65bc613b5f66339e84126800063b6a","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                kwargs[\u0027wait_until\u0027] \u003d \u0027ACTIVE\u0027"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"    body \u003d clients.servers_client.create_server(name, image_id, flavor,"},{"line_number":84,"context_line":"                                                **kwargs)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    # handle the case of multiple servers"},{"line_number":87,"context_line":"    servers \u003d [body]"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_1c0814d9","line":84,"in_reply_to":"3afb71cf_ac443549","updated":"2015-06-11 10:56:20.000000000","message":"humm, May be we can think of this and implement in further patches. as of now it is fine.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"22ed4d0e703a674432be91987e9b299acb186b18","unresolved":false,"context_lines":[{"line_number":112,"context_line":"            clients.floating_ips_client.associate_floating_ip_to_server("},{"line_number":113,"context_line":"                floating_ip\u003dvalidation_resources[\u0027floating_ip\u0027],"},{"line_number":114,"context_line":"                server_id\u003dservers[0])"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    return (body, servers)"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_78acbb8b","line":115,"updated":"2015-06-11 08:42:16.000000000","message":"So it is not actually doing SSH I think that part is depended on RemoteClient work. If so please add TODO.\n\nAfter that this should return servers and remote client as per spec","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":10016,"name":"Joseph Lanoux","email":"joseph.lanoux@hpe.com","username":"lanoux"},"change_message_id":"3de35dd9c166449bef20ae903052af275a924e10","unresolved":false,"context_lines":[{"line_number":112,"context_line":"            clients.floating_ips_client.associate_floating_ip_to_server("},{"line_number":113,"context_line":"                floating_ip\u003dvalidation_resources[\u0027floating_ip\u0027],"},{"line_number":114,"context_line":"                server_id\u003dservers[0])"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    return (body, servers)"}],"source_content_type":"text/x-python","patch_set":39,"id":"3afb71cf_ec7a2d76","line":115,"in_reply_to":"3afb71cf_78acbb8b","updated":"2015-06-11 09:58:21.000000000","message":"You\u0027re right. The create_server and remote_client will be merged in a following phase. This change was too large as a first step. For now, the ssh validation is done in the current remote_client.","commit_id":"0d29d050e0d25bd14f6f207c8b4ccfcf59f94b10"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"b2d09912cead60a7d4cad01dcea0759d4773e988","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    :param validatable: Whether the server will be pingable or sshable."},{"line_number":37,"context_line":"    :param validation_resources: Resources created for the connection to the"},{"line_number":38,"context_line":"    server. Include a keypair, a security group and an IP."},{"line_number":39,"context_line":"    :returns a tuple containing a server\u0027s parameters"},{"line_number":40,"context_line":"    \"\"\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    # TODO(jlanoux) add support of wait_until PINGABLE/SSHABLE"}],"source_content_type":"text/x-python","patch_set":40,"id":"3afb71cf_d8295d8b","line":39,"updated":"2015-06-11 14:55:28.000000000","message":"No, it returns a tuple (body, servers), where body is the raw response from the create message, and servers is a list of all servers created.","commit_id":"75add7a43d0451c8bb5727b045dc38f81609dec1"},{"author":{"_account_id":16433,"name":"izadorozhna","email":"izadorozhna@mirantis.com","username":"izadorozhna"},"change_message_id":"b23245dec13a3f8aef167bf7e1a85be5e980591c","unresolved":false,"context_lines":[{"line_number":110,"context_line":"    if CONF.validation.run_validation and validatable:"},{"line_number":111,"context_line":"        if CONF.validation.connect_method \u003d\u003d \u0027floating\u0027:"},{"line_number":112,"context_line":"            clients.floating_ips_client.associate_floating_ip_to_server("},{"line_number":113,"context_line":"                floating_ip\u003dvalidation_resources[\u0027floating_ip\u0027],"},{"line_number":114,"context_line":"                server_id\u003dservers[0])"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    return (body, servers)"}],"source_content_type":"text/x-python","patch_set":42,"id":"1af86dd1_d44b532a","line":113,"updated":"2015-06-12 13:02:12.000000000","message":"It seems that there is a typo (missed [\u0027ip\u0027]):\nfloating_ip\u003dvalidation_resources[\u0027floating_ip\u0027][\u0027ip\u0027]","commit_id":"e6824a02e316be371ca9a075dbf968dc5af6d848"},{"author":{"_account_id":16433,"name":"izadorozhna","email":"izadorozhna@mirantis.com","username":"izadorozhna"},"change_message_id":"ea845e2a55e83f6733b23cc64a14d15390ec0146","unresolved":false,"context_lines":[{"line_number":111,"context_line":"        if CONF.validation.connect_method \u003d\u003d \u0027floating\u0027:"},{"line_number":112,"context_line":"            clients.floating_ips_client.associate_floating_ip_to_server("},{"line_number":113,"context_line":"                floating_ip\u003dvalidation_resources[\u0027floating_ip\u0027],"},{"line_number":114,"context_line":"                server_id\u003dservers[0])"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"    return (body, servers)"}],"source_content_type":"text/x-python","patch_set":42,"id":"1af86dd1_a11adb15","line":114,"updated":"2015-06-12 12:42:22.000000000","message":"It seems like you missed [\u0027id\u0027]:\nserver_id\u003dservers[0][\u0027id\u0027]","commit_id":"e6824a02e316be371ca9a075dbf968dc5af6d848"},{"author":{"_account_id":9377,"name":"Sergey Slipushenko","email":"slipushenko@gmail.com","username":"sslypushenko"},"change_message_id":"d2a6aad46883ccaa79709d0ec1f30cbb84317b8f","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                                clients.servers_client.delete_server("},{"line_number":106,"context_line":"                                    server[\u0027id\u0027])"},{"line_number":107,"context_line":"                            except Exception:"},{"line_number":108,"context_line":"                                pass"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    if CONF.validation.run_validation and validatable:"},{"line_number":111,"context_line":"        if CONF.validation.connect_method \u003d\u003d \u0027floating\u0027:"}],"source_content_type":"text/x-python","patch_set":44,"id":"1af86dd1_6a3153a2","line":108,"updated":"2015-06-12 15:57:21.000000000","message":"excutils.save_and_reraise_exception can be helpful for this try block too","commit_id":"011c1a5642d3fde04074bced78126325640a0ee2"},{"author":{"_account_id":9377,"name":"Sergey Slipushenko","email":"slipushenko@gmail.com","username":"sslypushenko"},"change_message_id":"d2a6aad46883ccaa79709d0ec1f30cbb84317b8f","unresolved":false,"context_lines":[{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    if CONF.validation.run_validation and validatable:"},{"line_number":111,"context_line":"        if CONF.validation.connect_method \u003d\u003d \u0027floating\u0027:"},{"line_number":112,"context_line":"            clients.floating_ips_client.associate_floating_ip_to_server("},{"line_number":113,"context_line":"                floating_ip\u003dvalidation_resources[\u0027floating_ip\u0027][\u0027ip\u0027],"},{"line_number":114,"context_line":"                server_id\u003dservers[0][\u0027id\u0027])"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-python","patch_set":44,"id":"1af86dd1_aa52abdb","line":112,"updated":"2015-06-12 15:57:21.000000000","message":"izadorozhna and I hit a small issue in this place. If floating association fails, it will lead to cleanup failing because, in this place server is already existed, but isn\u0027t added to cls.servers variable, so it will stay alive after test failure. Such failure leads to staying a lot of garbage in the test cloud.\n\nSo maybe, it will be reasonable move lines 110-114 into try ... except block at line 99.","commit_id":"011c1a5642d3fde04074bced78126325640a0ee2"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"65781b6636a41b2a32fd4dba65bf834ae63cb121","unresolved":false,"context_lines":[{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    if CONF.validation.run_validation and validatable:"},{"line_number":111,"context_line":"        if CONF.validation.connect_method \u003d\u003d \u0027floating\u0027:"},{"line_number":112,"context_line":"            clients.floating_ips_client.associate_floating_ip_to_server("},{"line_number":113,"context_line":"                floating_ip\u003dvalidation_resources[\u0027floating_ip\u0027][\u0027ip\u0027],"},{"line_number":114,"context_line":"                server_id\u003dservers[0][\u0027id\u0027])"},{"line_number":115,"context_line":""}],"source_content_type":"text/x-python","patch_set":44,"id":"fa32b979_2146ef23","line":112,"in_reply_to":"1af86dd1_aa52abdb","updated":"2015-06-15 03:18:52.000000000","message":"Yes, that issue. I prefer to have cleanup part also in this method. One way is to have this function in a class inherited from TestCase and then use addCleanup.\n\nBut as of now we can have any workaround to merge this soon. and proper solution can be added in follow up patch.","commit_id":"011c1a5642d3fde04074bced78126325640a0ee2"},{"author":{"_account_id":7428,"name":"Yaroslav Lobankov","email":"lobankovyv@gmail.com","username":"ylobankov"},"change_message_id":"021b90db77d51ebdcf81d143e4d12ce2f4cd6490","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        # As a first implementation, multiple pingable or sshable servers will"},{"line_number":57,"context_line":"        # not be supported"},{"line_number":58,"context_line":"        if \u0027min_count\u0027 in kwargs or \u0027max_count\u0027 in kwargs:"},{"line_number":59,"context_line":"            msg \u003d (\"Multiple pingable or sshable servers not supported at\""},{"line_number":60,"context_line":"                   \"this stage.\")"},{"line_number":61,"context_line":"            raise ValueError(msg)"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"fa32b979_64be1bd6","line":59,"updated":"2015-06-15 22:32:01.000000000","message":"After \"at\" space is needed.","commit_id":"a8500265f43765b9478ee23c64e14ef4f0bbab12"},{"author":{"_account_id":10016,"name":"Joseph Lanoux","email":"joseph.lanoux@hpe.com","username":"lanoux"},"change_message_id":"f58a80c02d624bf8f1b6c91b82ed53c19303e025","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        # As a first implementation, multiple pingable or sshable servers will"},{"line_number":57,"context_line":"        # not be supported"},{"line_number":58,"context_line":"        if \u0027min_count\u0027 in kwargs or \u0027max_count\u0027 in kwargs:"},{"line_number":59,"context_line":"            msg \u003d (\"Multiple pingable or sshable servers not supported at\""},{"line_number":60,"context_line":"                   \"this stage.\")"},{"line_number":61,"context_line":"            raise ValueError(msg)"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"fa32b979_6fb30bea","line":59,"in_reply_to":"fa32b979_64be1bd6","updated":"2015-06-17 10:05:06.000000000","message":"Done","commit_id":"a8500265f43765b9478ee23c64e14ef4f0bbab12"},{"author":{"_account_id":7428,"name":"Yaroslav Lobankov","email":"lobankovyv@gmail.com","username":"ylobankov"},"change_message_id":"021b90db77d51ebdcf81d143e4d12ce2f4cd6490","unresolved":false,"context_lines":[{"line_number":87,"context_line":"    servers \u003d [body]"},{"line_number":88,"context_line":"    if \u0027min_count\u0027 in kwargs or \u0027max_count\u0027 in kwargs:"},{"line_number":89,"context_line":"        # Get servers created which name match with name param."},{"line_number":90,"context_line":"        body_servers \u003d clients.servers_client.list_servers()"},{"line_number":91,"context_line":"        servers \u003d \\"},{"line_number":92,"context_line":"            [s for s in body_servers[\u0027servers\u0027] if s[\u0027name\u0027].startswith(name)]"},{"line_number":93,"context_line":""}],"source_content_type":"text/x-python","patch_set":45,"id":"fa32b979_8430ff02","line":90,"updated":"2015-06-15 22:32:01.000000000","message":"-1 line\n\n    servers_list \u003d clients.servers_client.list_servers()[\u0027servers\u0027]\n    servers \u003d [s for s in servers_list if s[\u0027name\u0027].startswith(name)]\n\nBut it is up to you.","commit_id":"a8500265f43765b9478ee23c64e14ef4f0bbab12"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"95ff6c915f0e273b0f5c19ba264f828a208f46f9","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                        assoc(floating_ip\u003dvalidation_resources["},{"line_number":109,"context_line":"                              \u0027floating_ip\u0027][\u0027ip\u0027],"},{"line_number":110,"context_line":"                              server_id\u003dservers[0][\u0027id\u0027])"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"            except Exception:"},{"line_number":113,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":114,"context_line":"                    if (\u0027preserve_server_on_error\u0027 not in kwargs"}],"source_content_type":"text/x-python","patch_set":45,"id":"fa32b979_28ab011f","line":111,"updated":"2015-06-16 02:22:17.000000000","message":"This is not good to associate FIP here (in wait_until block and for each servers).\n\nI will prefer to have that part as in previous patch later we can fix this when using cleanup of servers in this method only. Please add TODO there.","commit_id":"a8500265f43765b9478ee23c64e14ef4f0bbab12"},{"author":{"_account_id":9377,"name":"Sergey Slipushenko","email":"slipushenko@gmail.com","username":"sslypushenko"},"change_message_id":"a95e419ac48a7ba736eb578089b24cf13f36a200","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                        assoc(floating_ip\u003dvalidation_resources["},{"line_number":109,"context_line":"                              \u0027floating_ip\u0027][\u0027ip\u0027],"},{"line_number":110,"context_line":"                              server_id\u003dservers[0][\u0027id\u0027])"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"            except Exception:"},{"line_number":113,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":114,"context_line":"                    if (\u0027preserve_server_on_error\u0027 not in kwargs"}],"source_content_type":"text/x-python","patch_set":45,"id":"fa32b979_ddc7b110","line":111,"in_reply_to":"fa32b979_28ab011f","updated":"2015-06-16 07:10:37.000000000","message":"I strongly against from leaving \"assoc\" with any cleanup as it was before. FIP association failure leaves a lot of resources in a test cloud which is hard to manually cleanup (ports, routers, nets, subnets, VM).\nSo, I think it will be better to leave it as it is and put TODO in this code.","commit_id":"a8500265f43765b9478ee23c64e14ef4f0bbab12"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"6c105292ee5b61a6664d9b7ebe87267bbc2f6075","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                        assoc(floating_ip\u003dvalidation_resources["},{"line_number":109,"context_line":"                              \u0027floating_ip\u0027][\u0027ip\u0027],"},{"line_number":110,"context_line":"                              server_id\u003dservers[0][\u0027id\u0027])"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"            except Exception:"},{"line_number":113,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":114,"context_line":"                    if (\u0027preserve_server_on_error\u0027 not in kwargs"}],"source_content_type":"text/x-python","patch_set":45,"id":"fa32b979_c61fa46a","line":111,"in_reply_to":"fa32b979_ddc7b110","updated":"2015-06-17 08:40:29.000000000","message":"humm, right, As wait_until is going to be extended to SSHABLE and PIGABLE status i think it is fine to associate FIP here.","commit_id":"a8500265f43765b9478ee23c64e14ef4f0bbab12"},{"author":{"_account_id":7428,"name":"Yaroslav Lobankov","email":"lobankovyv@gmail.com","username":"ylobankov"},"change_message_id":"021b90db77d51ebdcf81d143e4d12ce2f4cd6490","unresolved":false,"context_lines":[{"line_number":111,"context_line":""},{"line_number":112,"context_line":"            except Exception:"},{"line_number":113,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":114,"context_line":"                    if (\u0027preserve_server_on_error\u0027 not in kwargs"},{"line_number":115,"context_line":"                        or kwargs[\u0027preserve_server_on_error\u0027] is False):"},{"line_number":116,"context_line":"                        for server in servers:"},{"line_number":117,"context_line":"                            try:"}],"source_content_type":"text/x-python","patch_set":45,"id":"fa32b979_57be2bd5","line":114,"updated":"2015-06-15 22:32:01.000000000","message":"if (\u0027preserve_server_on_error\u0027 not in kwargs\n        or kwargs[\u0027preserve_server_on_error\u0027] is False):\n\nBut it is up to you.","commit_id":"a8500265f43765b9478ee23c64e14ef4f0bbab12"},{"author":{"_account_id":7428,"name":"Yaroslav Lobankov","email":"lobankovyv@gmail.com","username":"ylobankov"},"change_message_id":"021b90db77d51ebdcf81d143e4d12ce2f4cd6490","unresolved":false,"context_lines":[{"line_number":113,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":114,"context_line":"                    if (\u0027preserve_server_on_error\u0027 not in kwargs"},{"line_number":115,"context_line":"                        or kwargs[\u0027preserve_server_on_error\u0027] is False):"},{"line_number":116,"context_line":"                        for server in servers:"},{"line_number":117,"context_line":"                            try:"},{"line_number":118,"context_line":"                                clients.servers_client.delete_server("},{"line_number":119,"context_line":"                                    server[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":45,"id":"fa32b979_b77d371e","line":116,"updated":"2015-06-15 22:32:01.000000000","message":"-1 line\n\nfor s in servers:\n    try:\n        clients.servers_client.delete_server(s[\u0027id\u0027])\n    except Exception:\n        ...\n\nBut it is up to you.","commit_id":"a8500265f43765b9478ee23c64e14ef4f0bbab12"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"95ff6c915f0e273b0f5c19ba264f828a208f46f9","unresolved":false,"context_lines":[{"line_number":118,"context_line":"                                clients.servers_client.delete_server("},{"line_number":119,"context_line":"                                    server[\u0027id\u0027])"},{"line_number":120,"context_line":"                            except Exception:"},{"line_number":121,"context_line":"                                with excutils.save_and_reraise_exception():"},{"line_number":122,"context_line":"                                    pass"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    return (body, servers)"}],"source_content_type":"text/x-python","patch_set":45,"id":"fa32b979_c82375a6","line":121,"updated":"2015-06-16 02:22:17.000000000","message":"I did not get the much benefit of using excutils.save_and_reraise_exception() here.\nNow, it will raise the exception occurred in delete operation and same will be reraised on outer \u0027with\u0027 block. Original exception will not get raised just being logged.\n\nIMO, we should leave this part  as it was in original code and later we can improve that(specially log the delete operation exception). Because it would not be good to fix everything in this patch :).","commit_id":"a8500265f43765b9478ee23c64e14ef4f0bbab12"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"010758bde881cce713c8d56bbb4289bfdaf29e37","unresolved":false,"context_lines":[{"line_number":118,"context_line":"                                clients.servers_client.delete_server("},{"line_number":119,"context_line":"                                    server[\u0027id\u0027])"},{"line_number":120,"context_line":"                            except Exception:"},{"line_number":121,"context_line":"                                with excutils.save_and_reraise_exception():"},{"line_number":122,"context_line":"                                    pass"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    return (body, servers)"}],"source_content_type":"text/x-python","patch_set":45,"id":"fa32b979_e9e5db37","line":121,"in_reply_to":"fa32b979_a6d5c01c","updated":"2015-06-17 08:50:33.000000000","message":"May be we can log exception explicitly here.\n- LOG.exception(\u0027Deleting server %s failed\u0027 % server[\u0027id\u0027])","commit_id":"a8500265f43765b9478ee23c64e14ef4f0bbab12"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"6c105292ee5b61a6664d9b7ebe87267bbc2f6075","unresolved":false,"context_lines":[{"line_number":118,"context_line":"                                clients.servers_client.delete_server("},{"line_number":119,"context_line":"                                    server[\u0027id\u0027])"},{"line_number":120,"context_line":"                            except Exception:"},{"line_number":121,"context_line":"                                with excutils.save_and_reraise_exception():"},{"line_number":122,"context_line":"                                    pass"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    return (body, servers)"}],"source_content_type":"text/x-python","patch_set":45,"id":"fa32b979_a6d5c01c","line":121,"in_reply_to":"fa32b979_bd850d08","updated":"2015-06-17 08:40:29.000000000","message":"+1 for at least logging.\n\nBut I think by passing rerise\u003dFalse, excutils will not log the error. Or m missing anything?\n\n----------------------------------------------------------------------------\n    def __exit__(self, exc_type, exc_val, exc_tb):\n        if exc_type is not None:\n            if self.reraise:\n                self.logger.error(_LE(\u0027Original exception being dropped: %s\u0027),\n                                  traceback.format_exception(self.type_,\n                                                             self.value,\n                                                             self.tb))\n            return False\n        if self.reraise:\n            six.reraise(self.type_, self.value, self.tb)\n----------------------------------------------------------------------------","commit_id":"a8500265f43765b9478ee23c64e14ef4f0bbab12"},{"author":{"_account_id":9377,"name":"Sergey Slipushenko","email":"slipushenko@gmail.com","username":"sslypushenko"},"change_message_id":"a95e419ac48a7ba736eb578089b24cf13f36a200","unresolved":false,"context_lines":[{"line_number":118,"context_line":"                                clients.servers_client.delete_server("},{"line_number":119,"context_line":"                                    server[\u0027id\u0027])"},{"line_number":120,"context_line":"                            except Exception:"},{"line_number":121,"context_line":"                                with excutils.save_and_reraise_exception():"},{"line_number":122,"context_line":"                                    pass"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    return (body, servers)"}],"source_content_type":"text/x-python","patch_set":45,"id":"fa32b979_bd850d08","line":121,"in_reply_to":"fa32b979_c82375a6","updated":"2015-06-16 07:10:37.000000000","message":"Sure thing, fix everything can be not the best idea. But it is better than left \"try: ... except Exception\" in the code).\nIn the previous review, I meant that it will be nice to log exception instead of just skipping it. So, I think \nexcutils.save_and_reraise_exception(rerise\u003dFalse)\nwill be good enough.","commit_id":"a8500265f43765b9478ee23c64e14ef4f0bbab12"},{"author":{"_account_id":9377,"name":"Sergey Slipushenko","email":"slipushenko@gmail.com","username":"sslypushenko"},"change_message_id":"9915d399627f7000a761b61665f1929ef55d7a91","unresolved":false,"context_lines":[{"line_number":118,"context_line":"                                clients.servers_client.delete_server("},{"line_number":119,"context_line":"                                    server[\u0027id\u0027])"},{"line_number":120,"context_line":"                            except Exception:"},{"line_number":121,"context_line":"                                with excutils.save_and_reraise_exception():"},{"line_number":122,"context_line":"                                    pass"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    return (body, servers)"}],"source_content_type":"text/x-python","patch_set":45,"id":"fa32b979_5a24c75e","line":121,"in_reply_to":"fa32b979_e9e5db37","updated":"2015-06-17 10:17:00.000000000","message":"Agreed with explicit exception logging with LOG.exception","commit_id":"a8500265f43765b9478ee23c64e14ef4f0bbab12"},{"author":{"_account_id":10016,"name":"Joseph Lanoux","email":"joseph.lanoux@hpe.com","username":"lanoux"},"change_message_id":"f58a80c02d624bf8f1b6c91b82ed53c19303e025","unresolved":false,"context_lines":[{"line_number":118,"context_line":"                                clients.servers_client.delete_server("},{"line_number":119,"context_line":"                                    server[\u0027id\u0027])"},{"line_number":120,"context_line":"                            except Exception:"},{"line_number":121,"context_line":"                                with excutils.save_and_reraise_exception():"},{"line_number":122,"context_line":"                                    pass"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    return (body, servers)"}],"source_content_type":"text/x-python","patch_set":45,"id":"fa32b979_7a298b36","line":121,"in_reply_to":"fa32b979_e9e5db37","updated":"2015-06-17 10:05:06.000000000","message":"Done","commit_id":"a8500265f43765b9478ee23c64e14ef4f0bbab12"},{"author":{"_account_id":7428,"name":"Yaroslav Lobankov","email":"lobankovyv@gmail.com","username":"ylobankov"},"change_message_id":"021b90db77d51ebdcf81d143e4d12ce2f4cd6490","unresolved":false,"context_lines":[{"line_number":121,"context_line":"                                with excutils.save_and_reraise_exception():"},{"line_number":122,"context_line":"                                    pass"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    return (body, servers)"}],"source_content_type":"text/x-python","patch_set":45,"id":"fa32b979_373847a0","line":124,"updated":"2015-06-15 22:32:01.000000000","message":"You can drop parentheses here.\n\nreturn body, servers","commit_id":"a8500265f43765b9478ee23c64e14ef4f0bbab12"},{"author":{"_account_id":7227,"name":"Timur Nurlygayanov","email":"tnurlygayanov@mirantis.com","username":"tnurlygayanov"},"change_message_id":"5503a76a179db45a8e0a6c5bc4a22017ccfd1cf2","unresolved":false,"context_lines":[{"line_number":120,"context_line":"                            except Exception:"},{"line_number":121,"context_line":"                                LOG.exception(\u0027Deleting server %s failed\u0027"},{"line_number":122,"context_line":"                                              % server[\u0027id\u0027])"},{"line_number":123,"context_line":"                                pass"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    return body, servers"}],"source_content_type":"text/x-python","patch_set":46,"id":"fa32b979_9a7b2ff5","line":123,"updated":"2015-06-17 10:09:44.000000000","message":"we don\u0027t need this \u0027pass\u0027 here","commit_id":"bea617fea38ebdd6098a3189f19c6c8fc6934062"}],"tempest/common/server_ssh.py":[{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"ba7be1f8_177f25ac","updated":"2015-02-25 15:55:06.000000000","message":"Perhaps we could call this compute.py and include in future any shared compute related helper function?\nA similarly have network.py etc...","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from tempest.openstack.common import excutils"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"def _associate_floating_ip(self, floating_ip, server):"},{"line_number":23,"context_line":"    \"\"\"Associate a floating IP to the test server.\"\"\""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    build_timeout \u003d self.params[\u0027build_timeout\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_b223c7a5","line":22,"updated":"2015-02-25 15:55:06.000000000","message":"This is not a class method, so no \"self\".\nThe client (or client manager) is missing in the parameters.","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":22,"context_line":"def _associate_floating_ip(self, floating_ip, server):"},{"line_number":23,"context_line":"    \"\"\"Associate a floating IP to the test server.\"\"\""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    build_timeout \u003d self.params[\u0027build_timeout\u0027]"},{"line_number":26,"context_line":"    build_interval \u003d self.params[\u0027build_interval\u0027]"},{"line_number":27,"context_line":"    start \u003d int(time.time())"},{"line_number":28,"context_line":"    timed_out \u003d False"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_52760ba5","line":25,"updated":"2015-02-25 15:55:06.000000000","message":"There\u0027s no self, and no self.params, you\u0027re not in a class.","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    while(not timed_out):"},{"line_number":31,"context_line":"        time.sleep(build_interval)"},{"line_number":32,"context_line":"        try:"},{"line_number":33,"context_line":"            assoc \u003d self.floating_ips_client.associate_floating_ip_to_server"},{"line_number":34,"context_line":"            resp, body \u003d assoc(floating_ip\u003dfloating_ip[\u0027ip\u0027],"},{"line_number":35,"context_line":"                               server_id\u003dserver[\u0027id\u0027])"},{"line_number":36,"context_line":"            break"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_926c03d2","line":33,"updated":"2015-02-25 15:55:06.000000000","message":"ditto, there\u0027s no self.\n\nI\u0027d prefer to avoid \"assoc\" unless mandatory for pep8.","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        try:"},{"line_number":33,"context_line":"            assoc \u003d self.floating_ips_client.associate_floating_ip_to_server"},{"line_number":34,"context_line":"            resp, body \u003d assoc(floating_ip\u003dfloating_ip[\u0027ip\u0027],"},{"line_number":35,"context_line":"                               server_id\u003dserver[\u0027id\u0027])"},{"line_number":36,"context_line":"            break"},{"line_number":37,"context_line":"        except exceptions.BadRequest as bre:"},{"line_number":38,"context_line":"            timed_out \u003d int(time.time()) - start \u003e\u003d build_timeout"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_72d78f4b","line":35,"updated":"2015-02-25 15:55:06.000000000","message":"You\u0027re not using resp, so you can do:\n\n_, body \u003d ...","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":34,"context_line":"            resp, body \u003d assoc(floating_ip\u003dfloating_ip[\u0027ip\u0027],"},{"line_number":35,"context_line":"                               server_id\u003dserver[\u0027id\u0027])"},{"line_number":36,"context_line":"            break"},{"line_number":37,"context_line":"        except exceptions.BadRequest as bre:"},{"line_number":38,"context_line":"            timed_out \u003d int(time.time()) - start \u003e\u003d build_timeout"},{"line_number":39,"context_line":"            if timed_out:"},{"line_number":40,"context_line":"                raise exceptions.TimeoutException(bre)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_5207ebc9","line":37,"range":{"start_line":37,"start_character":8,"end_line":37,"end_character":14},"updated":"2015-02-25 15:55:06.000000000","message":"Can you add a comment to explain why this is necessary.","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":40,"context_line":"                raise exceptions.TimeoutException(bre)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"def create_test_server(self, sshable\u003dFalse, pingable\u003dFalse, **kwargs):"},{"line_number":44,"context_line":"    \"\"\"Create and return a test server that can be pingable or sshable.\"\"\""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    self.params \u003d validation_resources.get_ssh_validation_params()"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_77322160","line":43,"updated":"2015-02-25 15:55:06.000000000","message":"There\u0027s no \"self\"\n\nI don\u0027t see the need of having both sshable and pingable here. This is used to know whether to use the validation resources or not. It might be renamed to validatable.\n\nIn fact it may be possible to drop both: if resources are None it may be used to imply ssh-able is False. Even if fixed_ip is configured and n-net is used, at least keypair will be in the resources for an ssh-able server. However I though it would be good to explicitly set this. So best option probably is validatable\u003dNone by default. If None and resources are passed, or if True, use resources.\n\nThere are things missing without which this cannot work:\n- validation resources\n- client manager\n\nAlso I would prefer to have the wait_until as an explicit parameter. \nThe spec introduces support for wait-until PINGABLE / SSHABLE. I just wanted to clarify whether that\u0027s planned for an upcoming patch - which I think is fine, as this is a lot of code already :)","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"def create_test_server(self, sshable\u003dFalse, pingable\u003dFalse, **kwargs):"},{"line_number":44,"context_line":"    \"\"\"Create and return a test server that can be pingable or sshable.\"\"\""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    self.params \u003d validation_resources.get_ssh_validation_params()"},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_b70f3987","line":44,"updated":"2015-02-25 15:55:06.000000000","message":"It would be good to have some more docstring here, including params and return.","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":43,"context_line":"def create_test_server(self, sshable\u003dFalse, pingable\u003dFalse, **kwargs):"},{"line_number":44,"context_line":"    \"\"\"Create and return a test server that can be pingable or sshable.\"\"\""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    self.params \u003d validation_resources.get_ssh_validation_params()"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    if \u0027name\u0027 in kwargs:"},{"line_number":49,"context_line":"        name \u003d kwargs.pop(\u0027name\u0027)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_72da2f4d","line":46,"updated":"2015-02-25 15:55:06.000000000","message":"no self","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    flavor \u003d kwargs.get(\u0027flavor\u0027, self.params[\u0027flavor_ref\u0027])"},{"line_number":54,"context_line":"    image_id \u003d kwargs.get(\u0027image_id\u0027, self.params[\u0027image_ref\u0027])"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    self.os \u003d self.get_client_manager()"},{"line_number":57,"context_line":"    self.servers_client \u003d self.os.servers_client"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    if pingable or sshable:"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_d214bb45","line":56,"updated":"2015-02-25 15:55:06.000000000","message":"There\u0027s not get_client_manager here, this is not a test class.","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":54,"context_line":"    image_id \u003d kwargs.get(\u0027image_id\u0027, self.params[\u0027image_ref\u0027])"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    self.os \u003d self.get_client_manager()"},{"line_number":57,"context_line":"    self.servers_client \u003d self.os.servers_client"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    if pingable or sshable:"},{"line_number":60,"context_line":"        kwargs[\u0027security_groups\u0027] \u003d \\"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_b2e027fb","line":57,"updated":"2015-02-25 15:55:06.000000000","message":"no self","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    if pingable or sshable:"},{"line_number":60,"context_line":"        kwargs[\u0027security_groups\u0027] \u003d \\"},{"line_number":61,"context_line":"            [{\u0027name\u0027: self.v_data[\u0027security_group\u0027][\u0027name\u0027]}]"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    if sshable and self.params[\u0027auth_method\u0027] \u003d\u003d \u0027keypair\u0027:"},{"line_number":64,"context_line":"        kwargs[\u0027key_name\u0027] \u003d self.v_data[\u0027keypair\u0027][\u0027name\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_528a8b1b","line":61,"updated":"2015-02-25 15:55:06.000000000","message":"You should check if the SG is the resources, you cannot assume it is. \n\nAlso, no self, you must get resources as a parameter.\n\nDifferent from what I wrote in the spec, we may want to append the SG required for validation to the list of SGs already in kwargs (if any)","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":61,"context_line":"            [{\u0027name\u0027: self.v_data[\u0027security_group\u0027][\u0027name\u0027]}]"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    if sshable and self.params[\u0027auth_method\u0027] \u003d\u003d \u0027keypair\u0027:"},{"line_number":64,"context_line":"        kwargs[\u0027key_name\u0027] \u003d self.v_data[\u0027keypair\u0027][\u0027name\u0027]"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    body \u003d self.servers_client.create_server("},{"line_number":67,"context_line":"        name, image_id, flavor, **kwargs)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_72678f4d","line":64,"range":{"start_line":64,"start_character":9,"end_line":64,"end_character":59},"updated":"2015-02-25 15:55:06.000000000","message":"We should not override key_name if already provided, you should check if it exists in kwargs before setting it.\n\nAlso you cannot assume it exists in v_data.","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    if sshable and self.params[\u0027auth_method\u0027] \u003d\u003d \u0027keypair\u0027:"},{"line_number":64,"context_line":"        kwargs[\u0027key_name\u0027] \u003d self.v_data[\u0027keypair\u0027][\u0027name\u0027]"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    body \u003d self.servers_client.create_server("},{"line_number":67,"context_line":"        name, image_id, flavor, **kwargs)"},{"line_number":68,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_40ae3699","line":65,"updated":"2015-02-25 15:55:06.000000000","message":"So like mentioned in the spec, I think it would be good to have a section (or a function) that cares for merging resources and kwargs properly, and that does that only if sshable \u003d\u003d True and run_ssh \u003d\u003d True.\n\nWe also care about the server being attached to (at least) the right network (according to configuration) for ssh to work. So if n-net it\u0027s the fixed network, if neutron the network name for now comes either from tenant isolation or it\u0027s fixed network again. But this is a general problem, so out of scope for this patch.","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":70,"context_line":"    servers \u003d [body]"},{"line_number":71,"context_line":"    if \u0027min_count\u0027 in kwargs or \u0027max_count\u0027 in kwargs:"},{"line_number":72,"context_line":"        # Get servers created which name match with name param."},{"line_number":73,"context_line":"        r, b \u003d self.servers_client.list_servers()"},{"line_number":74,"context_line":"        servers \u003d [s for s in b[\u0027servers\u0027] if s[\u0027name\u0027].startswith(name)]"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    if \u0027wait_until\u0027 in kwargs:"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_20122262","line":73,"updated":"2015-02-25 15:55:06.000000000","message":"r is not used, so \"_\" instead.\nb \u003d\u003e could you use a more meaningful name?","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    if pingable or sshable:"},{"line_number":95,"context_line":"        if self.params[\u0027connect_method\u0027] \u003d\u003d \u0027floating\u0027:"},{"line_number":96,"context_line":"            for server in servers:"},{"line_number":97,"context_line":"                _associate_floating_ip(self,"},{"line_number":98,"context_line":"                                       floating_ip\u003dself.v_data[\u0027floating_ip\u0027],"},{"line_number":99,"context_line":"                                       server\u003dserver)"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_2044e272","line":97,"range":{"start_line":96,"start_character":0,"end_line":97,"end_character":0},"updated":"2015-02-25 15:55:06.000000000","message":"This is migrating the floating ip through all created servers.\nWe need a way in resources to mark if a floating IP is taken already. We could find out via API call but I\u0027d rather avoid the extra API call, the delay, and even relying on that.\n\nThe create_server could care about allocating more floating IPs on the fly when needed. Which poses the issue of their clean-up. Perhaps we could define a class: Server(dict), which is the server body and has extra attributes, like an associated floating IP and RemoteClient (same approach as ResponseBody). And the caller could then extract the extra floating IPs and set them up for clean-up. Or the cleanup procedure could extract the floating IPs from servers.","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":98,"context_line":"                                       floating_ip\u003dself.v_data[\u0027floating_ip\u0027],"},{"line_number":99,"context_line":"                                       server\u003dserver)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    return body"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_435ac02a","line":101,"updated":"2015-02-25 15:55:06.000000000","message":"The return should probably be so that it reduces the impact on existing callers as much as possible. However this should at least be the list of servers (in case *_count is used).","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"}],"tempest/common/utils/linux/remote_client.py":[{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"ba7be1f8_037b688c","updated":"2015-02-25 15:55:06.000000000","message":"This looks like an \"accidental\" change?","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"},{"author":{"_account_id":14271,"name":"Kallebe Monteiro","email":"kallebe.monteiro@lsbd.ufc.br","username":"kallebe"},"change_message_id":"8e1a5c320dab32a94a12408d28c95168dfc6736b","unresolved":false,"context_lines":[{"line_number":31,"context_line":"        network \u003d CONF.compute.network_for_ssh"},{"line_number":32,"context_line":"        ip_version \u003d CONF.compute.ip_version_for_ssh"},{"line_number":33,"context_line":"        ssh_channel_timeout \u003d CONF.compute.ssh_channel_timeout"},{"line_number":34,"context_line":"        \u0027\u0027\u0027if isinstance(server, six.string_types):"},{"line_number":35,"context_line":"            ip_address \u003d server"},{"line_number":36,"context_line":"        else:"},{"line_number":37,"context_line":"            addresses \u003d server[\u0027addresses\u0027][network]"}],"source_content_type":"text/x-python","patch_set":16,"id":"9a80dd14_07d98021","line":34,"updated":"2015-03-16 14:38:18.000000000","message":"I think it is better to remove than let this as commented.","commit_id":"61db62da70ef811a1e8d7654dddbe6ad5b89af7f"}],"tempest/common/validation_resources.py":[{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"1e5af409c4ac149f2a1b3b89b2d396666c18b9d8","unresolved":false,"context_lines":[{"line_number":91,"context_line":"                              % fip_id)"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"def get_ssh_validation_params():"},{"line_number":95,"context_line":"    params \u003d {"},{"line_number":96,"context_line":"        \u0027service\u0027: CONF.compute.catalog_type,"},{"line_number":97,"context_line":"        \u0027region\u0027: CONF.compute.region or CONF.identity.region,"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_c366b0ba","line":94,"updated":"2015-02-25 15:55:06.000000000","message":"The idea in the spec of having this kind of function(s) was to encapsulate any logic in them to avoid duplication.\nFor instance detecting inconsistencies like tenant_networks_reachable \u003d\u003d False and connect_method \u003d\u003d fixed_ip.\nAs it is it does not achieve anything and should be removed.","commit_id":"6e35e818ffe3fab6d174b1ac5b43c68f426ae179"}]}