)]}'
{"neutron_tempest_plugin/scenario/test_floatingip.py":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"e227818a2a9262d339120d1fb14b6cf723a80125","unresolved":false,"context_lines":[{"line_number":169,"context_line":"    same_network \u003d True"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    @classmethod"},{"line_number":172,"context_line":"    @utils.requires_ext(extension\u003d\"router\", service\u003d\"network\")"},{"line_number":173,"context_line":"    @utils.requires_ext(extension\u003d\"qos\", service\u003d\"network\")"},{"line_number":174,"context_line":"    @base_api.require_qos_rule_type(qos_consts.RULE_TYPE_BANDWIDTH_LIMIT)"},{"line_number":175,"context_line":"    def resource_setup(cls):"},{"line_number":176,"context_line":"        super(FloatingIPQosTest, cls).resource_setup()"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f93b717_f554a023","line":173,"range":{"start_line":172,"start_character":0,"end_line":173,"end_character":59},"updated":"2018-01-26 11:35:57.000000000","message":"I have always doubted these two extensions did not loaded at same time.","commit_id":"0ed89f7e9cc15a77befc3804586fd081db8dff71"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"d032ba46c8ea1d77ae91004dd63418b8ce3fe755","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from testscenarios.scenarios import multiply_scenarios"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from neutron_lib import constants as lib_constants"},{"line_number":25,"context_line":"from neutron_lib.services.qos import constants as qos_consts"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"from neutron_tempest_plugin.api import base as base_api"},{"line_number":28,"context_line":"from neutron_tempest_plugin.common import ssh"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa0c359_5c8bce1d","line":25,"updated":"2018-02-11 09:14:30.000000000","message":"neutron_lib imports should be probably in same section as imports from tempest as both are 3rd party libs","commit_id":"c462f3948e4700bc9fae64aac43a11bb06983aec"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"09e0de8195e41d17b3f4d037a76b16dbec710dfb","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from testscenarios.scenarios import multiply_scenarios"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from neutron_lib import constants as lib_constants"},{"line_number":25,"context_line":"from neutron_lib.services.qos import constants as qos_consts"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"from neutron_tempest_plugin.api import base as base_api"},{"line_number":28,"context_line":"from neutron_tempest_plugin.common import ssh"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa0c359_d1a0f2f6","line":25,"in_reply_to":"3fa0c359_5c8bce1d","updated":"2018-02-13 15:38:40.000000000","message":"So I need to move L24 to 3rd imports section too?","commit_id":"c462f3948e4700bc9fae64aac43a11bb06983aec"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0a74a361c0b916c2f2a8b8ede31ba8bd08ee5fa0","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from testscenarios.scenarios import multiply_scenarios"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from neutron_lib import constants as lib_constants"},{"line_number":25,"context_line":"from neutron_lib.services.qos import constants as qos_consts"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"from neutron_tempest_plugin.api import base as base_api"},{"line_number":28,"context_line":"from neutron_tempest_plugin.common import ssh"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa0c359_fb3c9dc0","line":25,"in_reply_to":"3fa0c359_d1a0f2f6","updated":"2018-02-16 09:01:46.000000000","message":"IMO would be good to fix those imports here","commit_id":"c462f3948e4700bc9fae64aac43a11bb06983aec"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"040b3c44cd809e8f624ca97ae1797d50f860bd72","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from testscenarios.scenarios import multiply_scenarios"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from neutron_lib import constants as lib_constants"},{"line_number":25,"context_line":"from neutron_lib.services.qos import constants as qos_consts"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"from neutron_tempest_plugin.api import base as base_api"},{"line_number":28,"context_line":"from neutron_tempest_plugin.common import ssh"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa0c359_3a428a10","line":25,"in_reply_to":"3fa0c359_fb3c9dc0","updated":"2018-02-22 02:34:23.000000000","message":"Done","commit_id":"c462f3948e4700bc9fae64aac43a11bb06983aec"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"d032ba46c8ea1d77ae91004dd63418b8ce3fe755","unresolved":false,"context_lines":[{"line_number":253,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":254,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":255,"context_line":"            max_burst_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":256,"context_line":"            direction\u003dlib_constants.EGRESS_DIRECTION)"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"        rules \u003d self.os_admin.network_client.list_bandwidth_limit_rules("},{"line_number":259,"context_line":"            policy_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa0c359_bc71b246","line":256,"updated":"2018-02-11 09:14:30.000000000","message":"do You need to create ingress and egress rules at once? If You want to check how it works in both directions maybe You could use scenarios for that?","commit_id":"c462f3948e4700bc9fae64aac43a11bb06983aec"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"09e0de8195e41d17b3f4d037a76b16dbec710dfb","unresolved":false,"context_lines":[{"line_number":253,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":254,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":255,"context_line":"            max_burst_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":256,"context_line":"            direction\u003dlib_constants.EGRESS_DIRECTION)"},{"line_number":257,"context_line":""},{"line_number":258,"context_line":"        rules \u003d self.os_admin.network_client.list_bandwidth_limit_rules("},{"line_number":259,"context_line":"            policy_id)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa0c359_5148622b","line":256,"in_reply_to":"3fa0c359_bc71b246","updated":"2018-02-13 15:38:40.000000000","message":"This is a basic test, and usually we set the bandwidth limit in both egress and ingress direction at the same time.","commit_id":"c462f3948e4700bc9fae64aac43a11bb06983aec"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"d032ba46c8ea1d77ae91004dd63418b8ce3fe755","unresolved":false,"context_lines":[{"line_number":260,"context_line":"        rules \u003d rules[\u0027bandwidth_limit_rules\u0027]"},{"line_number":261,"context_line":"        self.assertEqual(2, len(rules))"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        fip \u003d self.os_admin.network_client.get_floatingip("},{"line_number":264,"context_line":"            self.fip[\u0027id\u0027])[\u0027floatingip\u0027]"},{"line_number":265,"context_line":"        self.assertEqual(self.port[\u0027id\u0027], fip.get(\u0027port_id\u0027))"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"        self.os_admin.network_client.update_floatingip("},{"line_number":268,"context_line":"            self.fip[\u0027id\u0027],"},{"line_number":269,"context_line":"            qos_policy_id\u003dpolicy_id)"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"        fip \u003d self.os_admin.network_client.get_floatingip("},{"line_number":272,"context_line":"            self.fip[\u0027id\u0027])[\u0027floatingip\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa0c359_9c74f637","line":269,"range":{"start_line":263,"start_character":0,"end_line":269,"end_character":36},"updated":"2018-02-11 09:14:30.000000000","message":"except this part, Your code is the same as in https://github.com/openstack/neutron-tempest-plugin/blob/master/neutron_tempest_plugin/scenario/test_qos.py#L130\nMaybe You can think about some refactor of existing code and reuse same code in both tests?","commit_id":"c462f3948e4700bc9fae64aac43a11bb06983aec"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"09e0de8195e41d17b3f4d037a76b16dbec710dfb","unresolved":false,"context_lines":[{"line_number":260,"context_line":"        rules \u003d rules[\u0027bandwidth_limit_rules\u0027]"},{"line_number":261,"context_line":"        self.assertEqual(2, len(rules))"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        fip \u003d self.os_admin.network_client.get_floatingip("},{"line_number":264,"context_line":"            self.fip[\u0027id\u0027])[\u0027floatingip\u0027]"},{"line_number":265,"context_line":"        self.assertEqual(self.port[\u0027id\u0027], fip.get(\u0027port_id\u0027))"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"        self.os_admin.network_client.update_floatingip("},{"line_number":268,"context_line":"            self.fip[\u0027id\u0027],"},{"line_number":269,"context_line":"            qos_policy_id\u003dpolicy_id)"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"        fip \u003d self.os_admin.network_client.get_floatingip("},{"line_number":272,"context_line":"            self.fip[\u0027id\u0027])[\u0027floatingip\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa0c359_b1a3e6b1","line":269,"range":{"start_line":263,"start_character":0,"end_line":269,"end_character":36},"in_reply_to":"3fa0c359_9c74f637","updated":"2018-02-13 15:38:40.000000000","message":"Hmm... sounds reasonable, maybe we can do it later in a follow up patch.","commit_id":"c462f3948e4700bc9fae64aac43a11bb06983aec"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0a74a361c0b916c2f2a8b8ede31ba8bd08ee5fa0","unresolved":false,"context_lines":[{"line_number":260,"context_line":"        rules \u003d rules[\u0027bandwidth_limit_rules\u0027]"},{"line_number":261,"context_line":"        self.assertEqual(2, len(rules))"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        fip \u003d self.os_admin.network_client.get_floatingip("},{"line_number":264,"context_line":"            self.fip[\u0027id\u0027])[\u0027floatingip\u0027]"},{"line_number":265,"context_line":"        self.assertEqual(self.port[\u0027id\u0027], fip.get(\u0027port_id\u0027))"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"        self.os_admin.network_client.update_floatingip("},{"line_number":268,"context_line":"            self.fip[\u0027id\u0027],"},{"line_number":269,"context_line":"            qos_policy_id\u003dpolicy_id)"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"        fip \u003d self.os_admin.network_client.get_floatingip("},{"line_number":272,"context_line":"            self.fip[\u0027id\u0027])[\u0027floatingip\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa0c359_bb46154f","line":269,"range":{"start_line":263,"start_character":0,"end_line":269,"end_character":36},"in_reply_to":"3fa0c359_b1a3e6b1","updated":"2018-02-16 09:01:46.000000000","message":"why not now?","commit_id":"c462f3948e4700bc9fae64aac43a11bb06983aec"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"6ed5fac405aaa8c06d8861f3e15728abe040c089","unresolved":false,"context_lines":[{"line_number":260,"context_line":"        rules \u003d rules[\u0027bandwidth_limit_rules\u0027]"},{"line_number":261,"context_line":"        self.assertEqual(2, len(rules))"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        fip \u003d self.os_admin.network_client.get_floatingip("},{"line_number":264,"context_line":"            self.fip[\u0027id\u0027])[\u0027floatingip\u0027]"},{"line_number":265,"context_line":"        self.assertEqual(self.port[\u0027id\u0027], fip.get(\u0027port_id\u0027))"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"        self.os_admin.network_client.update_floatingip("},{"line_number":268,"context_line":"            self.fip[\u0027id\u0027],"},{"line_number":269,"context_line":"            qos_policy_id\u003dpolicy_id)"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"        fip \u003d self.os_admin.network_client.get_floatingip("},{"line_number":272,"context_line":"            self.fip[\u0027id\u0027])[\u0027floatingip\u0027]"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa0c359_3085c907","line":269,"range":{"start_line":263,"start_character":0,"end_line":269,"end_character":36},"in_reply_to":"3fa0c359_bb46154f","updated":"2018-02-22 05:02:23.000000000","message":"OK, done, let\u0027s refactor it.","commit_id":"c462f3948e4700bc9fae64aac43a11bb06983aec"},{"author":{"_account_id":21781,"name":"Hunt Xu","email":"mhuntxu@gmail.com","username":"huntxu"},"change_message_id":"5300b9eaf9f006c61edaa2c4c926dee140a85474","unresolved":false,"context_lines":[{"line_number":270,"context_line":""},{"line_number":271,"context_line":"        fip \u003d self.os_admin.network_client.get_floatingip("},{"line_number":272,"context_line":"            self.fip[\u0027id\u0027])[\u0027floatingip\u0027]"},{"line_number":273,"context_line":"        self.assertEqual(self.port[\u0027id\u0027], fip.get(\u0027qos_policy_id\u0027))"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        self._create_file_for_bw_tests(ssh_client)"},{"line_number":276,"context_line":"        common_utils.wait_until_true(lambda: self._check_bw("}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa0c359_3cf5a2ed","line":273,"range":{"start_line":273,"start_character":36,"end_line":273,"end_character":38},"updated":"2018-02-11 08:18:00.000000000","message":"qos_policy_id? Both failed zuul checks are failing on this.","commit_id":"c462f3948e4700bc9fae64aac43a11bb06983aec"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"09e0de8195e41d17b3f4d037a76b16dbec710dfb","unresolved":false,"context_lines":[{"line_number":270,"context_line":""},{"line_number":271,"context_line":"        fip \u003d self.os_admin.network_client.get_floatingip("},{"line_number":272,"context_line":"            self.fip[\u0027id\u0027])[\u0027floatingip\u0027]"},{"line_number":273,"context_line":"        self.assertEqual(self.port[\u0027id\u0027], fip.get(\u0027qos_policy_id\u0027))"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        self._create_file_for_bw_tests(ssh_client)"},{"line_number":276,"context_line":"        common_utils.wait_until_true(lambda: self._check_bw("}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa0c359_4e01a337","line":273,"range":{"start_line":273,"start_character":36,"end_line":273,"end_character":38},"in_reply_to":"3fa0c359_3cf5a2ed","updated":"2018-02-13 15:38:40.000000000","message":"Done, it should be policy_id.","commit_id":"c462f3948e4700bc9fae64aac43a11bb06983aec"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"040b3c44cd809e8f624ca97ae1797d50f860bd72","unresolved":false,"context_lines":[{"line_number":273,"context_line":"        self.assertEqual(policy_id, fip.get(\u0027qos_policy_id\u0027))"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        self._create_file_for_bw_tests(ssh_client)"},{"line_number":276,"context_line":"        common_utils.wait_until_true(lambda: self._check_bw("},{"line_number":277,"context_line":"            ssh_client,"},{"line_number":278,"context_line":"            self.fip[\u0027floating_ip_address\u0027],"},{"line_number":279,"context_line":"            port\u003dNC_PORT),"},{"line_number":280,"context_line":"            timeout\u003d120,"},{"line_number":281,"context_line":"            sleep\u003d1)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa0c359_daae06e6","line":281,"range":{"start_line":276,"start_character":8,"end_line":281,"end_character":20},"updated":"2018-02-22 02:34:23.000000000","message":"[neutron_tempest_plugin.scenario.test_qos] time_elapsed \u003d 0, total_bytes_read \u003d 1048576, bytes_per_second \u003d 4405668\n\nThe `time_elapsed` of this bandwidth checking is always 0.","commit_id":"25a30a91f16ecef7a25458d0717dd8f473527131"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"5ba07347c37ad537c0e8efc1051707a6eb130e83","unresolved":false,"context_lines":[{"line_number":273,"context_line":"        self.assertEqual(policy_id, fip.get(\u0027qos_policy_id\u0027))"},{"line_number":274,"context_line":""},{"line_number":275,"context_line":"        self._create_file_for_bw_tests(ssh_client)"},{"line_number":276,"context_line":"        common_utils.wait_until_true(lambda: self._check_bw("},{"line_number":277,"context_line":"            ssh_client,"},{"line_number":278,"context_line":"            self.fip[\u0027floating_ip_address\u0027],"},{"line_number":279,"context_line":"            port\u003dNC_PORT),"},{"line_number":280,"context_line":"            timeout\u003d120,"},{"line_number":281,"context_line":"            sleep\u003d1)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa0c359_9044ddb9","line":281,"range":{"start_line":276,"start_character":8,"end_line":281,"end_character":20},"in_reply_to":"3fa0c359_daae06e6","updated":"2018-02-22 05:13:39.000000000","message":"Refactor the log to \"%.16f\" to print the `time_elapsed`.","commit_id":"25a30a91f16ecef7a25458d0717dd8f473527131"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"1009201a46e9b6b004af55da916afda6d367cca4","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        policy_id, ssh_client \u003d self._test_qos_create_basic_resources()"},{"line_number":227,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":228,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":229,"context_line":"            max_burst_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":230,"context_line":"            direction\u003dlib_constants.INGRESS_DIRECTION)"},{"line_number":231,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":232,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"}],"source_content_type":"text/x-python","patch_set":11,"id":"df7087c5_426ce307","line":229,"range":{"start_line":229,"start_character":27,"end_line":229,"end_character":63},"updated":"2018-03-07 08:52:53.000000000","message":"should it be the same constant for burst like for max_kbps? I know that values can be the same but IMHO we should use two different constants for that","commit_id":"5574eec8e8af3c0bd3d8ac77bef580d4dbb30aaa"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"a3fb4aabdc50a7f89cfabc4a39206718efa9359d","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        policy_id, ssh_client \u003d self._test_qos_create_basic_resources()"},{"line_number":227,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":228,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":229,"context_line":"            max_burst_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":230,"context_line":"            direction\u003dlib_constants.INGRESS_DIRECTION)"},{"line_number":231,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":232,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"}],"source_content_type":"text/x-python","patch_set":11,"id":"df7087c5_f536fcb4","line":229,"range":{"start_line":229,"start_character":27,"end_line":229,"end_character":63},"in_reply_to":"df7087c5_426ce307","updated":"2018-03-08 02:10:02.000000000","message":"Done","commit_id":"5574eec8e8af3c0bd3d8ac77bef580d4dbb30aaa"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"1009201a46e9b6b004af55da916afda6d367cca4","unresolved":false,"context_lines":[{"line_number":230,"context_line":"            direction\u003dlib_constants.INGRESS_DIRECTION)"},{"line_number":231,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":232,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":233,"context_line":"            max_burst_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":234,"context_line":"            direction\u003dlib_constants.EGRESS_DIRECTION)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        rules \u003d self.os_admin.network_client.list_bandwidth_limit_rules("}],"source_content_type":"text/x-python","patch_set":11,"id":"df7087c5_0276ebd9","line":233,"range":{"start_line":233,"start_character":37,"end_line":233,"end_character":63},"updated":"2018-03-07 08:52:53.000000000","message":"same here","commit_id":"5574eec8e8af3c0bd3d8ac77bef580d4dbb30aaa"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"a3fb4aabdc50a7f89cfabc4a39206718efa9359d","unresolved":false,"context_lines":[{"line_number":230,"context_line":"            direction\u003dlib_constants.INGRESS_DIRECTION)"},{"line_number":231,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":232,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":233,"context_line":"            max_burst_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":234,"context_line":"            direction\u003dlib_constants.EGRESS_DIRECTION)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        rules \u003d self.os_admin.network_client.list_bandwidth_limit_rules("}],"source_content_type":"text/x-python","patch_set":11,"id":"df7087c5_95f540ce","line":233,"range":{"start_line":233,"start_character":37,"end_line":233,"end_character":63},"in_reply_to":"df7087c5_0276ebd9","updated":"2018-03-08 02:10:02.000000000","message":"Done","commit_id":"5574eec8e8af3c0bd3d8ac77bef580d4dbb30aaa"},{"author":{"_account_id":27329,"name":"Federico Ressi","email":"fressi@redhat.com","username":"fressi_redhat"},"change_message_id":"99107f099c40880a354337bdd516c52c3365efdc","unresolved":false,"context_lines":[{"line_number":233,"context_line":"            max_burst_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":234,"context_line":"            direction\u003dlib_constants.EGRESS_DIRECTION)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        rules \u003d self.os_admin.network_client.list_bandwidth_limit_rules("},{"line_number":237,"context_line":"            policy_id)"},{"line_number":238,"context_line":"        rules \u003d rules[\u0027bandwidth_limit_rules\u0027]"},{"line_number":239,"context_line":"        self.assertEqual(2, len(rules))"}],"source_content_type":"text/x-python","patch_set":11,"id":"df7087c5_cad7a123","line":236,"range":{"start_line":236,"start_character":8,"end_line":236,"end_character":13},"updated":"2018-03-07 14:49:58.000000000","message":"This variable is reassigned various time and each time it is used only once. There are no reason not to condense this code in to two simple lines like below:\n\n  rules \u003d self.os_admin.network_client.list_bandwidth_limit_rules\n  self.assertEqual(2, len(rules[\u0027bandwidth_limit_rules\u0027]))","commit_id":"5574eec8e8af3c0bd3d8ac77bef580d4dbb30aaa"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"a3fb4aabdc50a7f89cfabc4a39206718efa9359d","unresolved":false,"context_lines":[{"line_number":233,"context_line":"            max_burst_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":234,"context_line":"            direction\u003dlib_constants.EGRESS_DIRECTION)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        rules \u003d self.os_admin.network_client.list_bandwidth_limit_rules("},{"line_number":237,"context_line":"            policy_id)"},{"line_number":238,"context_line":"        rules \u003d rules[\u0027bandwidth_limit_rules\u0027]"},{"line_number":239,"context_line":"        self.assertEqual(2, len(rules))"}],"source_content_type":"text/x-python","patch_set":11,"id":"df7087c5_55bc8804","line":236,"range":{"start_line":236,"start_character":8,"end_line":236,"end_character":13},"in_reply_to":"df7087c5_cad7a123","updated":"2018-03-08 02:10:02.000000000","message":"Done","commit_id":"5574eec8e8af3c0bd3d8ac77bef580d4dbb30aaa"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"1009201a46e9b6b004af55da916afda6d367cca4","unresolved":false,"context_lines":[{"line_number":240,"context_line":""},{"line_number":241,"context_line":"        fip \u003d self.os_admin.network_client.get_floatingip("},{"line_number":242,"context_line":"            self.fip[\u0027id\u0027])[\u0027floatingip\u0027]"},{"line_number":243,"context_line":"        self.assertEqual(self.port[\u0027id\u0027], fip.get(\u0027port_id\u0027))"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"        self.os_admin.network_client.update_floatingip("},{"line_number":246,"context_line":"            self.fip[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":11,"id":"df7087c5_a235bff9","line":243,"range":{"start_line":243,"start_character":46,"end_line":243,"end_character":50},"updated":"2018-03-07 08:52:53.000000000","message":"why here You are using \"get()\" and for rules in L238 You are using [\u0027\u0027]?","commit_id":"5574eec8e8af3c0bd3d8ac77bef580d4dbb30aaa"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"a3fb4aabdc50a7f89cfabc4a39206718efa9359d","unresolved":false,"context_lines":[{"line_number":240,"context_line":""},{"line_number":241,"context_line":"        fip \u003d self.os_admin.network_client.get_floatingip("},{"line_number":242,"context_line":"            self.fip[\u0027id\u0027])[\u0027floatingip\u0027]"},{"line_number":243,"context_line":"        self.assertEqual(self.port[\u0027id\u0027], fip.get(\u0027port_id\u0027))"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"        self.os_admin.network_client.update_floatingip("},{"line_number":246,"context_line":"            self.fip[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":11,"id":"df7087c5_153cd091","line":243,"range":{"start_line":243,"start_character":46,"end_line":243,"end_character":50},"in_reply_to":"df7087c5_a235bff9","updated":"2018-03-08 02:10:02.000000000","message":"Done","commit_id":"5574eec8e8af3c0bd3d8ac77bef580d4dbb30aaa"},{"author":{"_account_id":27329,"name":"Federico Ressi","email":"fressi@redhat.com","username":"fressi_redhat"},"change_message_id":"99107f099c40880a354337bdd516c52c3365efdc","unresolved":false,"context_lines":[{"line_number":248,"context_line":""},{"line_number":249,"context_line":"        fip \u003d self.os_admin.network_client.get_floatingip("},{"line_number":250,"context_line":"            self.fip[\u0027id\u0027])[\u0027floatingip\u0027]"},{"line_number":251,"context_line":"        self.assertEqual(policy_id, fip.get(\u0027qos_policy_id\u0027))"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"        self._create_file_for_bw_tests(ssh_client)"},{"line_number":254,"context_line":"        common_utils.wait_until_true(lambda: self._check_bw("}],"source_content_type":"text/x-python","patch_set":11,"id":"df7087c5_aaa02da7","line":251,"range":{"start_line":251,"start_character":45,"end_line":251,"end_character":58},"updated":"2018-03-07 14:49:58.000000000","message":"You could use [] operator as above.","commit_id":"5574eec8e8af3c0bd3d8ac77bef580d4dbb30aaa"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"a3fb4aabdc50a7f89cfabc4a39206718efa9359d","unresolved":false,"context_lines":[{"line_number":248,"context_line":""},{"line_number":249,"context_line":"        fip \u003d self.os_admin.network_client.get_floatingip("},{"line_number":250,"context_line":"            self.fip[\u0027id\u0027])[\u0027floatingip\u0027]"},{"line_number":251,"context_line":"        self.assertEqual(policy_id, fip.get(\u0027qos_policy_id\u0027))"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"        self._create_file_for_bw_tests(ssh_client)"},{"line_number":254,"context_line":"        common_utils.wait_until_true(lambda: self._check_bw("}],"source_content_type":"text/x-python","patch_set":11,"id":"df7087c5_b5408409","line":251,"range":{"start_line":251,"start_character":45,"end_line":251,"end_character":58},"in_reply_to":"df7087c5_aaa02da7","updated":"2018-03-08 02:10:02.000000000","message":"Done","commit_id":"5574eec8e8af3c0bd3d8ac77bef580d4dbb30aaa"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0b3aeb87ff87ab559aaca735490dcd79d020d3df","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        policy_id, ssh_client \u003d self._test_qos_create_basic_resources()"},{"line_number":227,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":228,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":229,"context_line":"            max_burst_kbps\u003dconstants.LIMIT_KILO_BYTES_PER_SECOND,"},{"line_number":230,"context_line":"            direction\u003dlib_constants.INGRESS_DIRECTION)"},{"line_number":231,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":232,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"}],"source_content_type":"text/x-python","patch_set":12,"id":"df7087c5_242d2095","line":229,"updated":"2018-03-08 08:22:24.000000000","message":"burst is not \"per seconds\" so please change name of this constant maybe :)","commit_id":"6ffaec6ed19b51fb702ff9065c1aa1b1b0ac0f70"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"a7c2022eae3f18aa206daca6e67bfd5479d313ba","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        policy_id, ssh_client \u003d self._test_qos_create_basic_resources()"},{"line_number":227,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":228,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":229,"context_line":"            max_burst_kbps\u003dconstants.LIMIT_KILO_BYTES_PER_SECOND,"},{"line_number":230,"context_line":"            direction\u003dlib_constants.INGRESS_DIRECTION)"},{"line_number":231,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":232,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"}],"source_content_type":"text/x-python","patch_set":12,"id":"df7087c5_9e40f809","line":229,"in_reply_to":"df7087c5_242d2095","updated":"2018-03-08 10:43:25.000000000","message":"Ops... Done","commit_id":"6ffaec6ed19b51fb702ff9065c1aa1b1b0ac0f70"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0b3aeb87ff87ab559aaca735490dcd79d020d3df","unresolved":false,"context_lines":[{"line_number":230,"context_line":"            direction\u003dlib_constants.INGRESS_DIRECTION)"},{"line_number":231,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":232,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":233,"context_line":"            max_burst_kbps\u003dconstants.LIMIT_KILO_BYTES_PER_SECOND,"},{"line_number":234,"context_line":"            direction\u003dlib_constants.EGRESS_DIRECTION)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        rules \u003d self.os_admin.network_client.list_bandwidth_limit_rules("}],"source_content_type":"text/x-python","patch_set":12,"id":"df7087c5_04281c84","line":233,"updated":"2018-03-08 08:22:24.000000000","message":"burst is not \"per seconds\" so please change name of this constant maybe :)","commit_id":"6ffaec6ed19b51fb702ff9065c1aa1b1b0ac0f70"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"a7c2022eae3f18aa206daca6e67bfd5479d313ba","unresolved":false,"context_lines":[{"line_number":230,"context_line":"            direction\u003dlib_constants.INGRESS_DIRECTION)"},{"line_number":231,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":232,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":233,"context_line":"            max_burst_kbps\u003dconstants.LIMIT_KILO_BYTES_PER_SECOND,"},{"line_number":234,"context_line":"            direction\u003dlib_constants.EGRESS_DIRECTION)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        rules \u003d self.os_admin.network_client.list_bandwidth_limit_rules("}],"source_content_type":"text/x-python","patch_set":12,"id":"df7087c5_be45bc19","line":233,"in_reply_to":"df7087c5_04281c84","updated":"2018-03-08 10:43:25.000000000","message":"Done","commit_id":"6ffaec6ed19b51fb702ff9065c1aa1b1b0ac0f70"}],"neutron_tempest_plugin/scenario/test_qos.py":[{"author":{"_account_id":27329,"name":"Federico Ressi","email":"fressi@redhat.com","username":"fressi_redhat"},"change_message_id":"9130b14c63395d8db6243a876b011692d2bf9900","unresolved":false,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        return bytes_per_second \u003c\u003d QoSTest.LIMIT_BYTES_SEC"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def _test_qos_create_basic_resources(self):"},{"line_number":132,"context_line":"        self.setup_network_and_server()"},{"line_number":133,"context_line":"        self.check_connectivity(self.fip[\u0027floating_ip_address\u0027],"},{"line_number":134,"context_line":"                                CONF.validation.image_ssh_user,"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_fab437d3","line":131,"range":{"start_line":131,"start_character":8,"end_line":131,"end_character":40},"updated":"2018-03-13 11:41:15.000000000","message":"Is there any reason we have to create both ssh_client and qos_policy in the same method instead of two?\n\nI think calling this method like _create_qos_policy and _create_ssh_client would sound more like human English and easier to read ;-)","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"3c0ee5abff1571e21fad4a699fd22a2690c0cfa3","unresolved":false,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        return bytes_per_second \u003c\u003d QoSTest.LIMIT_BYTES_SEC"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def _test_qos_create_basic_resources(self):"},{"line_number":132,"context_line":"        self.setup_network_and_server()"},{"line_number":133,"context_line":"        self.check_connectivity(self.fip[\u0027floating_ip_address\u0027],"},{"line_number":134,"context_line":"                                CONF.validation.image_ssh_user,"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_c0c4e293","line":131,"range":{"start_line":131,"start_character":8,"end_line":131,"end_character":40},"in_reply_to":"df7087c5_202a7e7e","updated":"2018-03-13 12:17:31.000000000","message":"Done","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e296ac495784b7fd8ed43ccd64b15593e7d32717","unresolved":false,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        return bytes_per_second \u003c\u003d QoSTest.LIMIT_BYTES_SEC"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def _test_qos_create_basic_resources(self):"},{"line_number":132,"context_line":"        self.setup_network_and_server()"},{"line_number":133,"context_line":"        self.check_connectivity(self.fip[\u0027floating_ip_address\u0027],"},{"line_number":134,"context_line":"                                CONF.validation.image_ssh_user,"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_202a7e7e","line":131,"range":{"start_line":131,"start_character":8,"end_line":131,"end_character":40},"in_reply_to":"df7087c5_fab437d3","updated":"2018-03-13 12:12:39.000000000","message":"I agree that it can be split for two different methods","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"8ed4a901fe26cd2cd895672eb7c4e6bb149acf03","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        return bytes_per_second \u003c\u003d QoSTest.LIMIT_BYTES_SEC"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def _test_qos_create_basic_resources(self):"},{"line_number":132,"context_line":"        self.setup_network_and_server()"},{"line_number":133,"context_line":"        self.check_connectivity(self.fip[\u0027floating_ip_address\u0027],"},{"line_number":134,"context_line":"                                CONF.validation.image_ssh_user,"},{"line_number":135,"context_line":"                                self.keypair[\u0027private_key\u0027])"},{"line_number":136,"context_line":"        rulesets \u003d [{\u0027protocol\u0027: \u0027tcp\u0027,"},{"line_number":137,"context_line":"                     \u0027direction\u0027: \u0027ingress\u0027,"},{"line_number":138,"context_line":"                     \u0027port_range_min\u0027: self.NC_PORT,"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_e48afa61","line":135,"range":{"start_line":132,"start_character":0,"end_line":135,"end_character":60},"updated":"2018-03-16 10:49:11.000000000","message":"So do we need a new function for this?","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"3338cb1eb6a3d7776c3546571e5cd8ea233d7a96","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        return bytes_per_second \u003c\u003d QoSTest.LIMIT_BYTES_SEC"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"    def _test_qos_create_basic_resources(self):"},{"line_number":132,"context_line":"        self.setup_network_and_server()"},{"line_number":133,"context_line":"        self.check_connectivity(self.fip[\u0027floating_ip_address\u0027],"},{"line_number":134,"context_line":"                                CONF.validation.image_ssh_user,"},{"line_number":135,"context_line":"                                self.keypair[\u0027private_key\u0027])"},{"line_number":136,"context_line":"        rulesets \u003d [{\u0027protocol\u0027: \u0027tcp\u0027,"},{"line_number":137,"context_line":"                     \u0027direction\u0027: \u0027ingress\u0027,"},{"line_number":138,"context_line":"                     \u0027port_range_min\u0027: self.NC_PORT,"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_754f766a","line":135,"range":{"start_line":132,"start_character":0,"end_line":135,"end_character":60},"in_reply_to":"df7087c5_e48afa61","updated":"2018-03-16 12:56:54.000000000","message":"I think that this is fine","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":27329,"name":"Federico Ressi","email":"fressi@redhat.com","username":"fressi_redhat"},"change_message_id":"9130b14c63395d8db6243a876b011692d2bf9900","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                                self.keypair[\u0027private_key\u0027])"},{"line_number":136,"context_line":"        rulesets \u003d [{\u0027protocol\u0027: \u0027tcp\u0027,"},{"line_number":137,"context_line":"                     \u0027direction\u0027: \u0027ingress\u0027,"},{"line_number":138,"context_line":"                     \u0027port_range_min\u0027: self.NC_PORT,"},{"line_number":139,"context_line":"                     \u0027port_range_max\u0027: self.NC_PORT,"},{"line_number":140,"context_line":"                     \u0027remote_ip_prefix\u0027: \u00270.0.0.0/0\u0027}]"},{"line_number":141,"context_line":"        self.create_secgroup_rules(rulesets,"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_5a752331","line":138,"range":{"start_line":138,"start_character":39,"end_line":138,"end_character":51},"updated":"2018-03-13 11:41:15.000000000","message":"I think this value (port range min and port range max) should be passed from final test as argument to this method instead of defining it as a constant. Some time making thing explicit makes easier to find how to change/personalize parameters.\nYou could use NC_PORT constant as default value if another value is not passed.\n\n  def _create_qos_policy(self, nc_port\u003dNone):\n    nc_port \u003d nc_port or self.NC_PORT\n    ulesets \u003d [{\u0027protocol\u0027: \u0027tcp\u0027,\n                \u0027direction\u0027: \u0027ingress\u0027,\n                \u0027port_range_min\u0027: nc_port,\n                \u0027port_range_max\u0027: nc_port,\n                \u0027remote_ip_prefix\u0027: \u00270.0.0.0/0\u0027}]\n\nThe very best option would be if this port could be changed from a configuration file too.","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e296ac495784b7fd8ed43ccd64b15593e7d32717","unresolved":false,"context_lines":[{"line_number":135,"context_line":"                                self.keypair[\u0027private_key\u0027])"},{"line_number":136,"context_line":"        rulesets \u003d [{\u0027protocol\u0027: \u0027tcp\u0027,"},{"line_number":137,"context_line":"                     \u0027direction\u0027: \u0027ingress\u0027,"},{"line_number":138,"context_line":"                     \u0027port_range_min\u0027: self.NC_PORT,"},{"line_number":139,"context_line":"                     \u0027port_range_max\u0027: self.NC_PORT,"},{"line_number":140,"context_line":"                     \u0027remote_ip_prefix\u0027: \u00270.0.0.0/0\u0027}]"},{"line_number":141,"context_line":"        self.create_secgroup_rules(rulesets,"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_1d332516","line":138,"range":{"start_line":138,"start_character":39,"end_line":138,"end_character":51},"in_reply_to":"df7087c5_5a752331","updated":"2018-03-13 12:12:39.000000000","message":"I don\u0027t think it is really necessary now. If such customization will be needed in future it can be done together with new test which will use it","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":27329,"name":"Federico Ressi","email":"fressi@redhat.com","username":"fressi_redhat"},"change_message_id":"9130b14c63395d8db6243a876b011692d2bf9900","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        self.create_secgroup_rules(rulesets,"},{"line_number":142,"context_line":"                                   self.security_groups[-1][\u0027id\u0027])"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        ssh_client \u003d ssh.Client(self.fip[\u0027floating_ip_address\u0027],"},{"line_number":145,"context_line":"                                CONF.validation.image_ssh_user,"},{"line_number":146,"context_line":"                                pkey\u003dself.keypair[\u0027private_key\u0027])"},{"line_number":147,"context_line":"        policy \u003d self.os_admin.network_client.create_qos_policy("}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_1ac00be1","line":144,"range":{"start_line":144,"start_character":8,"end_line":144,"end_character":18},"updated":"2018-03-13 11:41:15.000000000","message":"SSH client should be created in a separate method called _create_ssh_client","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e296ac495784b7fd8ed43ccd64b15593e7d32717","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        self.create_secgroup_rules(rulesets,"},{"line_number":142,"context_line":"                                   self.security_groups[-1][\u0027id\u0027])"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        ssh_client \u003d ssh.Client(self.fip[\u0027floating_ip_address\u0027],"},{"line_number":145,"context_line":"                                CONF.validation.image_ssh_user,"},{"line_number":146,"context_line":"                                pkey\u003dself.keypair[\u0027private_key\u0027])"},{"line_number":147,"context_line":"        policy \u003d self.os_admin.network_client.create_qos_policy("}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_3d2149b1","line":144,"range":{"start_line":144,"start_character":8,"end_line":144,"end_character":18},"in_reply_to":"df7087c5_1ac00be1","updated":"2018-03-13 12:12:39.000000000","message":"do we really need separate method just for that?","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"8ed4a901fe26cd2cd895672eb7c4e6bb149acf03","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        self.create_secgroup_rules(rulesets,"},{"line_number":142,"context_line":"                                   self.security_groups[-1][\u0027id\u0027])"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        ssh_client \u003d ssh.Client(self.fip[\u0027floating_ip_address\u0027],"},{"line_number":145,"context_line":"                                CONF.validation.image_ssh_user,"},{"line_number":146,"context_line":"                                pkey\u003dself.keypair[\u0027private_key\u0027])"},{"line_number":147,"context_line":"        policy \u003d self.os_admin.network_client.create_qos_policy("}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_e45c3a1a","line":144,"range":{"start_line":144,"start_character":8,"end_line":144,"end_character":18},"in_reply_to":"df7087c5_3d2149b1","updated":"2018-03-16 10:49:11.000000000","message":"Yes, same question, is it necessary to add a new function for that?","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":27329,"name":"Federico Ressi","email":"fressi@redhat.com","username":"fressi_redhat"},"change_message_id":"9130b14c63395d8db6243a876b011692d2bf9900","unresolved":false,"context_lines":[{"line_number":148,"context_line":"                                        name\u003d\u0027test-policy\u0027,"},{"line_number":149,"context_line":"                                        description\u003d\u0027test-qos-policy\u0027,"},{"line_number":150,"context_line":"                                        shared\u003dTrue)"},{"line_number":151,"context_line":"        return policy[\u0027policy\u0027][\u0027id\u0027], ssh_client"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    @decorators.idempotent_id(\u00271f7ed39b-428f-410a-bd2b-db9f465680df\u0027)"},{"line_number":154,"context_line":"    def test_qos(self):"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_7a4a67ac","line":151,"updated":"2018-03-13 11:41:15.000000000","message":"Please create a named tuple with named parameters to make this tool easier to reuse and refactor.\n\n  BasicQosResources \u003d collections.namedtuple(\n    \u0027QosPolicy\u0027, [\u0027id\u0027, \u0027nc_port\u0027])\n\nThen you can use it here as below:\n\n  return BasicQosResources(id\u003dpolicy[\u0027policy\u0027][\u0027id\u0027],\n                           nc_port\u003dnc_port)\n\nWhen using this method code should look more like this:\n\n  policy \u003d self._create_qos_policy()\n  # policy.nc_port gives the port\n  # policy.id gives the policy id","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e296ac495784b7fd8ed43ccd64b15593e7d32717","unresolved":false,"context_lines":[{"line_number":148,"context_line":"                                        name\u003d\u0027test-policy\u0027,"},{"line_number":149,"context_line":"                                        description\u003d\u0027test-qos-policy\u0027,"},{"line_number":150,"context_line":"                                        shared\u003dTrue)"},{"line_number":151,"context_line":"        return policy[\u0027policy\u0027][\u0027id\u0027], ssh_client"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    @decorators.idempotent_id(\u00271f7ed39b-428f-410a-bd2b-db9f465680df\u0027)"},{"line_number":154,"context_line":"    def test_qos(self):"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_80302aea","line":151,"in_reply_to":"df7087c5_7a4a67ac","updated":"2018-03-13 12:12:39.000000000","message":"I\u0027m not sure if it is necessary. As I said before, IMHO making nc_port to be customizable isn\u0027t really necessary now","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"3c0ee5abff1571e21fad4a699fd22a2690c0cfa3","unresolved":false,"context_lines":[{"line_number":148,"context_line":"                                        name\u003d\u0027test-policy\u0027,"},{"line_number":149,"context_line":"                                        description\u003d\u0027test-qos-policy\u0027,"},{"line_number":150,"context_line":"                                        shared\u003dTrue)"},{"line_number":151,"context_line":"        return policy[\u0027policy\u0027][\u0027id\u0027], ssh_client"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    @decorators.idempotent_id(\u00271f7ed39b-428f-410a-bd2b-db9f465680df\u0027)"},{"line_number":154,"context_line":"    def test_qos(self):"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_80682a87","line":151,"in_reply_to":"df7087c5_80302aea","updated":"2018-03-13 12:17:31.000000000","message":"For that collections.namedtuple, I don\u0027t think its necessary.","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":27329,"name":"Federico Ressi","email":"fressi@redhat.com","username":"fressi_redhat"},"change_message_id":"9130b14c63395d8db6243a876b011692d2bf9900","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        return policy[\u0027policy\u0027][\u0027id\u0027], ssh_client"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    @decorators.idempotent_id(\u00271f7ed39b-428f-410a-bd2b-db9f465680df\u0027)"},{"line_number":154,"context_line":"    def test_qos(self):"},{"line_number":155,"context_line":"        \"\"\"This is a basic test that check that a QoS policy with"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"           a bandwidth limit rule is applied correctly by sending"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_fa167755","line":154,"range":{"start_line":154,"start_character":8,"end_line":154,"end_character":16},"updated":"2018-03-13 11:41:15.000000000","message":"I think a final test case should never be derived by another test class. You should have two final classes using methods from shared mixin classes if you want. This method should be for example _test_qos in the mixin and then used by final test cases to implement real test_qos in a more explicit way eventually passing parameters.\n\nWhy? Making code explicit and improving readability of the code. If somebody changes this test would not realize that it is going to affect another test case.","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"3c0ee5abff1571e21fad4a699fd22a2690c0cfa3","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        return policy[\u0027policy\u0027][\u0027id\u0027], ssh_client"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    @decorators.idempotent_id(\u00271f7ed39b-428f-410a-bd2b-db9f465680df\u0027)"},{"line_number":154,"context_line":"    def test_qos(self):"},{"line_number":155,"context_line":"        \"\"\"This is a basic test that check that a QoS policy with"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"           a bandwidth limit rule is applied correctly by sending"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_80fd8ab2","line":154,"range":{"start_line":154,"start_character":8,"end_line":154,"end_character":16},"in_reply_to":"df7087c5_fa167755","updated":"2018-03-13 12:17:31.000000000","message":"This is a general questions, developer should know what they have done.","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":27329,"name":"Federico Ressi","email":"fressi@redhat.com","username":"fressi_redhat"},"change_message_id":"9130b14c63395d8db6243a876b011692d2bf9900","unresolved":false,"context_lines":[{"line_number":159,"context_line":"           Then calculating the bandwidth every ~1 sec by the number of bits"},{"line_number":160,"context_line":"           received / elapsed time."},{"line_number":161,"context_line":"        \"\"\""},{"line_number":162,"context_line":"        policy_id, ssh_client \u003d self._test_qos_create_basic_resources()"},{"line_number":163,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":164,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":165,"context_line":"            max_burst_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND)"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_7d17c146","line":162,"range":{"start_line":162,"start_character":37,"end_line":162,"end_character":69},"updated":"2018-03-13 11:41:15.000000000","message":"I would prefer having two separate calls here:\n  policy \u003d self._create_qos_policy()\n  ssh_client \u003d self._create_ssh_client()\n\nIt would be more explicit and easier to read, change and maintain.","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e296ac495784b7fd8ed43ccd64b15593e7d32717","unresolved":false,"context_lines":[{"line_number":159,"context_line":"           Then calculating the bandwidth every ~1 sec by the number of bits"},{"line_number":160,"context_line":"           received / elapsed time."},{"line_number":161,"context_line":"        \"\"\""},{"line_number":162,"context_line":"        policy_id, ssh_client \u003d self._test_qos_create_basic_resources()"},{"line_number":163,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":164,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":165,"context_line":"            max_burst_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND)"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_e0548600","line":162,"range":{"start_line":162,"start_character":37,"end_line":162,"end_character":69},"in_reply_to":"df7087c5_7d17c146","updated":"2018-03-13 12:12:39.000000000","message":"+1","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"3c0ee5abff1571e21fad4a699fd22a2690c0cfa3","unresolved":false,"context_lines":[{"line_number":159,"context_line":"           Then calculating the bandwidth every ~1 sec by the number of bits"},{"line_number":160,"context_line":"           received / elapsed time."},{"line_number":161,"context_line":"        \"\"\""},{"line_number":162,"context_line":"        policy_id, ssh_client \u003d self._test_qos_create_basic_resources()"},{"line_number":163,"context_line":"        self.os_admin.network_client.create_bandwidth_limit_rule("},{"line_number":164,"context_line":"            policy_id, max_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND,"},{"line_number":165,"context_line":"            max_burst_kbps\u003dconstants.LIMIT_KILO_BITS_PER_SECOND)"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_4035328e","line":162,"range":{"start_line":162,"start_character":37,"end_line":162,"end_character":69},"in_reply_to":"df7087c5_7d17c146","updated":"2018-03-13 12:17:31.000000000","message":"Done","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":27329,"name":"Federico Ressi","email":"fressi@redhat.com","username":"fressi_redhat"},"change_message_id":"9130b14c63395d8db6243a876b011692d2bf9900","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        utils.wait_until_true(lambda: self._check_bw("},{"line_number":173,"context_line":"            ssh_client,"},{"line_number":174,"context_line":"            self.fip[\u0027floating_ip_address\u0027],"},{"line_number":175,"context_line":"            port\u003dself.NC_PORT),"},{"line_number":176,"context_line":"            timeout\u003d120,"},{"line_number":177,"context_line":"            sleep\u003d1)"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_3da2a9fe","line":175,"range":{"start_line":175,"start_character":17,"end_line":175,"end_character":29},"updated":"2018-03-13 11:41:15.000000000","message":"Network ports and addresses should never be used as constants as they should be configurable. There fore here it would be better to use a variable written by self._create_qos_policy method like for example \u0027policy.nc_port\u0027.","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e296ac495784b7fd8ed43ccd64b15593e7d32717","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        utils.wait_until_true(lambda: self._check_bw("},{"line_number":173,"context_line":"            ssh_client,"},{"line_number":174,"context_line":"            self.fip[\u0027floating_ip_address\u0027],"},{"line_number":175,"context_line":"            port\u003dself.NC_PORT),"},{"line_number":176,"context_line":"            timeout\u003d120,"},{"line_number":177,"context_line":"            sleep\u003d1)"}],"source_content_type":"text/x-python","patch_set":14,"id":"df7087c5_0073bab2","line":175,"range":{"start_line":175,"start_character":17,"end_line":175,"end_character":29},"in_reply_to":"df7087c5_3da2a9fe","updated":"2018-03-13 12:12:39.000000000","message":"again, do we really need so many knobs if we will not use it?","commit_id":"9baa519fe25793855677fd515f1fecae0b5300b8"}],"neutron_tempest_plugin/services/network/json/network_client.py":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"e227818a2a9262d339120d1fb14b6cf723a80125","unresolved":false,"context_lines":[{"line_number":916,"context_line":"        update_body.update(kwargs)"},{"line_number":917,"context_line":"        update_body \u003d dict(floatingip\u003dupdate_body)"},{"line_number":918,"context_line":"        update_body \u003d jsonutils.dumps(update_body)"},{"line_number":919,"context_line":"        resp, body \u003d self.put(uri, update_body)"},{"line_number":920,"context_line":"        self.expected_success(200, resp.status)"},{"line_number":921,"context_line":"        body \u003d jsonutils.loads(body)"},{"line_number":922,"context_line":"        return service_client.ResponseBody(resp, body)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5f93b717_3520c8b0","line":919,"range":{"start_line":919,"start_character":8,"end_line":919,"end_character":47},"updated":"2018-01-26 11:35:57.000000000","message":"Here will get a 500 from neutron-server:\n\nUPDATE statement on table \u0027standardattributes\u0027 expected to update 1 row(s); 0 were matched.","commit_id":"0ed89f7e9cc15a77befc3804586fd081db8dff71"},{"author":{"_account_id":27329,"name":"Federico Ressi","email":"fressi@redhat.com","username":"fressi_redhat"},"change_message_id":"99107f099c40880a354337bdd516c52c3365efdc","unresolved":false,"context_lines":[{"line_number":912,"context_line":""},{"line_number":913,"context_line":"    def get_floatingip(self, fip_id):"},{"line_number":914,"context_line":"        uri \u003d \u0027%s/floatingips/%s\u0027 % (self.uri_prefix, fip_id)"},{"line_number":915,"context_line":"        resp, body \u003d self.get(uri)"},{"line_number":916,"context_line":"        self.expected_success(200, resp.status)"},{"line_number":917,"context_line":"        body \u003d jsonutils.loads(body)"},{"line_number":918,"context_line":"        return service_client.ResponseBody(resp, body)"}],"source_content_type":"text/x-python","patch_set":11,"id":"df7087c5_ead94522","line":915,"range":{"start_line":915,"start_character":14,"end_line":915,"end_character":18},"updated":"2018-03-07 14:49:58.000000000","message":"This variable is reassigned twice. Can you please recall this something like for example body_json to mark what is it?","commit_id":"5574eec8e8af3c0bd3d8ac77bef580d4dbb30aaa"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"a3fb4aabdc50a7f89cfabc4a39206718efa9359d","unresolved":false,"context_lines":[{"line_number":912,"context_line":""},{"line_number":913,"context_line":"    def get_floatingip(self, fip_id):"},{"line_number":914,"context_line":"        uri \u003d \u0027%s/floatingips/%s\u0027 % (self.uri_prefix, fip_id)"},{"line_number":915,"context_line":"        resp, body \u003d self.get(uri)"},{"line_number":916,"context_line":"        self.expected_success(200, resp.status)"},{"line_number":917,"context_line":"        body \u003d jsonutils.loads(body)"},{"line_number":918,"context_line":"        return service_client.ResponseBody(resp, body)"}],"source_content_type":"text/x-python","patch_set":11,"id":"df7087c5_9547801e","line":915,"range":{"start_line":915,"start_character":14,"end_line":915,"end_character":18},"in_reply_to":"df7087c5_ead94522","updated":"2018-03-08 02:10:02.000000000","message":"Actually such code is living in this file everywhere. Let\u0027s rename it to resp_body.","commit_id":"5574eec8e8af3c0bd3d8ac77bef580d4dbb30aaa"},{"author":{"_account_id":27329,"name":"Federico Ressi","email":"fressi@redhat.com","username":"fressi_redhat"},"change_message_id":"99107f099c40880a354337bdd516c52c3365efdc","unresolved":false,"context_lines":[{"line_number":921,"context_line":"        uri \u003d \u0027%s/floatingips/%s\u0027 % (self.uri_prefix, fip_id)"},{"line_number":922,"context_line":"        resp, _ \u003d self.get(uri)"},{"line_number":923,"context_line":"        self.expected_success(200, resp.status)"},{"line_number":924,"context_line":"        update_body \u003d {}"},{"line_number":925,"context_line":"        update_body.update(kwargs)"},{"line_number":926,"context_line":"        update_body \u003d dict(floatingip\u003dupdate_body)"},{"line_number":927,"context_line":"        update_body \u003d jsonutils.dumps(update_body)"},{"line_number":928,"context_line":"        resp, body \u003d self.put(uri, update_body)"},{"line_number":929,"context_line":"        self.expected_success(200, resp.status)"},{"line_number":930,"context_line":"        body \u003d jsonutils.loads(body)"}],"source_content_type":"text/x-python","patch_set":11,"id":"df7087c5_ea4e25c0","line":927,"range":{"start_line":924,"start_character":8,"end_line":927,"end_character":50},"updated":"2018-03-07 14:49:58.000000000","message":"Please stop reassigning variables like this. This makes the code very hard to follow. You should prefer populate dictionaries in one single simple declarative step when possible, like for example:\n\n  update_json \u003d jsonutils.dumps(\n    { \u0027floatingip\u0027: kwargs})\n\nIsn\u0027t sudgested code simpler and easier to read?","commit_id":"5574eec8e8af3c0bd3d8ac77bef580d4dbb30aaa"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"a3fb4aabdc50a7f89cfabc4a39206718efa9359d","unresolved":false,"context_lines":[{"line_number":921,"context_line":"        uri \u003d \u0027%s/floatingips/%s\u0027 % (self.uri_prefix, fip_id)"},{"line_number":922,"context_line":"        resp, _ \u003d self.get(uri)"},{"line_number":923,"context_line":"        self.expected_success(200, resp.status)"},{"line_number":924,"context_line":"        update_body \u003d {}"},{"line_number":925,"context_line":"        update_body.update(kwargs)"},{"line_number":926,"context_line":"        update_body \u003d dict(floatingip\u003dupdate_body)"},{"line_number":927,"context_line":"        update_body \u003d jsonutils.dumps(update_body)"},{"line_number":928,"context_line":"        resp, body \u003d self.put(uri, update_body)"},{"line_number":929,"context_line":"        self.expected_success(200, resp.status)"},{"line_number":930,"context_line":"        body \u003d jsonutils.loads(body)"}],"source_content_type":"text/x-python","patch_set":11,"id":"df7087c5_d545f818","line":927,"range":{"start_line":924,"start_character":8,"end_line":927,"end_character":50},"in_reply_to":"df7087c5_ea4e25c0","updated":"2018-03-08 02:10:02.000000000","message":"Ditto, and done, let\u0027s start a new code style.","commit_id":"5574eec8e8af3c0bd3d8ac77bef580d4dbb30aaa"},{"author":{"_account_id":27329,"name":"Federico Ressi","email":"fressi@redhat.com","username":"fressi_redhat"},"change_message_id":"99107f099c40880a354337bdd516c52c3365efdc","unresolved":false,"context_lines":[{"line_number":927,"context_line":"        update_body \u003d jsonutils.dumps(update_body)"},{"line_number":928,"context_line":"        resp, body \u003d self.put(uri, update_body)"},{"line_number":929,"context_line":"        self.expected_success(200, resp.status)"},{"line_number":930,"context_line":"        body \u003d jsonutils.loads(body)"},{"line_number":931,"context_line":"        return service_client.ResponseBody(resp, body)"},{"line_number":932,"context_line":""},{"line_number":933,"context_line":"    def create_network_keystone_v3(self, name, project_id, tenant_id\u003dNone):"}],"source_content_type":"text/x-python","patch_set":11,"id":"df7087c5_eae385a3","line":930,"range":{"start_line":930,"start_character":8,"end_line":930,"end_character":12},"updated":"2018-03-07 14:49:58.000000000","message":"Can you assign a value with an actual meaning to all these bodies instead of always body. What does they contain in this context? Shouldn\u0027t update_result a better name for this variable (for example)?","commit_id":"5574eec8e8af3c0bd3d8ac77bef580d4dbb30aaa"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"a3fb4aabdc50a7f89cfabc4a39206718efa9359d","unresolved":false,"context_lines":[{"line_number":927,"context_line":"        update_body \u003d jsonutils.dumps(update_body)"},{"line_number":928,"context_line":"        resp, body \u003d self.put(uri, update_body)"},{"line_number":929,"context_line":"        self.expected_success(200, resp.status)"},{"line_number":930,"context_line":"        body \u003d jsonutils.loads(body)"},{"line_number":931,"context_line":"        return service_client.ResponseBody(resp, body)"},{"line_number":932,"context_line":""},{"line_number":933,"context_line":"    def create_network_keystone_v3(self, name, project_id, tenant_id\u003dNone):"}],"source_content_type":"text/x-python","patch_set":11,"id":"df7087c5_754aec27","line":930,"range":{"start_line":930,"start_character":8,"end_line":930,"end_character":12},"in_reply_to":"df7087c5_eae385a3","updated":"2018-03-08 02:10:02.000000000","message":"ditto","commit_id":"5574eec8e8af3c0bd3d8ac77bef580d4dbb30aaa"},{"author":{"_account_id":27329,"name":"Federico Ressi","email":"fressi@redhat.com","username":"fressi_redhat"},"change_message_id":"5b013eebbaa7cc36f08c6fc91128020b98c50092","unresolved":false,"context_lines":[{"line_number":921,"context_line":"        uri \u003d \u0027%s/floatingips/%s\u0027 % (self.uri_prefix, fip_id)"},{"line_number":922,"context_line":"        get_resp, _ \u003d self.get(uri)"},{"line_number":923,"context_line":"        self.expected_success(200, get_resp.status)"},{"line_number":924,"context_line":"        fip_input \u003d {}"},{"line_number":925,"context_line":"        fip_input.update(kwargs)"},{"line_number":926,"context_line":"        put_body \u003d {\u0027floatingip\u0027: fip_input}"},{"line_number":927,"context_line":"        put_body \u003d jsonutils.dumps(put_body)"}],"source_content_type":"text/x-python","patch_set":13,"id":"df7087c5_21d176fc","line":924,"updated":"2018-03-09 07:41:43.000000000","message":"Please keep things short and simple to make everything easy to read and maintain. There is no reason of not putting many things in one line if required characters are so few.\n\n\nAs you aren\u0027t putting any other value in fit_input dictionary you could do:\n\n  fip_input \u003d dict(kwargs)\n\n\nor in this case also:  \n\n  fip_input \u003d kwargs\n\n\nor just this shorter form (my favorite in absolute):\n\n  put_body \u003d jsonutils.dumps({\u0027floatingip\u0027: kwargs})","commit_id":"5ed95fa5b06d2eced74d3676a5840f491809970e"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"8766a4bde116c959effce0d5d282bc44743fcaf9","unresolved":false,"context_lines":[{"line_number":921,"context_line":"        uri \u003d \u0027%s/floatingips/%s\u0027 % (self.uri_prefix, fip_id)"},{"line_number":922,"context_line":"        get_resp, _ \u003d self.get(uri)"},{"line_number":923,"context_line":"        self.expected_success(200, get_resp.status)"},{"line_number":924,"context_line":"        fip_input \u003d {}"},{"line_number":925,"context_line":"        fip_input.update(kwargs)"},{"line_number":926,"context_line":"        put_body \u003d {\u0027floatingip\u0027: fip_input}"},{"line_number":927,"context_line":"        put_body \u003d jsonutils.dumps(put_body)"}],"source_content_type":"text/x-python","patch_set":13,"id":"df7087c5_b22a928d","line":924,"in_reply_to":"df7087c5_21d176fc","updated":"2018-03-12 06:09:18.000000000","message":"Done","commit_id":"5ed95fa5b06d2eced74d3676a5840f491809970e"}]}