)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"e19bc3e500d01f2935cd21631406465efabb0e72","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Enable instance validation with the server_actions v2"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Adding the base methods for instance validation on both"},{"line_number":10,"context_line":" fixed and floating ip."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"The after the init_instance_validation method call the"},{"line_number":13,"context_line":" create_test_server method will help in setting up an"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"7adec928_c51f3ab6","line":10,"updated":"2014-05-22 07:07:17.000000000","message":"extra space at beginning of the line.","commit_id":"800dbd5f52c9fe96803bc7982401b15025c51142"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"e19bc3e500d01f2935cd21631406465efabb0e72","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Adding the base methods for instance validation on both"},{"line_number":10,"context_line":" fixed and floating ip."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"The after the init_instance_validation method call the"},{"line_number":13,"context_line":" create_test_server method will help in setting up an"},{"line_number":14,"context_line":" ssh able server."},{"line_number":15,"context_line":"If the init_instance_validation not called the create_test_server,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"7adec928_257e1ee5","line":12,"updated":"2014-05-22 07:07:17.000000000","message":"Then?","commit_id":"800dbd5f52c9fe96803bc7982401b15025c51142"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"e19bc3e500d01f2935cd21631406465efabb0e72","unresolved":false,"context_lines":[{"line_number":15,"context_line":"If the init_instance_validation not called the create_test_server,"},{"line_number":16,"context_line":" behaves as before."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Now just the test_rebuild_server will run with instance validation"},{"line_number":19,"context_line":"as a demo."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Change-Id: I499a11a128b1caa0f92f2392e2f7a511583bf3c9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"7adec928_25653e45","line":18,"updated":"2014-05-22 07:07:17.000000000","message":"It\u0027s actually more than this test only","commit_id":"800dbd5f52c9fe96803bc7982401b15025c51142"}],"tempest/api/compute/base.py":[{"author":{"_account_id":7227,"name":"Timur Nurlygayanov","email":"tnurlygayanov@mirantis.com","username":"tnurlygayanov"},"change_message_id":"2a4f83723ff88ea9f004c4100e17f28d92776407","unresolved":false,"context_lines":[{"line_number":210,"context_line":"            name, image_id, flavor, **server_args)"},{"line_number":211,"context_line":"        if (hasattr(cls, \u0027iv_data\u0027) and"},{"line_number":212,"context_line":"            CONF.compute.ssh_auth_method \u003d\u003d \u0027adminpass\u0027):"},{"line_number":213,"context_line":"                cls.iv_data[\u0027password\u0027] \u003d body[\u0027adminPass\u0027]"},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"        # handle the case of multiple servers"},{"line_number":216,"context_line":"        servers \u003d [body]"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAWH%2F%2Bunc%3D","line":213,"updated":"2014-03-24 19:17:07.000000000","message":"it is ok for pep8, but I suggest to use 4 white spaces for operators after \u0027:\u0027","commit_id":"2ea10fa18e10cd298403432c44efef607fcac872"},{"author":{"_account_id":8824,"name":"Sergey Murashov","email":"smurashov@mirantis.com","username":"SergeyMurashov"},"change_message_id":"bdd5753ea52ee2e2258960d69ab2e9043f88abc2","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        s_description \u003d data_utils.rand_name(\u0027description-\u0027)"},{"line_number":62,"context_line":"        _, secgrp \u003d sec_cli.create_security_group(s_name,"},{"line_number":63,"context_line":"                                                  s_description)"},{"line_number":64,"context_line":"        _, rule \u003d sec_cli.create_security_group_rule(secgrp[\u0027id\u0027],"},{"line_number":65,"context_line":"                                                     \u0027tcp\u0027, 22, 22)"},{"line_number":66,"context_line":"        cls.security_groups.append(secgrp)"},{"line_number":67,"context_line":"        return secgrp"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAWH%2F9v7o%3D","line":64,"updated":"2014-04-02 12:14:15.000000000","message":"unused variable \"rule\", may be would be better:\nsec_cli.create_security_group_rule(secgrp[\u0027id\u0027], \u0027tcp\u0027, 22, 22) ?\ninstead:\n_, rule \u003d sec_cli.create_security_group_rule(secgrp[\u0027id\u0027], \u0027tcp\u0027, 22, 22)","commit_id":"c3bfd3ece1e483b7ff47936ec7bac2238d50bb88"},{"author":{"_account_id":7227,"name":"Timur Nurlygayanov","email":"tnurlygayanov@mirantis.com","username":"tnurlygayanov"},"change_message_id":"8231292bfb2c785280c514956d0059586e688bbd","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        s_description \u003d data_utils.rand_name(\u0027description-\u0027)"},{"line_number":62,"context_line":"        _, secgrp \u003d sec_cli.create_security_group(s_name,"},{"line_number":63,"context_line":"                                                  s_description)"},{"line_number":64,"context_line":"        _, rule \u003d sec_cli.create_security_group_rule(secgrp[\u0027id\u0027],"},{"line_number":65,"context_line":"                                                     \u0027tcp\u0027, 22, 22)"},{"line_number":66,"context_line":"        cls.security_groups.append(secgrp)"},{"line_number":67,"context_line":"        return secgrp"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAWH%2F9v3E%3D","line":64,"in_reply_to":"AAAAWH%2F9v7o%3D","updated":"2014-04-02 12:19:47.000000000","message":"let\u0027s use just\n\n\nsec_cli.create_security_group_rule(secgrp[\u0027id\u0027],\n                                   \u0027tcp\u0027, 22, 22)\n\nwithout  _, rule.","commit_id":"c3bfd3ece1e483b7ff47936ec7bac2238d50bb88"},{"author":{"_account_id":5174,"name":"Mauro Sergio Martins Rodrigues","email":"maurosr@linux.vnet.ibm.com","username":"maurosr"},"change_message_id":"2a21668a1f1e3fadea47d362831fda9295fc4a29","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            cls.iv_data[\u0027floating_ip_associated\u0027] \u003d None"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    @classmethod"},{"line_number":85,"context_line":"    def destoy_instance_validation(cls):"},{"line_number":86,"context_line":"        if hasattr(cls, \u0027iv_data\u0027):"},{"line_number":87,"context_line":"            if \u0027floating_ip\u0027 in cls.iv_data:"},{"line_number":88,"context_line":"                floating_cli \u003d cls.os.floating_ips_client"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAWH%2F9Ook%3D","line":85,"updated":"2014-04-07 20:15:34.000000000","message":"typo dest*r*oy, same for the calls","commit_id":"0a1d75ce2fedffe2f402c4e5c6cd1c8a65cf976c"},{"author":{"_account_id":5174,"name":"Mauro Sergio Martins Rodrigues","email":"maurosr@linux.vnet.ibm.com","username":"maurosr"},"change_message_id":"2a21668a1f1e3fadea47d362831fda9295fc4a29","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        cls.clear_images()"},{"line_number":185,"context_line":"        cls.clear_servers()"},{"line_number":186,"context_line":"        cls.clear_security_groups()"},{"line_number":187,"context_line":"        cls.destoy_instance_validation()"},{"line_number":188,"context_line":"        cls.clear_isolated_creds()"},{"line_number":189,"context_line":"        super(BaseComputeTest, cls).tearDownClass()"},{"line_number":190,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAWH%2F9OlI%3D","line":187,"updated":"2014-04-07 20:15:34.000000000","message":"typo: dest*r*oy.. I would name it clear_servers_validation (to follow name convetion on the methods using servers instead of instance) and maybe call it from inside  clear_servers method","commit_id":"0a1d75ce2fedffe2f402c4e5c6cd1c8a65cf976c"},{"author":{"_account_id":1192,"name":"David Kranz","email":"dkranz@redhat.com","username":"david-kranz"},"change_message_id":"24e67ca6fbdcc3156322d42b1597fffc04528879","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        _, secgrp \u003d sec_cli.create_security_group(s_name,"},{"line_number":63,"context_line":"                                                  s_description)"},{"line_number":64,"context_line":"        sec_cli.create_security_group_rule(secgrp[\u0027id\u0027], \u0027tcp\u0027, 22, 22)"},{"line_number":65,"context_line":"        cls.security_groups.append(secgrp)"},{"line_number":66,"context_line":"        return secgrp"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":9,"id":"9ad9bd40_04969d94","line":65,"updated":"2014-04-30 19:54:33.000000000","message":"Should go right after the call create_security_group call.","commit_id":"318fdb0d9e60ef00e171df095f5470474a1ec5da"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"e19bc3e500d01f2935cd21631406465efabb0e72","unresolved":false,"context_lines":[{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    @classmethod"},{"line_number":125,"context_line":"    def init_instance_validation(cls):"},{"line_number":126,"context_line":"        if CONF.compute.ssh_auth_method !\u003d \u0027disabled\u0027:"},{"line_number":127,"context_line":"            cls.iv_data \u003d {}"},{"line_number":128,"context_line":"            cls.iv_data[\u0027ssh_user\u0027] \u003d CONF.compute.ssh_user"},{"line_number":129,"context_line":"            cls.iv_data[\u0027secgroup\u0027] \u003d cls.create_ssh_sec_group()"}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_a53b0e37","line":126,"updated":"2014-05-22 07:07:17.000000000","message":"Could you add some docstring to explain how to use this function - i.e. only to be called once per class.","commit_id":"800dbd5f52c9fe96803bc7982401b15025c51142"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"e19bc3e500d01f2935cd21631406465efabb0e72","unresolved":false,"context_lines":[{"line_number":278,"context_line":"        super(BaseComputeTest, cls).tearDownClass()"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"    @classmethod"},{"line_number":281,"context_line":"    def create_test_server(cls, **kwargs):"},{"line_number":282,"context_line":"        \"\"\"Wrapper utility that returns a test server.\"\"\""},{"line_number":283,"context_line":"        name \u003d data_utils.rand_name(cls.__name__ + \"-instance\")"},{"line_number":284,"context_line":"        if \u0027validate_server\u0027 in kwargs:"}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_c5dada16","line":281,"updated":"2014-05-22 07:07:17.000000000","message":"I think we need some docstring here explaining how to use this function and its new parameter.","commit_id":"800dbd5f52c9fe96803bc7982401b15025c51142"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"e19bc3e500d01f2935cd21631406465efabb0e72","unresolved":false,"context_lines":[{"line_number":281,"context_line":"    def create_test_server(cls, **kwargs):"},{"line_number":282,"context_line":"        \"\"\"Wrapper utility that returns a test server.\"\"\""},{"line_number":283,"context_line":"        name \u003d data_utils.rand_name(cls.__name__ + \"-instance\")"},{"line_number":284,"context_line":"        if \u0027validate_server\u0027 in kwargs:"},{"line_number":285,"context_line":"            validate_server \u003d kwargs.pop(\u0027validate_server\u0027)"},{"line_number":286,"context_line":"        else:"},{"line_number":287,"context_line":"            validate_server \u003d False"}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_85945245","line":284,"updated":"2014-05-22 07:07:17.000000000","message":"This flag does not trigger any server validation, it makes it possible to happen if needed. What about \"sshable\" or \"enable_ssh\" or \"ssh_enabled\"","commit_id":"800dbd5f52c9fe96803bc7982401b15025c51142"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"e19bc3e500d01f2935cd21631406465efabb0e72","unresolved":false,"context_lines":[{"line_number":282,"context_line":"        \"\"\"Wrapper utility that returns a test server.\"\"\""},{"line_number":283,"context_line":"        name \u003d data_utils.rand_name(cls.__name__ + \"-instance\")"},{"line_number":284,"context_line":"        if \u0027validate_server\u0027 in kwargs:"},{"line_number":285,"context_line":"            validate_server \u003d kwargs.pop(\u0027validate_server\u0027)"},{"line_number":286,"context_line":"        else:"},{"line_number":287,"context_line":"            validate_server \u003d False"},{"line_number":288,"context_line":"        if \u0027name\u0027 in kwargs:"}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_058f6224","line":285,"updated":"2014-05-22 07:07:17.000000000","message":"If you make this as a parameter in the signature you won\u0027t need to pop it from kwargs nor set the default in L287, e.g.\n\ndef create_test_server(cls, sshable\u003dFalse, **kwargs)","commit_id":"800dbd5f52c9fe96803bc7982401b15025c51142"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"e19bc3e500d01f2935cd21631406465efabb0e72","unresolved":false,"context_lines":[{"line_number":289,"context_line":"            name \u003d kwargs.pop(\u0027name\u0027)"},{"line_number":290,"context_line":"        flavor \u003d kwargs.get(\u0027flavor\u0027, cls.flavor_ref)"},{"line_number":291,"context_line":"        image_id \u003d kwargs.get(\u0027image_id\u0027, cls.image_ref)"},{"line_number":292,"context_line":"        if CONF.compute.ssh_auth_method !\u003d \u0027disabled\u0027:"},{"line_number":293,"context_line":"            cls.init_instance_validation()"},{"line_number":294,"context_line":"        server_args \u003d kwargs.copy()"},{"line_number":295,"context_line":"        if validate_server and hasattr(cls, \u0027iv_data\u0027):"}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_45e7aae4","line":292,"updated":"2014-05-22 07:07:17.000000000","message":"You are already checking for !\u003d disabled in the init_instance_validation method, this check is redundant.","commit_id":"800dbd5f52c9fe96803bc7982401b15025c51142"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"e19bc3e500d01f2935cd21631406465efabb0e72","unresolved":false,"context_lines":[{"line_number":290,"context_line":"        flavor \u003d kwargs.get(\u0027flavor\u0027, cls.flavor_ref)"},{"line_number":291,"context_line":"        image_id \u003d kwargs.get(\u0027image_id\u0027, cls.image_ref)"},{"line_number":292,"context_line":"        if CONF.compute.ssh_auth_method !\u003d \u0027disabled\u0027:"},{"line_number":293,"context_line":"            cls.init_instance_validation()"},{"line_number":294,"context_line":"        server_args \u003d kwargs.copy()"},{"line_number":295,"context_line":"        if validate_server and hasattr(cls, \u0027iv_data\u0027):"},{"line_number":296,"context_line":"            keypair \u003d cls.iv_data.get(\u0027keypair\u0027, None)"}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_85cd3289","line":293,"updated":"2014-05-22 07:07:17.000000000","message":"Calling the init_instance_validation every time a server is created will unnecessarily recreate kp, sg and floating IP, and will also cause to leak the previous ones.\n\nI would add some logic so that init_instance_validation is called only the very first time create_test_server is called.\nOr you could have logic in init_instance_validation so that it only creates resources which are not defined yet.","commit_id":"800dbd5f52c9fe96803bc7982401b15025c51142"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"e19bc3e500d01f2935cd21631406465efabb0e72","unresolved":false,"context_lines":[{"line_number":304,"context_line":"            name, image_id, flavor, **server_args)"},{"line_number":305,"context_line":"        if (validate_server and hasattr(cls, \u0027iv_data\u0027) and"},{"line_number":306,"context_line":"                CONF.compute.ssh_auth_method \u003d\u003d \u0027adminpass\u0027):"},{"line_number":307,"context_line":"            cls.iv_data[\u0027password\u0027] \u003d body[\u0027adminPass\u0027]"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"        # handle the case of multiple servers"},{"line_number":310,"context_line":"        servers \u003d [body]"}],"source_content_type":"text/x-python","patch_set":10,"id":"7adec928_a5c6ae47","line":307,"updated":"2014-05-22 07:07:17.000000000","message":"There is only one iv_data dictionary per class. So if a new server is created and we are in adminpass mode, the previous admin pass will be lost. This may be ok, but this limitation should then be included in a comment to make it explicit.\nElse we could have here a dict vm-uuid -\u003e adminpass.","commit_id":"800dbd5f52c9fe96803bc7982401b15025c51142"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"8d22530671626ad431c84f319cac524eaed2f433","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        Builds instance validation data based on ssh_auth_method"},{"line_number":128,"context_line":"        to be called once per test class. This is taken care of"},{"line_number":129,"context_line":"        by invoking it in create_test_server() only if the instance"},{"line_number":130,"context_line":"        is not intialized"},{"line_number":131,"context_line":"        \"\"\""},{"line_number":132,"context_line":"        if CONF.compute.ssh_auth_method !\u003d \u0027disabled\u0027 and \\"},{"line_number":133,"context_line":"            not hasattr(cls, \u0027iv_data\u0027):"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_b1b0d8de","line":130,"updated":"2014-05-27 12:52:36.000000000","message":"The docstring is slightly misleading I believe.\n\nCreates resources required for instance validation based on ssh_auth_method. It creates resources only the first it is\ninvoked, as resouces can be reused by all test servers.","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"72aaf4f324e6751dff3646b27b533f2de14b4051","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        by invoking it in create_test_server() only if the instance"},{"line_number":130,"context_line":"        is not intialized"},{"line_number":131,"context_line":"        \"\"\""},{"line_number":132,"context_line":"        if CONF.compute.ssh_auth_method !\u003d \u0027disabled\u0027 and \\"},{"line_number":133,"context_line":"            not hasattr(cls, \u0027iv_data\u0027):"},{"line_number":134,"context_line":"            cls.iv_data \u003d {}"},{"line_number":135,"context_line":"            cls.iv_data[\u0027ssh_user\u0027] \u003d CONF.compute.ssh_user"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_fd78712d","line":132,"updated":"2014-05-27 15:41:19.000000000","message":"What about run_ssh? Do we still need that option if we can just set disabled in auth?","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"72aaf4f324e6751dff3646b27b533f2de14b4051","unresolved":false,"context_lines":[{"line_number":138,"context_line":"            if CONF.compute.ssh_auth_method \u003d\u003d \u0027keypair\u0027:"},{"line_number":139,"context_line":"                k_name \u003d data_utils.rand_name(\u0027keypair\u0027)"},{"line_number":140,"context_line":"                _, cls.iv_data[\u0027keypair\u0027] \u003d \\"},{"line_number":141,"context_line":"                    cls.os.keypairs_client.create_keypair(k_name)"},{"line_number":142,"context_line":"            floating_cli \u003d cls.os.floating_ips_client"},{"line_number":143,"context_line":"            if CONF.compute.ssh_connect_method \u003d\u003d \u0027floating\u0027:"},{"line_number":144,"context_line":"                _, cls.iv_data[\u0027floating_ip\u0027] \u003d \\"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_c35f5671","line":141,"updated":"2014-05-27 15:41:19.000000000","message":"why not just use the aliased version from setUpClass()","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"72aaf4f324e6751dff3646b27b533f2de14b4051","unresolved":false,"context_lines":[{"line_number":139,"context_line":"                k_name \u003d data_utils.rand_name(\u0027keypair\u0027)"},{"line_number":140,"context_line":"                _, cls.iv_data[\u0027keypair\u0027] \u003d \\"},{"line_number":141,"context_line":"                    cls.os.keypairs_client.create_keypair(k_name)"},{"line_number":142,"context_line":"            floating_cli \u003d cls.os.floating_ips_client"},{"line_number":143,"context_line":"            if CONF.compute.ssh_connect_method \u003d\u003d \u0027floating\u0027:"},{"line_number":144,"context_line":"                _, cls.iv_data[\u0027floating_ip\u0027] \u003d \\"},{"line_number":145,"context_line":"                    floating_cli.create_floating_ip()"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_5d04057e","line":142,"updated":"2014-05-27 15:41:19.000000000","message":"You really don\u0027t need to set this also keypairs client is already aliased in setUpClass() to cls.keypairs_client. Also floating_cli is a bit confusing.","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"72aaf4f324e6751dff3646b27b533f2de14b4051","unresolved":false,"context_lines":[{"line_number":143,"context_line":"            if CONF.compute.ssh_connect_method \u003d\u003d \u0027floating\u0027:"},{"line_number":144,"context_line":"                _, cls.iv_data[\u0027floating_ip\u0027] \u003d \\"},{"line_number":145,"context_line":"                    floating_cli.create_floating_ip()"},{"line_number":146,"context_line":"                cls.iv_data[\u0027floating_ip_associated\u0027] \u003d None"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    @classmethod"},{"line_number":149,"context_line":"    def clear_servers_validation(cls):"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_23f2ba9a","line":146,"updated":"2014-05-27 15:41:19.000000000","message":"This part is incorrect, it would limit the class to have one floating ip for ssh auth. While in reality the config option is used to say a floating ip is needed to be able to ssh into it. In other words fixed ips probably aren\u0027t externally routable but floating ips are.\n\nFor example, if we were going to ssh auth several servers in series without tearing down anything until tearDownClass() (like if we wanted to do a list with details) this would prevent it. Or what if a test is making 2 servers with one api call that need to be sshable?","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5803,"name":"Attila Fazekas","email":"afazekas@redhat.com","username":"afazekas"},"change_message_id":"02fb564fc5e22b5d90011ee4995b747939c15e01","unresolved":false,"context_lines":[{"line_number":143,"context_line":"            if CONF.compute.ssh_connect_method \u003d\u003d \u0027floating\u0027:"},{"line_number":144,"context_line":"                _, cls.iv_data[\u0027floating_ip\u0027] \u003d \\"},{"line_number":145,"context_line":"                    floating_cli.create_floating_ip()"},{"line_number":146,"context_line":"                cls.iv_data[\u0027floating_ip_associated\u0027] \u003d None"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    @classmethod"},{"line_number":149,"context_line":"    def clear_servers_validation(cls):"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_295e11e7","line":146,"in_reply_to":"3ae8d1ca_23f2ba9a","updated":"2014-05-27 20:50:53.000000000","message":"You do not need more than 1 floating IP even if you have multiple servers.\nThe floating IP can be moved to any other server anytime.","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"72aaf4f324e6751dff3646b27b533f2de14b4051","unresolved":false,"context_lines":[{"line_number":149,"context_line":"    def clear_servers_validation(cls):"},{"line_number":150,"context_line":"        if hasattr(cls, \u0027iv_data\u0027):"},{"line_number":151,"context_line":"            if \u0027floating_ip\u0027 in cls.iv_data:"},{"line_number":152,"context_line":"                floating_cli \u003d cls.os.floating_ips_client"},{"line_number":153,"context_line":"                fip_id \u003d cls.iv_data[\u0027floating_ip\u0027][\u0027id\u0027]"},{"line_number":154,"context_line":"                try:"},{"line_number":155,"context_line":"                    floating_cli.delete_floating_ip(fip_id)"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_e867d1e8","line":152,"updated":"2014-05-27 15:41:19.000000000","message":"Same thing here","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"72aaf4f324e6751dff3646b27b533f2de14b4051","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                try:"},{"line_number":155,"context_line":"                    floating_cli.delete_floating_ip(fip_id)"},{"line_number":156,"context_line":"                except exceptions.NotFound:"},{"line_number":157,"context_line":"                    # The ip may have already been deleted which is OK."},{"line_number":158,"context_line":"                    pass"},{"line_number":159,"context_line":"                except Exception:"},{"line_number":160,"context_line":"                    LOG.exception(\u0027Exception raised while deleting ip %s\u0027"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_03513e62","line":157,"updated":"2014-05-27 15:41:19.000000000","message":"There should be a warn log message here.","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"72aaf4f324e6751dff3646b27b533f2de14b4051","unresolved":false,"context_lines":[{"line_number":157,"context_line":"                    # The ip may have already been deleted which is OK."},{"line_number":158,"context_line":"                    pass"},{"line_number":159,"context_line":"                except Exception:"},{"line_number":160,"context_line":"                    LOG.exception(\u0027Exception raised while deleting ip %s\u0027"},{"line_number":161,"context_line":"                                  % fip_id)"},{"line_number":162,"context_line":"            if \u0027keypair\u0027 in cls.iv_data:"},{"line_number":163,"context_line":"                k_name \u003d cls.iv_data[\u0027keypair\u0027][\u0027name\u0027]"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_839aae12","line":160,"updated":"2014-05-27 15:41:19.000000000","message":"I think we just want to have everything fall apart if there is an unexpected exception. Continuing and just logging like this is a bad idea. It will mask real bugs","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"72aaf4f324e6751dff3646b27b533f2de14b4051","unresolved":false,"context_lines":[{"line_number":164,"context_line":"                try:"},{"line_number":165,"context_line":"                    cls.os.keypairs_client.delete_keypair(k_name)"},{"line_number":166,"context_line":"                except exceptions.NotFound:"},{"line_number":167,"context_line":"                    # The key may have already been deleted which is OK."},{"line_number":168,"context_line":"                    pass"},{"line_number":169,"context_line":"                except Exception:"},{"line_number":170,"context_line":"                    LOG.exception(\u0027Exception raised while deleting key %s\u0027"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_83f38e72","line":167,"updated":"2014-05-27 15:41:19.000000000","message":"Same here","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"72aaf4f324e6751dff3646b27b533f2de14b4051","unresolved":false,"context_lines":[{"line_number":167,"context_line":"                    # The key may have already been deleted which is OK."},{"line_number":168,"context_line":"                    pass"},{"line_number":169,"context_line":"                except Exception:"},{"line_number":170,"context_line":"                    LOG.exception(\u0027Exception raised while deleting key %s\u0027"},{"line_number":171,"context_line":"                                  % k_name)"},{"line_number":172,"context_line":"            del cls.iv_data"},{"line_number":173,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_6395a2e3","line":170,"updated":"2014-05-27 15:41:19.000000000","message":"I think we just want to have everything fall apart if there is an unexpected exception. Continuing and just logging like this is a bad idea. It will mask real bugs","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"72aaf4f324e6751dff3646b27b533f2de14b4051","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        if CONF.compute.ssh_connect_method \u003d\u003d \u0027floating\u0027:"},{"line_number":179,"context_line":"            fip \u003d cls.iv_data[\u0027floating_ip\u0027][\u0027ip\u0027]"},{"line_number":180,"context_line":"            if cls.iv_data[\u0027floating_ip_associated\u0027] !\u003d server_id:"},{"line_number":181,"context_line":"                floating_cli \u003d cls.os.floating_ips_client"},{"line_number":182,"context_line":"                floating_cli.associate_floating_ip_to_server(fip,"},{"line_number":183,"context_line":"                                                             server_id)"},{"line_number":184,"context_line":"                cls.iv_data[\u0027floating_ip_associated\u0027] \u003d server_id"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_e35d324a","line":181,"updated":"2014-05-27 15:41:19.000000000","message":"You don\u0027t need to alias this again, it\u0027s done in setUpClass() and just like above cli is a bit confusing.","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5803,"name":"Attila Fazekas","email":"afazekas@redhat.com","username":"afazekas"},"change_message_id":"02fb564fc5e22b5d90011ee4995b747939c15e01","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        if CONF.compute.ssh_connect_method \u003d\u003d \u0027floating\u0027:"},{"line_number":179,"context_line":"            fip \u003d cls.iv_data[\u0027floating_ip\u0027][\u0027ip\u0027]"},{"line_number":180,"context_line":"            if cls.iv_data[\u0027floating_ip_associated\u0027] !\u003d server_id:"},{"line_number":181,"context_line":"                floating_cli \u003d cls.os.floating_ips_client"},{"line_number":182,"context_line":"                floating_cli.associate_floating_ip_to_server(fip,"},{"line_number":183,"context_line":"                                                             server_id)"},{"line_number":184,"context_line":"                cls.iv_data[\u0027floating_ip_associated\u0027] \u003d server_id"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_e93189a3","line":181,"in_reply_to":"3ae8d1ca_e35d324a","updated":"2014-05-27 20:50:53.000000000","message":"The line is too long without any aliasing.","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"72aaf4f324e6751dff3646b27b533f2de14b4051","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            fip \u003d cls.iv_data[\u0027floating_ip\u0027][\u0027ip\u0027]"},{"line_number":180,"context_line":"            if cls.iv_data[\u0027floating_ip_associated\u0027] !\u003d server_id:"},{"line_number":181,"context_line":"                floating_cli \u003d cls.os.floating_ips_client"},{"line_number":182,"context_line":"                floating_cli.associate_floating_ip_to_server(fip,"},{"line_number":183,"context_line":"                                                             server_id)"},{"line_number":184,"context_line":"                cls.iv_data[\u0027floating_ip_associated\u0027] \u003d server_id"},{"line_number":185,"context_line":"            remote_client_args[\u0027server\u0027] \u003d fip"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_7531d5eb","line":182,"updated":"2014-05-27 15:41:19.000000000","message":"If the server already has an associated floating ip you\u0027ll be duplicating work. Which probably isn\u0027t a problem, but I think you need to get a bit more clever about how you\u0027re handling the floating ips here.\n\nFor example, you could just generate a new floating ip for this method. Or you could try to get some more complex reuse logic in place. But, just having a single won\u0027t work well. It\u0027ll also make debugging more difficult than it needs to be.","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5803,"name":"Attila Fazekas","email":"afazekas@redhat.com","username":"afazekas"},"change_message_id":"02fb564fc5e22b5d90011ee4995b747939c15e01","unresolved":false,"context_lines":[{"line_number":179,"context_line":"            fip \u003d cls.iv_data[\u0027floating_ip\u0027][\u0027ip\u0027]"},{"line_number":180,"context_line":"            if cls.iv_data[\u0027floating_ip_associated\u0027] !\u003d server_id:"},{"line_number":181,"context_line":"                floating_cli \u003d cls.os.floating_ips_client"},{"line_number":182,"context_line":"                floating_cli.associate_floating_ip_to_server(fip,"},{"line_number":183,"context_line":"                                                             server_id)"},{"line_number":184,"context_line":"                cls.iv_data[\u0027floating_ip_associated\u0027] \u003d server_id"},{"line_number":185,"context_line":"            remote_client_args[\u0027server\u0027] \u003d fip"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_e9bfc9ce","line":182,"in_reply_to":"3ae8d1ca_7531d5eb","updated":"2014-05-27 20:50:53.000000000","message":"\u0027if cls.iv_data[\u0027floating_ip_associated\u0027] !\u003d server_id\u0027\n\nThis part is not ruining if the server already has the single floating ip.","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"72aaf4f324e6751dff3646b27b533f2de14b4051","unresolved":false,"context_lines":[{"line_number":325,"context_line":"        flavor \u003d kwargs.get(\u0027flavor\u0027, cls.flavor_ref)"},{"line_number":326,"context_line":"        image_id \u003d kwargs.get(\u0027image_id\u0027, cls.image_ref)"},{"line_number":327,"context_line":"        if sshable:"},{"line_number":328,"context_line":"            cls.init_instance_validation()"},{"line_number":329,"context_line":"        server_args \u003d kwargs.copy()"},{"line_number":330,"context_line":"        if sshable and hasattr(cls, \u0027iv_data\u0027):"},{"line_number":331,"context_line":"            keypair \u003d cls.iv_data.get(\u0027keypair\u0027, None)"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_d50f09e0","line":328,"updated":"2014-05-27 15:41:19.000000000","message":"If you\u0027re storing info as class variables to be reused for all test classes. Shouldn\u0027t you do this in setupClass()? It seems weird to do this on every create call. The comment on the method above implies it\u0027s only run once per class too.","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"72aaf4f324e6751dff3646b27b533f2de14b4051","unresolved":false,"context_lines":[{"line_number":327,"context_line":"        if sshable:"},{"line_number":328,"context_line":"            cls.init_instance_validation()"},{"line_number":329,"context_line":"        server_args \u003d kwargs.copy()"},{"line_number":330,"context_line":"        if sshable and hasattr(cls, \u0027iv_data\u0027):"},{"line_number":331,"context_line":"            keypair \u003d cls.iv_data.get(\u0027keypair\u0027, None)"},{"line_number":332,"context_line":"            secgroup \u003d cls.iv_data.get(\u0027secgroup\u0027, None)"},{"line_number":333,"context_line":"            if \u0027key_name\u0027 not in server_args and keypair:"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_d5b469f4","line":330,"updated":"2014-05-27 15:41:19.000000000","message":"why do you need a seperate if block here? While I don\u0027t think you should be doing init_instance_validation in this method, there isn\u0027t a reason to separate it like this.","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"72aaf4f324e6751dff3646b27b533f2de14b4051","unresolved":false,"context_lines":[{"line_number":337,"context_line":""},{"line_number":338,"context_line":"        resp, body \u003d cls.servers_client.create_server("},{"line_number":339,"context_line":"            name, image_id, flavor, **server_args)"},{"line_number":340,"context_line":"        if (sshable and hasattr(cls, \u0027iv_data\u0027)):"},{"line_number":341,"context_line":"            if CONF.compute.ssh_auth_method \u003d\u003d \u0027adminPass\u0027:"},{"line_number":342,"context_line":"                cls.iv_data[\u0027password\u0027][body[\u0027id\u0027]] \u003d \\"},{"line_number":343,"context_line":"                    CONF.compute.image_ssh_password"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_f5b965ea","line":340,"updated":"2014-05-27 15:41:19.000000000","message":"Since if sshable is used to call init_instance_validation then cls will always have iv_data if sshable is true.","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"72aaf4f324e6751dff3646b27b533f2de14b4051","unresolved":false,"context_lines":[{"line_number":356,"context_line":"                try:"},{"line_number":357,"context_line":"                    cls.servers_client.wait_for_server_status("},{"line_number":358,"context_line":"                        server[\u0027id\u0027], kwargs[\u0027wait_until\u0027])"},{"line_number":359,"context_line":"                    if (sshable and hasattr(cls, \u0027iv_data\u0027)):"},{"line_number":360,"context_line":"                        if CONF.compute.ssh_auth_method \u003d\u003d \u0027adminPass\u0027:"},{"line_number":361,"context_line":"                            cls.iv_data[\u0027password\u0027][server[\u0027id\u0027]] \u003d \\"},{"line_number":362,"context_line":"                                CONF.compute.image_ssh_password"}],"source_content_type":"text/x-python","patch_set":14,"id":"3ae8d1ca_55c85974","line":359,"updated":"2014-05-27 15:41:19.000000000","message":"same here","commit_id":"60da146bac2df925cd8e6777fe625bc90247c863"}],"tempest/api/compute/servers/test_server_actions.py":[{"author":{"_account_id":7227,"name":"Timur Nurlygayanov","email":"tnurlygayanov@mirantis.com","username":"tnurlygayanov"},"change_message_id":"79ebc7a33b0fb8c33ca116b1d08da9bb1e77b9ae","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    def setUpClass(cls):"},{"line_number":46,"context_line":"        cls.instance_validation_prepare()"},{"line_number":47,"context_line":"        super(ServerActionsTestJSON, cls).setUpClass()"},{"line_number":48,"context_line":"        if (CONF.compute.ssh_auth_method !\u003d \u0027disabled\u0027):"},{"line_number":49,"context_line":"            cls.init_instance_validation()"},{"line_number":50,"context_line":"        cls.client \u003d cls.servers_client"},{"line_number":51,"context_line":"        cls.server_id \u003d cls.rebuild_server(None)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAWH%2F9TOQ%3D","line":48,"updated":"2014-04-07 09:22:00.000000000","message":"The characters \u0027(\u0027 and \u0027)\u0027 are not required here.","commit_id":"02babc5bbb795995ab09f3208dc7ca402b0fe94a"},{"author":{"_account_id":1192,"name":"David Kranz","email":"dkranz@redhat.com","username":"david-kranz"},"change_message_id":"24e67ca6fbdcc3156322d42b1597fffc04528879","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    def setUpClass(cls):"},{"line_number":46,"context_line":"        cls.prepare_instance_network()"},{"line_number":47,"context_line":"        super(ServerActionsTestJSON, cls).setUpClass()"},{"line_number":48,"context_line":"        if CONF.compute.ssh_auth_method !\u003d \u0027disabled\u0027:"},{"line_number":49,"context_line":"            cls.init_instance_validation()"},{"line_number":50,"context_line":"        cls.client \u003d cls.servers_client"},{"line_number":51,"context_line":"        cls.server_id \u003d cls.rebuild_server(None)"}],"source_content_type":"text/x-python","patch_set":9,"id":"9ad9bd40_84054dc4","line":48,"updated":"2014-04-30 19:54:33.000000000","message":"Why is this to be done in every setUpClass rather than as part of create_test_server?","commit_id":"318fdb0d9e60ef00e171df095f5470474a1ec5da"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"9669acccfb0a4ab6105d6428cec9c72cc64bc12a","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    def setUpClass(cls):"},{"line_number":46,"context_line":"        cls.prepare_instance_network()"},{"line_number":47,"context_line":"        super(ServerActionsTestJSON, cls).setUpClass()"},{"line_number":48,"context_line":"        if CONF.compute.ssh_auth_method !\u003d \u0027disabled\u0027:"},{"line_number":49,"context_line":"            cls.init_instance_validation()"},{"line_number":50,"context_line":"        cls.client \u003d cls.servers_client"},{"line_number":51,"context_line":"        cls.server_id \u003d cls.rebuild_server(None)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7adec928_78a0e8a5","line":48,"in_reply_to":"9ad9bd40_84054dc4","updated":"2014-05-21 13:55:55.000000000","message":"The way the init_instance_validation is written, it will set-up a class level dict and put key and floating IP in there.\n\nIf init_instance_validation is called more than once, it will re-init the dict, and lose the link to keypair, SG and floating IP, which will be leaked.\n\nIf on the other side init_instance_validation is only called in the setupClass like here, we\u0027ll get one keypair, SG and floating which will be reused for every server created.\n\nKP and SG can be re-used at will. Floating IP is only attached then the remote client is requested, so it should also be fine.\n\nHowever I would make request a few changes here:\n- add the check for auth_method !\u003d disabled to the init_instance_validation\n- call the init_instance_validation in create_test_server, but only the first time the method is used. This way resources will only be created by test classes that need them.\n\nAlso we need some docstring to explain a bit how the init method is meant to be used.","commit_id":"318fdb0d9e60ef00e171df095f5470474a1ec5da"},{"author":{"_account_id":1192,"name":"David Kranz","email":"dkranz@redhat.com","username":"david-kranz"},"change_message_id":"24e67ca6fbdcc3156322d42b1597fffc04528879","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        self.client.wait_for_server_status(self.server_id, \u0027ACTIVE\u0027)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"        if (CONF.compute.ssh_auth_method !\u003d \u0027disabled\u0027 and"},{"line_number":64,"context_line":"            CONF.compute.ssh_auth_method \u003d\u003d \u0027adminpass\u0027):"},{"line_number":65,"context_line":"            # Verify that the user can authenticate with the new password"},{"line_number":66,"context_line":"            linux_client \u003d self.get_remote_client_for_server(self.server_id)"},{"line_number":67,"context_line":"            linux_client.validate_authentication()"}],"source_content_type":"text/x-python","patch_set":9,"id":"9ad9bd40_842c6d4c","line":64,"updated":"2014-04-30 19:54:33.000000000","message":"Since this test is explicitly checking the admin password, shouldn\u0027t this code always be executed even if the normal way is some other auth_method?","commit_id":"318fdb0d9e60ef00e171df095f5470474a1ec5da"}]}