Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13019 )
Change subject: IMPALA-8419 : Validate event processing related configurations ...................................................................... Patch Set 11: Code-Review+1 (2 comments) Will let Vihang take a look at it, too and I can promote it to +2. http://gerrit.cloudera.org:8080/#/c/13019/11/fe/src/main/java/org/apache/impala/catalog/events/EventProcessorValidator.java File fe/src/main/java/org/apache/impala/catalog/events/EventProcessorValidator.java: http://gerrit.cloudera.org:8080/#/c/13019/11/fe/src/main/java/org/apache/impala/catalog/events/EventProcessorValidator.java@182 PS11, Line 182: class ValidationResult Use static class http://gerrit.cloudera.org:8080/#/c/13019/9/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java: http://gerrit.cloudera.org:8080/#/c/13019/9/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@290 PS9, Line 290: } : } : : /** : * Test that when HiveConf.METASTORE_PARAMETER_EXCLUDE_PATTERNS contains a regex : * that filters out any parameter required for event processing, the config : * validation fails. Config validation should succeed when the regex does not match : * any of the required parameters. : */ : @Test : public void testParameterFilterValidation() throws TException { : MetastoreEventsProcessor mockEventsProcessor = : Mockito.mock(MetastoreEventsProcessor.class); : : //Regex to filter all parameters starting with impala : when(mockEventsProcessor.getConfigValueFromMetastore( : METASTORE_PARAMETER_EXCLUDE_PATTERNS, : DEFAULT_METASTORE_CONFIG_VALUE)).thenReturn("^impala"); : ValidationResult testResult = : validateMetastoreEventParameters(mockEventsProcessor); : assertFalse(testResult.isValid()); : assertTrue(testResult.getReason().isPresent()); : : when(mockEventsProcessor.getConfigValueFromMetastore( : METASTORE_PARAMETER_EXCLUDE_PATTERNS, : DEFAULT_METASTORE_CONFIG_VALUE)).thenReturn("impala*"); : testResult = : validateMetastoreEventParameters(mockEventsProcessor); : assertFalse(testResult.isValid()); : assertTrue(testResult.getReason().isPresent()); : : when(mockEventsProcessor.getConfigValueFromMetastore( : METASTORE_PARAMETER_EXCLUDE_PATTERNS, : DEFAULT_METASTORE_CONFIG_VALUE)).thenReturn(""); : testResult = : validateMetastoreEventParameters(mockEventsProcessor); : assertTrue(testResult.isValid()); : assertFalse(testResult.getReason().isPresent()); : : // Test with default return value : when(mockEventsProcessor.getConfigValueFromMetastore( : METASTORE_PARAMETER_EXCLUDE_PATTERNS, : DEFAULT_METASTORE_CONFIG_VALUE)) : .thenReturn(DEFAULT_METASTORE_CONFIG_VALUE); : testResult = : validateMetastoreEventParameters(mockEventsProcessor); : assertTrue(testResult.isValid()); : assertFalse(testResult.getReason().isPresent()); : : when(mockEventsProcessor.getConfigValueFromMetastore( : METASTORE_PARAMETER_EXCLUDE_PATTERNS, : DEFAULT_METASTORE_CONFIG_VALUE)).thenReturn("randomString1, impala" : + ".disableHmsSync, randomString2"); : testResult = : validateMetastoreEventParameters(mockEventsProcessor); : assertFalse(testResult.isValid()); : assertTrue(testResult.getReason().isPresent()); : : //Test when a required parameter is given as regex : String requiredParameter = MetastoreEventPropertyKey.CATALOG_SERVICE_ID.getKey(); : when(mockEventsProcessor.getConfigValueFromMetastore( : METASTORE_PARAMETER_EXCLUDE_PATTERNS, DEFAULT_METASTORE_CONFIG_VALUE)) : .thenReturn(requiredParameter); : testResult = > I don't feel we should add checks for all the tests. I am adding a check fo Fair enough. -- To view, visit http://gerrit.cloudera.org:8080/13019 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I94c2783e36287a65122003aa55d8075a806bc606 Gerrit-Change-Number: 13019 Gerrit-PatchSet: 11 Gerrit-Owner: Bharath Krishna <bhar...@cloudera.com> Gerrit-Reviewer: Anurag Mantripragada <anu...@cloudera.com> Gerrit-Reviewer: Bharath Krishna <bhar...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Comment-Date: Mon, 29 Apr 2019 17:48:59 +0000 Gerrit-HasComments: Yes
