)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"10aac428fdd3da2bfc3a11ba1110dd2bed084920","unresolved":true,"context_lines":[{"line_number":16,"context_line":"that tests that a load balancer pool"},{"line_number":17,"context_line":"can recover from a load balancer failover"},{"line_number":18,"context_line":"* Merged _test_basic_traffic and _test_disturbed_traffic into a single method"},{"line_number":19,"context_line":"* fixed typo on line 207"},{"line_number":20,"context_line":"* fixed udp port clash on line 262"},{"line_number":21,"context_line":"* fixed typo on line 263"},{"line_number":22,"context_line":"* corrected the class name on line 263"},{"line_number":23,"context_line":"* corrected the invocation of the dsturb_func"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Change-Id: I4f6c16a164e08996581ba60a48ba7c480b5c4902"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"bba7a01f_46d63901","line":23,"range":{"start_line":19,"start_character":0,"end_line":23,"end_character":45},"updated":"2021-02-04 15:29:51.000000000","message":"these lines are not necessary, they describe some changes between your patchsets","commit_id":"11bf25563fae6ea544919afa51388e82145fae1e"},{"author":{"_account_id":32137,"name":"Arieh Maron","email":"amaron@redhat.com","username":"amaron"},"change_message_id":"4e96d240dd3a1263213fd6c342814281a5cfe93d","unresolved":false,"context_lines":[{"line_number":16,"context_line":"that tests that a load balancer pool"},{"line_number":17,"context_line":"can recover from a load balancer failover"},{"line_number":18,"context_line":"* Merged _test_basic_traffic and _test_disturbed_traffic into a single method"},{"line_number":19,"context_line":"* fixed typo on line 207"},{"line_number":20,"context_line":"* fixed udp port clash on line 262"},{"line_number":21,"context_line":"* fixed typo on line 263"},{"line_number":22,"context_line":"* corrected the class name on line 263"},{"line_number":23,"context_line":"* corrected the invocation of the dsturb_func"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Change-Id: I4f6c16a164e08996581ba60a48ba7c480b5c4902"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"29a82d42_24e547a7","line":23,"range":{"start_line":19,"start_character":0,"end_line":23,"end_character":45},"in_reply_to":"bba7a01f_46d63901","updated":"2021-05-09 12:04:18.000000000","message":"OK, I will drop them in the next update","commit_id":"11bf25563fae6ea544919afa51388e82145fae1e"}],"octavia_tempest_plugin/tests/scenario/v2/test_traffic_ops.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"1d9dd236e4532a43f0cf5a8d170a3c6179a42c29","unresolved":true,"context_lines":[{"line_number":203,"context_line":"    def _test_disturbed_traffic("},{"line_number":204,"context_line":"            self, protocol, protocol_port, listener_id, pool_id,"},{"line_number":205,"context_line":"            persistent\u003dTrue, traffic_member_count\u003d2, source_port\u003dNone,"},{"line_number":206,"context_line":"            delay\u003dNone, disturb_func\u003dNone):"},{"line_number":207,"context_line":"        \"\"\"Tests sending traffic through a loadbalancer with a disturbance"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"        * Set up members on a loadbalancer."}],"source_content_type":"text/x-python","patch_set":3,"id":"90adad76_3bdb244b","line":206,"range":{"start_line":206,"start_character":24,"end_line":206,"end_character":41},"updated":"2021-01-20 21:12:47.000000000","message":"This looks like a required argument, since if None() is called below it will fail.\n\nActually, this looks exactly like _test_basic_traffic() above.  IMO it would be better to add the argument there to save duplication, then:\n\nif disturb_function:\n    disturb_function()\n    wait to stabilize...\n\nDon\u0027t know if others agree.","commit_id":"3a2923fb09b413fe6eaa754b3d2d8cf615a3b612"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"1d9dd236e4532a43f0cf5a8d170a3c6179a42c29","unresolved":true,"context_lines":[{"line_number":316,"context_line":"                self.api_version, \u00272.1\u0027):"},{"line_number":317,"context_line":"            raise self.skipException(\u0027UDP listener support is only available \u0027"},{"line_number":318,"context_line":"                                     \u0027in Octavia API version 2.1 or newer\u0027)"},{"line_number":319,"context_line":"        listener_id, pool_id \u003d self._listener_pool_create(const.UDP, 8080)"},{"line_number":320,"context_line":"        lb_failover \u003d load_balancer.loadbalance_client.failover_loadbalancer"},{"line_number":321,"context_line":"        self._test_disturbed_traffic(const.UDP, 8080, listener_id, pool_id,"},{"line_number":322,"context_line":"                                     disturb_func\u003dlb_failover)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ee8d43df_331d8951","line":319,"range":{"start_line":319,"start_character":69,"end_line":319,"end_character":73},"updated":"2021-01-20 21:12:47.000000000","message":"Should this be 8081 to make sure it\u0027s unique port?","commit_id":"3a2923fb09b413fe6eaa754b3d2d8cf615a3b612"},{"author":{"_account_id":32137,"name":"Arieh Maron","email":"amaron@redhat.com","username":"amaron"},"change_message_id":"bc2fe799101eb245a361c20672e0d9c64f91c640","unresolved":false,"context_lines":[{"line_number":316,"context_line":"                self.api_version, \u00272.1\u0027):"},{"line_number":317,"context_line":"            raise self.skipException(\u0027UDP listener support is only available \u0027"},{"line_number":318,"context_line":"                                     \u0027in Octavia API version 2.1 or newer\u0027)"},{"line_number":319,"context_line":"        listener_id, pool_id \u003d self._listener_pool_create(const.UDP, 8080)"},{"line_number":320,"context_line":"        lb_failover \u003d load_balancer.loadbalance_client.failover_loadbalancer"},{"line_number":321,"context_line":"        self._test_disturbed_traffic(const.UDP, 8080, listener_id, pool_id,"},{"line_number":322,"context_line":"                                     disturb_func\u003dlb_failover)"}],"source_content_type":"text/x-python","patch_set":3,"id":"b0ca6f4b_e40abb91","line":319,"range":{"start_line":319,"start_character":69,"end_line":319,"end_character":73},"in_reply_to":"ee8d43df_331d8951","updated":"2021-01-25 07:47:28.000000000","message":"Done","commit_id":"3a2923fb09b413fe6eaa754b3d2d8cf615a3b612"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"10aac428fdd3da2bfc3a11ba1110dd2bed084920","unresolved":true,"context_lines":[{"line_number":144,"context_line":"            self, protocol, protocol_port, listener_id, pool_id,"},{"line_number":145,"context_line":"            persistent\u003dTrue, traffic_member_count\u003d2, source_port\u003dNone,"},{"line_number":146,"context_line":"            delay\u003dNone, disturb_func\u003dNone,"},{"line_number":147,"context_line":"            d_args: list \u003d None, d_kwargs: dict \u003d None):"},{"line_number":148,"context_line":"        \"\"\"Tests sending traffic through a loadbalancer with a disturbance"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"        * Set up members on a loadbalancer."}],"source_content_type":"text/x-python","patch_set":10,"id":"0f4b585f_71872727","line":147,"range":{"start_line":147,"start_character":12,"end_line":147,"end_character":54},"updated":"2021-02-04 15:29:51.000000000","message":"not sure if typing is allowed here","commit_id":"11bf25563fae6ea544919afa51388e82145fae1e"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"10aac428fdd3da2bfc3a11ba1110dd2bed084920","unresolved":true,"context_lines":[{"line_number":257,"context_line":"    @testtools.skipIf(CONF.load_balancer.test_with_noop,"},{"line_number":258,"context_line":"                      \u0027Traffic tests will not work in noop mode.\u0027)"},{"line_number":259,"context_line":"    @decorators.idempotent_id(\u002763c25c45-b832-42b7-840a-ec803e2f201d\u0027)"},{"line_number":260,"context_line":"    def test_failover_udp_traffic(self):"},{"line_number":261,"context_line":"        if not self.mem_listener_client.is_version_supported("},{"line_number":262,"context_line":"                self.api_version, \u00272.1\u0027):"},{"line_number":263,"context_line":"            raise self.skipException(\u0027UDP listener support is only available \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"a69f0d01_6b1bea73","line":260,"range":{"start_line":260,"start_character":8,"end_line":260,"end_character":33},"updated":"2021-02-04 15:29:51.000000000","message":"one question about the test itself: why only UDP?\nwhy not a failover test with UDP listener in active-standby?","commit_id":"11bf25563fae6ea544919afa51388e82145fae1e"},{"author":{"_account_id":32137,"name":"Arieh Maron","email":"amaron@redhat.com","username":"amaron"},"change_message_id":"4e96d240dd3a1263213fd6c342814281a5cfe93d","unresolved":false,"context_lines":[{"line_number":257,"context_line":"    @testtools.skipIf(CONF.load_balancer.test_with_noop,"},{"line_number":258,"context_line":"                      \u0027Traffic tests will not work in noop mode.\u0027)"},{"line_number":259,"context_line":"    @decorators.idempotent_id(\u002763c25c45-b832-42b7-840a-ec803e2f201d\u0027)"},{"line_number":260,"context_line":"    def test_failover_udp_traffic(self):"},{"line_number":261,"context_line":"        if not self.mem_listener_client.is_version_supported("},{"line_number":262,"context_line":"                self.api_version, \u00272.1\u0027):"},{"line_number":263,"context_line":"            raise self.skipException(\u0027UDP listener support is only available \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"4e2f67ba_0b720fb9","line":260,"range":{"start_line":260,"start_character":8,"end_line":260,"end_character":33},"in_reply_to":"a69f0d01_6b1bea73","updated":"2021-05-09 12:04:18.000000000","message":"I thought that tests should be as atomic as possible and test 1 single feature. I can add that as an additional test","commit_id":"11bf25563fae6ea544919afa51388e82145fae1e"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"bb86d8d6c9a9cee7414d5d829ac7de000397c7e4","unresolved":true,"context_lines":[{"line_number":168,"context_line":"        self.addCleanup("},{"line_number":169,"context_line":"            self.mem_member_client.cleanup_member,"},{"line_number":170,"context_line":"            member1[const.ID], pool_id\u003dpool_id,"},{"line_number":171,"context_line":"            lb_client\u003dself.mem_lb_client, lb_id\u003dcls.lb_id)"},{"line_number":172,"context_line":"        waiters.wait_for_status("},{"line_number":173,"context_line":"            self.mem_lb_client.show_loadbalancer, cls.lb_id,"},{"line_number":174,"context_line":"            const.PROVISIONING_STATUS, const.ACTIVE,"}],"source_content_type":"text/x-python","patch_set":17,"id":"e3ce834e_b642ea03","line":171,"range":{"start_line":171,"start_character":48,"end_line":171,"end_character":51},"updated":"2021-05-06 08:25:50.000000000","message":"hm, that doesn\u0027t make sense, why cls?","commit_id":"750db84e2b210e64fbdc9c170af5df152478a099"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"6b087d77117adc4cefbb5d98a0e3ecf54f01105a","unresolved":true,"context_lines":[{"line_number":140,"context_line":"    def _test_basic_traffic("},{"line_number":141,"context_line":"            self, protocol, protocol_port, listener_id, pool_id,"},{"line_number":142,"context_line":"            persistent\u003dTrue, traffic_member_count\u003d2, source_port\u003dNone,"},{"line_number":143,"context_line":"            delay\u003dNone, disturb_func\u003dNone,"},{"line_number":144,"context_line":"            d_args: list \u003d None, d_kwargs: dict \u003d None):"},{"line_number":145,"context_line":"        \"\"\"Tests sending traffic through a loadbalancer with a disturbance"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"        * Set up members on a loadbalancer."}],"source_content_type":"text/x-python","patch_set":20,"id":"f687d327_a33f37d6","line":144,"range":{"start_line":143,"start_character":24,"end_line":144,"end_character":56},"updated":"2021-05-10 15:55:32.000000000","message":"These are not descriptive and we don\u0027t usually allow generic parameters like args and kwargs as they are not explicitly defining the test function. This can make it hard for future developers and people trying to troubleshoot tests.\n\nI would also argue that adding this complication to the \"test_BASIC_traffic\" method is counter to the intended purpose of this shared method. The addition code here implies that a non-standalone topology would be required for the test to be successful (standalone implies downtime is possible during a failover), which is not clear by adding it here. Maybe special test or helper method would be more appropriate for your scenario.","commit_id":"1f708addb01b7f2a3ad9de95affe3f95a8c74ea9"}]}
