)]}'
{"neutron_tempest_plugin/scenario/base.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"2fcb7a252d4e329c4713282fd1e769cb2810036c","unresolved":false,"context_lines":[{"line_number":491,"context_line":"                                CONF.validation.image_ssh_user,"},{"line_number":492,"context_line":"                                self.keypair[\u0027private_key\u0027])"},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"    def setup_network(self, network\u003dNone, router\u003dNone,"},{"line_number":495,"context_line":"                      subnet\u003dNone, port\u003dNone):"},{"line_number":496,"context_line":"        \"\"\"Setup a basic network with internal subnet and router."},{"line_number":497,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_e0bd29b2","line":494,"updated":"2020-01-07 11:34:02.000000000","message":"You can reuse this method in https://github.com/openstack/neutron-tempest-plugin/blob/c0a62c5feca0a5d9329927b2a9a9c799c3e581eb/neutron_tempest_plugin/scenario/base.py#L174","commit_id":"a05792b5f82e586c9c5d4940730dabff78acc717"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"6140f80f1bfe2b0354a10e07fb1eff643012fa1e","unresolved":false,"context_lines":[{"line_number":452,"context_line":"        self.assertEqual(0, result.exit_status)"},{"line_number":453,"context_line":"        return result.stdout"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":"    def create_server_by_port(self, port\u003dNone, server_name\u003dNone, **kwargs):"},{"line_number":456,"context_line":"        \"\"\"Launch an instance using a port interface;"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        In case that the given port is None, a new port is created,"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_64490a64","line":455,"updated":"2020-02-17 09:53:30.000000000","message":"I\u0027m not sure if we really need this method. Can\u0027t You simply spawn instance and prepare needed resources in the test cast or test class where You need it? Or maybe add some helper method directly in the test class if it\u0027s used more than once.","commit_id":"3271056d620062123bca2f42d17cbcd88b30b513"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"6140f80f1bfe2b0354a10e07fb1eff643012fa1e","unresolved":false,"context_lines":[{"line_number":452,"context_line":"        self.assertEqual(0, result.exit_status)"},{"line_number":453,"context_line":"        return result.stdout"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":"    def create_server_by_port(self, port\u003dNone, server_name\u003dNone, **kwargs):"},{"line_number":456,"context_line":"        \"\"\"Launch an instance using a port interface;"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        In case that the given port is None, a new port is created,"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_446deecc","line":455,"range":{"start_line":455,"start_character":67,"end_line":455,"end_character":73},"updated":"2020-02-17 09:53:30.000000000","message":"You are not using those kwargs anywhere so why You need it here?","commit_id":"3271056d620062123bca2f42d17cbcd88b30b513"},{"author":{"_account_id":30920,"name":"Noy fridman","email":"nfridman@redhat.com","username":"nfridman"},"change_message_id":"914070bce3b09c4354f92dab31879bb4625321a7","unresolved":false,"context_lines":[{"line_number":452,"context_line":"        self.assertEqual(0, result.exit_status)"},{"line_number":453,"context_line":"        return result.stdout"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":"    def create_server_by_port(self, port\u003dNone, server_name\u003dNone, **kwargs):"},{"line_number":456,"context_line":"        \"\"\"Launch an instance using a port interface;"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        In case that the given port is None, a new port is created,"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_13c3b68e","line":455,"range":{"start_line":455,"start_character":67,"end_line":455,"end_character":73},"in_reply_to":"3fa7e38b_446deecc","updated":"2020-02-17 15:55:05.000000000","message":"Done","commit_id":"3271056d620062123bca2f42d17cbcd88b30b513"},{"author":{"_account_id":30920,"name":"Noy fridman","email":"nfridman@redhat.com","username":"nfridman"},"change_message_id":"914070bce3b09c4354f92dab31879bb4625321a7","unresolved":false,"context_lines":[{"line_number":452,"context_line":"        self.assertEqual(0, result.exit_status)"},{"line_number":453,"context_line":"        return result.stdout"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":"    def create_server_by_port(self, port\u003dNone, server_name\u003dNone, **kwargs):"},{"line_number":456,"context_line":"        \"\"\"Launch an instance using a port interface;"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        In case that the given port is None, a new port is created,"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_7e72f1b1","line":455,"in_reply_to":"3fa7e38b_64490a64","updated":"2020-02-17 15:55:05.000000000","message":"Will be moved to the scenario.test_qos as a separated function","commit_id":"3271056d620062123bca2f42d17cbcd88b30b513"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"6140f80f1bfe2b0354a10e07fb1eff643012fa1e","unresolved":false,"context_lines":[{"line_number":460,"context_line":"        \"\"\""},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        # Create and activate the port that will be assign to the instance."},{"line_number":463,"context_line":"        self.port \u003d port or self.create_port(self.network)"},{"line_number":464,"context_line":""},{"line_number":465,"context_line":"        if port is None:"},{"line_number":466,"context_line":"            self.fip \u003d self.create_floatingip(port\u003dself.port)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_e4065a1a","line":463,"updated":"2020-02-17 09:53:30.000000000","message":"I would move it to inside \"if\" below. So it would be something like:\n\n    self.port \u003d port\n    if self.port is None:\n        self.port \u003d self.create_port(self.network)\n        self.fip \u003d ...\n        ...","commit_id":"3271056d620062123bca2f42d17cbcd88b30b513"},{"author":{"_account_id":30920,"name":"Noy fridman","email":"nfridman@redhat.com","username":"nfridman"},"change_message_id":"914070bce3b09c4354f92dab31879bb4625321a7","unresolved":false,"context_lines":[{"line_number":460,"context_line":"        \"\"\""},{"line_number":461,"context_line":""},{"line_number":462,"context_line":"        # Create and activate the port that will be assign to the instance."},{"line_number":463,"context_line":"        self.port \u003d port or self.create_port(self.network)"},{"line_number":464,"context_line":""},{"line_number":465,"context_line":"        if port is None:"},{"line_number":466,"context_line":"            self.fip \u003d self.create_floatingip(port\u003dself.port)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_b3850256","line":463,"in_reply_to":"3fa7e38b_e4065a1a","updated":"2020-02-17 15:55:05.000000000","message":"Done","commit_id":"3271056d620062123bca2f42d17cbcd88b30b513"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"6140f80f1bfe2b0354a10e07fb1eff643012fa1e","unresolved":false,"context_lines":[{"line_number":466,"context_line":"            self.fip \u003d self.create_floatingip(port\u003dself.port)"},{"line_number":467,"context_line":""},{"line_number":468,"context_line":"            secgroup \u003d self.os_primary.network_client.create_security_group("},{"line_number":469,"context_line":"                name\u003ddata_utils.rand_name(\u0027secgroup\u0027))"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"            self.security_groups.append(secgroup[\u0027security_group\u0027])"},{"line_number":472,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_c4323ef5","line":469,"updated":"2020-02-17 09:53:30.000000000","message":"You can use create_security_group() method from https://github.com/openstack/neutron-tempest-plugin/blob/b7e0eef8de92f6a70c16c879f6a9a20377e82882/neutron_tempest_plugin/api/base.py#L820 - then L471 shouldn\u0027t be needed","commit_id":"3271056d620062123bca2f42d17cbcd88b30b513"},{"author":{"_account_id":30920,"name":"Noy fridman","email":"nfridman@redhat.com","username":"nfridman"},"change_message_id":"914070bce3b09c4354f92dab31879bb4625321a7","unresolved":false,"context_lines":[{"line_number":466,"context_line":"            self.fip \u003d self.create_floatingip(port\u003dself.port)"},{"line_number":467,"context_line":""},{"line_number":468,"context_line":"            secgroup \u003d self.os_primary.network_client.create_security_group("},{"line_number":469,"context_line":"                name\u003ddata_utils.rand_name(\u0027secgroup\u0027))"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"            self.security_groups.append(secgroup[\u0027security_group\u0027])"},{"line_number":472,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_9388066d","line":469,"in_reply_to":"3fa7e38b_c4323ef5","updated":"2020-02-17 15:55:05.000000000","message":"Done","commit_id":"3271056d620062123bca2f42d17cbcd88b30b513"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"6140f80f1bfe2b0354a10e07fb1eff643012fa1e","unresolved":false,"context_lines":[{"line_number":498,"context_line":"        if server_name is not None:"},{"line_number":499,"context_line":"            server_kwargs[\u0027name\u0027] \u003d server_name"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"        self.server \u003d self.create_server(**server_kwargs)"},{"line_number":502,"context_line":"        self.wait_for_server_active(self.server[\u0027server\u0027])"},{"line_number":503,"context_line":"        self.check_connectivity(self.fip[\u0027floating_ip_address\u0027],"},{"line_number":504,"context_line":"                                CONF.validation.image_ssh_user,"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_842c4695","line":501,"updated":"2020-02-17 09:53:30.000000000","message":"I think that You should return this server object rather than assign it to self.server always.\nNow, this method can\u0027t be used in e.g. test where 2 or more servers will be needed.","commit_id":"3271056d620062123bca2f42d17cbcd88b30b513"}],"neutron_tempest_plugin/scenario/test_qos.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"2fcb7a252d4e329c4713282fd1e769cb2810036c","unresolved":false,"context_lines":[{"line_number":342,"context_line":"        self._create_file_for_bw_tests(ssh_client)"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        # Check that the actual BW is limited according to the port\u0027s policy"},{"line_number":345,"context_line":"        utils.wait_until_true(lambda: self._check_bw("},{"line_number":346,"context_line":"            ssh_client,"},{"line_number":347,"context_line":"            self.fip[\u0027floating_ip_address\u0027],"},{"line_number":348,"context_line":"            port\u003dself.NC_PORT,"},{"line_number":349,"context_line":"            expected_bw\u003dQoSTest.LIMIT_BYTES_SEC * 2),"},{"line_number":350,"context_line":"                              timeout\u003dself.FILE_DOWNLOAD_TIMEOUT,"},{"line_number":351,"context_line":"                              sleep\u003d1)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_a0d97128","line":351,"range":{"start_line":345,"start_character":8,"end_line":351,"end_character":38},"updated":"2020-01-07 11:34:02.000000000","message":"do we really need to check this here? Isn\u0027t enough to check that port still has QoS policy set in API? It would be faster IMO.","commit_id":"a05792b5f82e586c9c5d4940730dabff78acc717"},{"author":{"_account_id":31450,"name":"Alex Katz","email":"akatz@redhat.com","username":"akatz"},"change_message_id":"a3a83d8d4c90f281d1bd80a195258f9c8d01bd13","unresolved":false,"context_lines":[{"line_number":316,"context_line":"        self.os_admin.network_client.update_port("},{"line_number":317,"context_line":"            self.port[\u0027id\u0027], qos_policy_id\u003dport_policy[\u0027id\u0027])"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        retrieved_port \u003d self.os_admin.network_client.show_port("},{"line_number":320,"context_line":"            self.port[\u0027id\u0027])"},{"line_number":321,"context_line":"        self.assertEqual("},{"line_number":322,"context_line":"            port_policy[\u0027id\u0027], retrieved_port[\u0027port\u0027][\u0027qos_policy_id\u0027])"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        # Delete the instance"},{"line_number":325,"context_line":"        self.os_primary.servers_client.delete_server(self.server[\u0027server\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_cf9d80fd","line":322,"range":{"start_line":319,"start_character":8,"end_line":322,"end_character":71},"updated":"2020-02-16 09:04:45.000000000","message":"There is a separate test for that:\nhttps://opendev.org/openstack/neutron-tempest-plugin/src/branch/master/neutron_tempest_plugin/api/test_qos.py#L285\nI think that it is not necessary to be tested here.","commit_id":"3271056d620062123bca2f42d17cbcd88b30b513"},{"author":{"_account_id":30920,"name":"Noy fridman","email":"nfridman@redhat.com","username":"nfridman"},"change_message_id":"914070bce3b09c4354f92dab31879bb4625321a7","unresolved":false,"context_lines":[{"line_number":316,"context_line":"        self.os_admin.network_client.update_port("},{"line_number":317,"context_line":"            self.port[\u0027id\u0027], qos_policy_id\u003dport_policy[\u0027id\u0027])"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        retrieved_port \u003d self.os_admin.network_client.show_port("},{"line_number":320,"context_line":"            self.port[\u0027id\u0027])"},{"line_number":321,"context_line":"        self.assertEqual("},{"line_number":322,"context_line":"            port_policy[\u0027id\u0027], retrieved_port[\u0027port\u0027][\u0027qos_policy_id\u0027])"},{"line_number":323,"context_line":""},{"line_number":324,"context_line":"        # Delete the instance"},{"line_number":325,"context_line":"        self.os_primary.servers_client.delete_server(self.server[\u0027server\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_936d6632","line":322,"range":{"start_line":319,"start_character":8,"end_line":322,"end_character":71},"in_reply_to":"3fa7e38b_cf9d80fd","updated":"2020-02-17 15:55:05.000000000","message":"Done","commit_id":"3271056d620062123bca2f42d17cbcd88b30b513"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"6140f80f1bfe2b0354a10e07fb1eff643012fa1e","unresolved":false,"context_lines":[{"line_number":334,"context_line":"        retrieved_port \u003d self.os_admin.network_client.show_port("},{"line_number":335,"context_line":"            self.port[\u0027id\u0027])"},{"line_number":336,"context_line":"        self.assertEqual("},{"line_number":337,"context_line":"            port_policy[\u0027id\u0027], retrieved_port[\u0027port\u0027][\u0027qos_policy_id\u0027])"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_246c72ce","line":337,"updated":"2020-02-17 09:53:30.000000000","message":"This whole test is more like API test instead of scenario test. Maybe You could move it to the api module? Or e.g. add verification if bw limit is really working on this new instance also?","commit_id":"3271056d620062123bca2f42d17cbcd88b30b513"},{"author":{"_account_id":30920,"name":"Noy fridman","email":"nfridman@redhat.com","username":"nfridman"},"change_message_id":"914070bce3b09c4354f92dab31879bb4625321a7","unresolved":false,"context_lines":[{"line_number":334,"context_line":"        retrieved_port \u003d self.os_admin.network_client.show_port("},{"line_number":335,"context_line":"            self.port[\u0027id\u0027])"},{"line_number":336,"context_line":"        self.assertEqual("},{"line_number":337,"context_line":"            port_policy[\u0027id\u0027], retrieved_port[\u0027port\u0027][\u0027qos_policy_id\u0027])"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_de09a52a","line":337,"in_reply_to":"3fa7e38b_246c72ce","updated":"2020-02-17 15:55:05.000000000","message":"The reason for writing this test as \"scenario\" is the process of launching and connecting the server to the network (which call functions of scenario.base module).","commit_id":"3271056d620062123bca2f42d17cbcd88b30b513"},{"author":{"_account_id":31450,"name":"Alex Katz","email":"akatz@redhat.com","username":"akatz"},"change_message_id":"a5f067f72040efd217caf8e7e523f460301055d5","unresolved":false,"context_lines":[{"line_number":169,"context_line":"        \"\"\""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"        # Create and activate the port that will be assign to the instance."},{"line_number":172,"context_line":"        self.port \u003d port"},{"line_number":173,"context_line":"        if self.port is None:"},{"line_number":174,"context_line":"            self.port \u003d self.create_port(self.network)"},{"line_number":175,"context_line":"            self.fip \u003d self.create_floatingip(port\u003dself.port)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1fa4df85_89e3fda4","line":172,"range":{"start_line":172,"start_character":8,"end_line":172,"end_character":24},"updated":"2020-02-23 14:53:16.000000000","message":"Better not to reassign the variable twice but do something like the following:\n\nif port:\n    self.port \u003d port\nelse:\n    ...","commit_id":"a2212eff506550fb111384c9e1770c0f16b8047e"},{"author":{"_account_id":31450,"name":"Alex Katz","email":"akatz@redhat.com","username":"akatz"},"change_message_id":"a5f067f72040efd217caf8e7e523f460301055d5","unresolved":false,"context_lines":[{"line_number":170,"context_line":""},{"line_number":171,"context_line":"        # Create and activate the port that will be assign to the instance."},{"line_number":172,"context_line":"        self.port \u003d port"},{"line_number":173,"context_line":"        if self.port is None:"},{"line_number":174,"context_line":"            self.port \u003d self.create_port(self.network)"},{"line_number":175,"context_line":"            self.fip \u003d self.create_floatingip(port\u003dself.port)"},{"line_number":176,"context_line":"            secgroup \u003d self.create_security_group()"}],"source_content_type":"text/x-python","patch_set":11,"id":"1fa4df85_89fc5d3f","line":173,"range":{"start_line":173,"start_character":11,"end_line":173,"end_character":20},"updated":"2020-02-23 14:53:16.000000000","message":"I think it is better to explicitly return the port in the end of the function but not to keep it as the \"hidden\" instance variable. It might be not so clear for someone who will use this method.","commit_id":"a2212eff506550fb111384c9e1770c0f16b8047e"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"61c61d8b12fb7a541155b525ecb85649fe64d1e7","unresolved":false,"context_lines":[{"line_number":196,"context_line":"            \u0027image_ref\u0027: CONF.compute.image_ref,"},{"line_number":197,"context_line":"            \u0027key_name\u0027: self.keypair[\u0027name\u0027],"},{"line_number":198,"context_line":"            \u0027networks\u0027: [{\u0027port\u0027: self.port[\u0027id\u0027]}],"},{"line_number":199,"context_line":"            \u0027security_groups\u0027: [{\u0027name\u0027: self.security_groups[-1][\u0027name\u0027]}],"},{"line_number":200,"context_line":"        }"},{"line_number":201,"context_line":"        if server_name is not None:"},{"line_number":202,"context_line":"            server_kwargs[\u0027name\u0027] \u003d server_name"}],"source_content_type":"text/x-python","patch_set":11,"id":"1fa4df85_c93d752d","line":199,"range":{"start_line":199,"start_character":0,"end_line":199,"end_character":76},"updated":"2020-02-23 14:17:28.000000000","message":"Will \"-1\" always work for us here?\nMaybe and we can use the ID instead taking the last item from list, so it will be more accurate?","commit_id":"a2212eff506550fb111384c9e1770c0f16b8047e"},{"author":{"_account_id":31450,"name":"Alex Katz","email":"akatz@redhat.com","username":"akatz"},"change_message_id":"a5f067f72040efd217caf8e7e523f460301055d5","unresolved":false,"context_lines":[{"line_number":196,"context_line":"            \u0027image_ref\u0027: CONF.compute.image_ref,"},{"line_number":197,"context_line":"            \u0027key_name\u0027: self.keypair[\u0027name\u0027],"},{"line_number":198,"context_line":"            \u0027networks\u0027: [{\u0027port\u0027: self.port[\u0027id\u0027]}],"},{"line_number":199,"context_line":"            \u0027security_groups\u0027: [{\u0027name\u0027: self.security_groups[-1][\u0027name\u0027]}],"},{"line_number":200,"context_line":"        }"},{"line_number":201,"context_line":"        if server_name is not None:"},{"line_number":202,"context_line":"            server_kwargs[\u0027name\u0027] \u003d server_name"}],"source_content_type":"text/x-python","patch_set":11,"id":"1fa4df85_c9d9f5f4","line":199,"updated":"2020-02-23 14:53:16.000000000","message":"You don\u0027t need to specify security group during the server creation if it has been already assigned to the port","commit_id":"a2212eff506550fb111384c9e1770c0f16b8047e"},{"author":{"_account_id":31450,"name":"Alex Katz","email":"akatz@redhat.com","username":"akatz"},"change_message_id":"a5f067f72040efd217caf8e7e523f460301055d5","unresolved":false,"context_lines":[{"line_number":198,"context_line":"            \u0027networks\u0027: [{\u0027port\u0027: self.port[\u0027id\u0027]}],"},{"line_number":199,"context_line":"            \u0027security_groups\u0027: [{\u0027name\u0027: self.security_groups[-1][\u0027name\u0027]}],"},{"line_number":200,"context_line":"        }"},{"line_number":201,"context_line":"        if server_name is not None:"},{"line_number":202,"context_line":"            server_kwargs[\u0027name\u0027] \u003d server_name"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        server \u003d self.create_server(**server_kwargs)"},{"line_number":205,"context_line":"        self.wait_for_server_active(server[\u0027server\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"1fa4df85_49ef4587","line":202,"range":{"start_line":201,"start_character":0,"end_line":202,"end_character":47},"updated":"2020-02-23 14:53:16.000000000","message":"I don\u0027t think that we need a specific server name here. Anyway you can find it it the return value","commit_id":"a2212eff506550fb111384c9e1770c0f16b8047e"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"61c61d8b12fb7a541155b525ecb85649fe64d1e7","unresolved":false,"context_lines":[{"line_number":202,"context_line":"            server_kwargs[\u0027name\u0027] \u003d server_name"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        server \u003d self.create_server(**server_kwargs)"},{"line_number":205,"context_line":"        self.wait_for_server_active(server[\u0027server\u0027])"},{"line_number":206,"context_line":"        self.check_connectivity(self.fip[\u0027floating_ip_address\u0027],"},{"line_number":207,"context_line":"                                CONF.validation.image_ssh_user,"},{"line_number":208,"context_line":"                                self.keypair[\u0027private_key\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"1fa4df85_692fa1f6","line":205,"range":{"start_line":205,"start_character":0,"end_line":205,"end_character":53},"updated":"2020-02-23 14:17:28.000000000","message":"Is that possible to indicate user (to add assertion) that server creation has failed?\nFor example, let\u0027s say that after creation threshold is reached, but server is still in \"BUILD\" status.\nBTW it could also be ERROR","commit_id":"a2212eff506550fb111384c9e1770c0f16b8047e"},{"author":{"_account_id":31450,"name":"Alex Katz","email":"akatz@redhat.com","username":"akatz"},"change_message_id":"a5f067f72040efd217caf8e7e523f460301055d5","unresolved":false,"context_lines":[{"line_number":334,"context_line":""},{"line_number":335,"context_line":"    @decorators.idempotent_id(\u002766e5673e-0522-11ea-8d71-362b9e155667\u0027)"},{"line_number":336,"context_line":"    def test_attach_previously_used_port_to_new_instance(self):"},{"line_number":337,"context_line":"        \"\"\"The test covers:"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        Attaching a previously used port with associated policy to"},{"line_number":340,"context_line":"        a new instance enforces the policy\u0027s rules on it."}],"source_content_type":"text/x-python","patch_set":11,"id":"1fa4df85_09e94d7f","line":337,"range":{"start_line":337,"start_character":11,"end_line":337,"end_character":27},"updated":"2020-02-23 14:53:16.000000000","message":"It is better to specify something valuable in the first string of dock-string","commit_id":"a2212eff506550fb111384c9e1770c0f16b8047e"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"61c61d8b12fb7a541155b525ecb85649fe64d1e7","unresolved":false,"context_lines":[{"line_number":374,"context_line":"        retrieved_port \u003d self.os_admin.network_client.show_port("},{"line_number":375,"context_line":"            self.port[\u0027id\u0027])"},{"line_number":376,"context_line":"        self.assertEqual("},{"line_number":377,"context_line":"            port_policy[\u0027id\u0027], retrieved_port[\u0027port\u0027][\u0027qos_policy_id\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"1fa4df85_691801d3","line":377,"range":{"start_line":377,"start_character":12,"end_line":377,"end_character":70},"updated":"2020-02-23 14:17:28.000000000","message":"I\u0027d suggest to add a message to the user, so in case of failure it will add some informative message into the log/output.","commit_id":"a2212eff506550fb111384c9e1770c0f16b8047e"},{"author":{"_account_id":28609,"name":"Arkady Shtempler","email":"ashtempl@redhat.com","username":"ashtempl"},"change_message_id":"61c61d8b12fb7a541155b525ecb85649fe64d1e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"1fa4df85_e9239104","line":378,"updated":"2020-02-23 14:17:28.000000000","message":"I\u0027m not 100% sure, but maybe we have also check that policy\u0027s rule remained the same?\nIt could also be done by comparing rules\u0027 IDs or by its values: \"max_kbps\" and \"max_burst_kbps\".","commit_id":"a2212eff506550fb111384c9e1770c0f16b8047e"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"5f33b62e74e024e76bad7d710c3593b9543c78d1","unresolved":false,"context_lines":[{"line_number":185,"context_line":"                                       secgroup[\u0027id\u0027])"},{"line_number":186,"context_line":"            self.update_port(port\u003dport,"},{"line_number":187,"context_line":"                             port_security_enabled\u003dTrue,"},{"line_number":188,"context_line":"                             security_groups\u003d[secgroup[\u0027id\u0027]])"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        keypair \u003d self.create_keypair()"},{"line_number":191,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"1fa4df85_178bbb67","line":188,"updated":"2020-03-10 09:11:39.000000000","message":"Why You need to do this update()? Why not create security group first and then pass it to the create_port() method?","commit_id":"24b0249595315c272c265dddb90cdfdd34c422a0"},{"author":{"_account_id":30920,"name":"Noy fridman","email":"nfridman@redhat.com","username":"nfridman"},"change_message_id":"2c201e520176e6bd3d7d57763b21631a264562dd","unresolved":false,"context_lines":[{"line_number":185,"context_line":"                                       secgroup[\u0027id\u0027])"},{"line_number":186,"context_line":"            self.update_port(port\u003dport,"},{"line_number":187,"context_line":"                             port_security_enabled\u003dTrue,"},{"line_number":188,"context_line":"                             security_groups\u003d[secgroup[\u0027id\u0027]])"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        keypair \u003d self.create_keypair()"},{"line_number":191,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"1fa4df85_cd891475","line":188,"in_reply_to":"1fa4df85_178bbb67","updated":"2020-03-10 12:03:21.000000000","message":"Done","commit_id":"24b0249595315c272c265dddb90cdfdd34c422a0"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"6a3e1c2750ebec030b76fde432d1c3eae8503c8b","unresolved":false,"context_lines":[{"line_number":332,"context_line":"        \"\"\"The test spawns new instance using port with QoS policy."},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"        Ports with attached QoS policy could be used multiple times."},{"line_number":335,"context_line":"        The policy rules have to be enforced on the new machines. "},{"line_number":336,"context_line":"        \"\"\""},{"line_number":337,"context_line":"        self.network \u003d self.create_network()"},{"line_number":338,"context_line":"        self.subnet \u003d self.create_subnet(self.network)"}],"source_content_type":"text/x-python","patch_set":12,"id":"1fa4df85_fe186f1e","line":335,"updated":"2020-03-05 19:58:58.000000000","message":"nit: please remove trailing whitespace","commit_id":"24b0249595315c272c265dddb90cdfdd34c422a0"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"5f33b62e74e024e76bad7d710c3593b9543c78d1","unresolved":false,"context_lines":[{"line_number":332,"context_line":"        \"\"\"The test spawns new instance using port with QoS policy."},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"        Ports with attached QoS policy could be used multiple times."},{"line_number":335,"context_line":"        The policy rules have to be enforced on the new machines. "},{"line_number":336,"context_line":"        \"\"\""},{"line_number":337,"context_line":"        self.network \u003d self.create_network()"},{"line_number":338,"context_line":"        self.subnet \u003d self.create_subnet(self.network)"}],"source_content_type":"text/x-python","patch_set":12,"id":"1fa4df85_57da9377","line":335,"in_reply_to":"1fa4df85_fe186f1e","updated":"2020-03-10 09:11:39.000000000","message":"+1","commit_id":"24b0249595315c272c265dddb90cdfdd34c422a0"},{"author":{"_account_id":30920,"name":"Noy fridman","email":"nfridman@redhat.com","username":"nfridman"},"change_message_id":"2c201e520176e6bd3d7d57763b21631a264562dd","unresolved":false,"context_lines":[{"line_number":332,"context_line":"        \"\"\"The test spawns new instance using port with QoS policy."},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"        Ports with attached QoS policy could be used multiple times."},{"line_number":335,"context_line":"        The policy rules have to be enforced on the new machines. "},{"line_number":336,"context_line":"        \"\"\""},{"line_number":337,"context_line":"        self.network \u003d self.create_network()"},{"line_number":338,"context_line":"        self.subnet \u003d self.create_subnet(self.network)"}],"source_content_type":"text/x-python","patch_set":12,"id":"1fa4df85_6da34001","line":335,"in_reply_to":"1fa4df85_fe186f1e","updated":"2020-03-10 12:03:21.000000000","message":"Done","commit_id":"24b0249595315c272c265dddb90cdfdd34c422a0"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"5f33b62e74e024e76bad7d710c3593b9543c78d1","unresolved":false,"context_lines":[{"line_number":339,"context_line":"        self.router \u003d self.create_router_by_client()"},{"line_number":340,"context_line":"        self.create_router_interface(self.router[\u0027id\u0027], self.subnet[\u0027id\u0027])"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        first_vm, first_vm_port \u003d self._create_server_by_port()"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        port_policy \u003d self.os_admin.network_client.create_qos_policy("},{"line_number":345,"context_line":"            name\u003d\u0027port-policy\u0027,"}],"source_content_type":"text/x-python","patch_set":12,"id":"1fa4df85_57c37389","line":342,"range":{"start_line":342,"start_character":8,"end_line":342,"end_character":16},"updated":"2020-03-10 09:11:39.000000000","message":"IMO \"vm\" and \"port\" would be enough as names. You are not using \"second_vm\" or \"second_port\" anywhere","commit_id":"24b0249595315c272c265dddb90cdfdd34c422a0"},{"author":{"_account_id":30920,"name":"Noy fridman","email":"nfridman@redhat.com","username":"nfridman"},"change_message_id":"2c201e520176e6bd3d7d57763b21631a264562dd","unresolved":false,"context_lines":[{"line_number":339,"context_line":"        self.router \u003d self.create_router_by_client()"},{"line_number":340,"context_line":"        self.create_router_interface(self.router[\u0027id\u0027], self.subnet[\u0027id\u0027])"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        first_vm, first_vm_port \u003d self._create_server_by_port()"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":"        port_policy \u003d self.os_admin.network_client.create_qos_policy("},{"line_number":345,"context_line":"            name\u003d\u0027port-policy\u0027,"}],"source_content_type":"text/x-python","patch_set":12,"id":"1fa4df85_2db1c847","line":342,"range":{"start_line":342,"start_character":8,"end_line":342,"end_character":16},"in_reply_to":"1fa4df85_57c37389","updated":"2020-03-10 12:03:21.000000000","message":"Done","commit_id":"24b0249595315c272c265dddb90cdfdd34c422a0"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"6a3e1c2750ebec030b76fde432d1c3eae8503c8b","unresolved":false,"context_lines":[{"line_number":372,"context_line":"                         format(port_policy[\u0027id\u0027],"},{"line_number":373,"context_line":"                                retrieved_port[\u0027port\u0027][\u0027qos_policy_id\u0027]))"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"        retrieved_policy \u003d self.os_admin.network_client.show_qos_policy("},{"line_number":376,"context_line":"                           retrieved_port[\u0027port\u0027][\u0027qos_policy_id\u0027])"},{"line_number":377,"context_line":"        retrieved_rule_id \u003d retrieved_policy[\u0027policy\u0027][\u0027rules\u0027][0][\u0027id\u0027]"},{"line_number":378,"context_line":"        self.assertEqual(rule[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":12,"id":"1fa4df85_08c0b029","line":375,"updated":"2020-03-05 19:58:58.000000000","message":"Is it possible to get tot his point in the test if the binding of the port is deferred?  Would it be worthwhile to make sure that the port has successfully been attached to the host?  If it hasn\u0027t, then this is not really testing anything.","commit_id":"24b0249595315c272c265dddb90cdfdd34c422a0"},{"author":{"_account_id":30920,"name":"Noy fridman","email":"nfridman@redhat.com","username":"nfridman"},"change_message_id":"a7654e1dd94e212ac8623068aca5b1e5dbe510bf","unresolved":false,"context_lines":[{"line_number":372,"context_line":"                         format(port_policy[\u0027id\u0027],"},{"line_number":373,"context_line":"                                retrieved_port[\u0027port\u0027][\u0027qos_policy_id\u0027]))"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"        retrieved_policy \u003d self.os_admin.network_client.show_qos_policy("},{"line_number":376,"context_line":"                           retrieved_port[\u0027port\u0027][\u0027qos_policy_id\u0027])"},{"line_number":377,"context_line":"        retrieved_rule_id \u003d retrieved_policy[\u0027policy\u0027][\u0027rules\u0027][0][\u0027id\u0027]"},{"line_number":378,"context_line":"        self.assertEqual(rule[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":12,"id":"1fa4df85_96158342","line":375,"in_reply_to":"1fa4df85_08c0b029","updated":"2020-03-08 08:48:31.000000000","message":"Hi Nate:)\nIn case that the server status is up -  it that possible that the port\u0027s binding has not been finished yet ? (because that what I based my test on)\n\nIf so, does checking that the associated port to the server is up is enough? (using: https://github.com/openstack/tempest/blob/master/tempest/common/waiters.py#L312)\n\nThanks a lot!!","commit_id":"24b0249595315c272c265dddb90cdfdd34c422a0"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"5f33b62e74e024e76bad7d710c3593b9543c78d1","unresolved":false,"context_lines":[{"line_number":372,"context_line":"                         format(port_policy[\u0027id\u0027],"},{"line_number":373,"context_line":"                                retrieved_port[\u0027port\u0027][\u0027qos_policy_id\u0027]))"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"        retrieved_policy \u003d self.os_admin.network_client.show_qos_policy("},{"line_number":376,"context_line":"                           retrieved_port[\u0027port\u0027][\u0027qos_policy_id\u0027])"},{"line_number":377,"context_line":"        retrieved_rule_id \u003d retrieved_policy[\u0027policy\u0027][\u0027rules\u0027][0][\u0027id\u0027]"},{"line_number":378,"context_line":"        self.assertEqual(rule[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":12,"id":"1fa4df85_d76523b6","line":375,"in_reply_to":"1fa4df85_96158342","updated":"2020-03-10 09:11:39.000000000","message":"I think that it\u0027s fine here as in L201 there is connectivity to this new vm checked so if port wouldn\u0027t be bound properly this connectivity test would fail.","commit_id":"24b0249595315c272c265dddb90cdfdd34c422a0"},{"author":{"_account_id":30920,"name":"Noy fridman","email":"nfridman@redhat.com","username":"nfridman"},"change_message_id":"2c201e520176e6bd3d7d57763b21631a264562dd","unresolved":false,"context_lines":[{"line_number":372,"context_line":"                         format(port_policy[\u0027id\u0027],"},{"line_number":373,"context_line":"                                retrieved_port[\u0027port\u0027][\u0027qos_policy_id\u0027]))"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"        retrieved_policy \u003d self.os_admin.network_client.show_qos_policy("},{"line_number":376,"context_line":"                           retrieved_port[\u0027port\u0027][\u0027qos_policy_id\u0027])"},{"line_number":377,"context_line":"        retrieved_rule_id \u003d retrieved_policy[\u0027policy\u0027][\u0027rules\u0027][0][\u0027id\u0027]"},{"line_number":378,"context_line":"        self.assertEqual(rule[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":12,"id":"1fa4df85_0db2cc49","line":375,"in_reply_to":"1fa4df85_d76523b6","updated":"2020-03-10 12:03:21.000000000","message":"Done","commit_id":"24b0249595315c272c265dddb90cdfdd34c422a0"}]}