Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14049 )
Change subject: IMPALA-8847: Ignore add partition events with empty partition list ...................................................................... Patch Set 3: (3 comments) Overall, looks good to me. I like moving EventProcessorUtils out. If not in this change, would be nice to create a JIRA to clean up test_event_processing.py. We can use EventProcessorUtils for insert events too. http://gerrit.cloudera.org:8080/#/c/14049/3/tests/custom_cluster/test_event_processing.py File tests/custom_cluster/test_event_processing.py: http://gerrit.cloudera.org:8080/#/c/14049/3/tests/custom_cluster/test_event_processing.py@181 PS3, Line 181: def wait_for_insert_event_processing(self, previous_event_id): I like that you moved out EventProcessorUtils. However, this file heavily uses the same utils. Maybe we can create another JIRA to clean up this file. Especially, tests for insert events can be modified to use EventProcessorUtils functions. http://gerrit.cloudera.org:8080/#/c/14049/3/tests/custom_cluster/test_event_processing.py@200 PS3, Line 200: get_event_processor_metrics Same as my above comment, this function can be removed here and modify this file to use EventProcessorUtils. http://gerrit.cloudera.org:8080/#/c/14049/3/tests/custom_cluster/test_event_processing.py@215 PS3, Line 215: get_last_synced_event_id Same as previous comment. -- To view, visit http://gerrit.cloudera.org:8080/14049 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I877ce6233934e7090cd18e497f748bc6479838cb Gerrit-Change-Number: 14049 Gerrit-PatchSet: 3 Gerrit-Owner: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Reviewer: Anurag Mantripragada <anu...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Comment-Date: Mon, 12 Aug 2019 21:30:03 +0000 Gerrit-HasComments: Yes
