From dbd10d486e764088a984094c0517dc15924d0170 Mon Sep 17 00:00:00 2001 From: Chia-Ping Tsai Date: Mon, 23 Mar 2020 20:04:10 +0800 Subject: [PATCH 1/2] Minor: remove useless log messages from connector configs validation --- .../apache/kafka/connect/runtime/AbstractHerder.java | 7 ++++++- .../java/org/apache/kafka/connect/runtime/Herder.java | 10 ++++++++++ .../rest/resources/ConnectorPluginsResource.java | 3 ++- .../rest/resources/ConnectorPluginsResourceTest.java | 10 +++++----- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java index 8d8934bcf80ab..a4476f871edb9 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java @@ -300,6 +300,11 @@ protected Map validateBasicConnectorConfig(Connector connec @Override public ConfigInfos validateConnectorConfig(Map connectorProps) { + return validateConnectorConfig(connectorProps, true); + } + + @Override + public ConfigInfos validateConnectorConfig(Map connectorProps, boolean doLog) { if (worker.configTransformer() != null) { connectorProps = worker.configTransformer().transform(connectorProps); } @@ -354,7 +359,7 @@ public ConfigInfos validateConnectorConfig(Map connectorProps) { configValues.addAll(config.configValues()); ConfigInfos configInfos = generateResult(connType, configKeys, configValues, new ArrayList<>(allGroups)); - AbstractConfig connectorConfig = new AbstractConfig(new ConfigDef(), connectorProps); + AbstractConfig connectorConfig = new AbstractConfig(new ConfigDef(), connectorProps, doLog); String connName = connectorProps.get(ConnectorConfig.NAME_CONFIG); ConfigInfos producerConfigInfos = null; ConfigInfos consumerConfigInfos = null; diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java index 6cb1a475b88f1..86c4d049d00a1 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java @@ -180,6 +180,16 @@ public interface Herder { */ ConfigInfos validateConnectorConfig(Map connectorConfig); + /** + * Validate the provided connector config values against the configuration definition. + * @param connectorConfig the provided connector config values + * @param doLog set it to true to log all connectorConfig with INFO level. false to log nothing. + * Noted: there are many endpoints requiring this method but not all configs are worth being logged. + */ + default ConfigInfos validateConnectorConfig(Map connectorConfig, boolean doLog) { + return validateConnectorConfig(connectorConfig); + } + /** * Restart the task with the given id. * @param id id of the task diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java index 24eb93b8c0d5f..d648369b4ea56 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java @@ -78,7 +78,8 @@ public ConfigInfos validateConfigs( ); } - return herder.validateConnectorConfig(connectorConfig); + // the validated configs don't need to be logger. + return herder.validateConnectorConfig(connectorConfig, false); } @GET diff --git a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResourceTest.java b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResourceTest.java index 2882355eccbec..72ef29f6212d9 100644 --- a/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResourceTest.java +++ b/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResourceTest.java @@ -198,7 +198,7 @@ private void expectPlugins() { @Test public void testValidateConfigWithSingleErrorDueToMissingConnectorClassname() throws Throwable { - herder.validateConnectorConfig(EasyMock.eq(partialProps)); + herder.validateConnectorConfig(EasyMock.eq(partialProps), EasyMock.anyBoolean()); PowerMock.expectLastCall().andAnswer((IAnswer) () -> { ConfigDef connectorConfigDef = ConnectorConfig.configDef(); @@ -243,7 +243,7 @@ public void testValidateConfigWithSingleErrorDueToMissingConnectorClassname() th @Test public void testValidateConfigWithSimpleName() throws Throwable { - herder.validateConnectorConfig(EasyMock.eq(props)); + herder.validateConnectorConfig(EasyMock.eq(props), EasyMock.anyBoolean()); PowerMock.expectLastCall().andAnswer((IAnswer) () -> { ConfigDef connectorConfigDef = ConnectorConfig.configDef(); @@ -284,7 +284,7 @@ public void testValidateConfigWithSimpleName() throws Throwable { @Test public void testValidateConfigWithAlias() throws Throwable { - herder.validateConnectorConfig(EasyMock.eq(props)); + herder.validateConnectorConfig(EasyMock.eq(props), EasyMock.anyBoolean()); PowerMock.expectLastCall().andAnswer((IAnswer) () -> { ConfigDef connectorConfigDef = ConnectorConfig.configDef(); @@ -325,7 +325,7 @@ public void testValidateConfigWithAlias() throws Throwable { @Test(expected = BadRequestException.class) public void testValidateConfigWithNonExistentName() throws Throwable { - herder.validateConnectorConfig(EasyMock.eq(props)); + herder.validateConnectorConfig(EasyMock.eq(props), EasyMock.anyBoolean()); PowerMock.expectLastCall().andAnswer((IAnswer) () -> { ConfigDef connectorConfigDef = ConnectorConfig.configDef(); @@ -362,7 +362,7 @@ public void testValidateConfigWithNonExistentName() throws Throwable { @Test(expected = BadRequestException.class) public void testValidateConfigWithNonExistentAlias() throws Throwable { - herder.validateConnectorConfig(EasyMock.eq(props)); + herder.validateConnectorConfig(EasyMock.eq(props), EasyMock.anyBoolean()); PowerMock.expectLastCall().andAnswer((IAnswer) () -> { ConfigDef connectorConfigDef = ConnectorConfig.configDef(); From c60d83e6589c034cb0079b9d6d10233acbcbfe70 Mon Sep 17 00:00:00 2001 From: Konstantine Karantasis Date: Tue, 24 Mar 2020 16:28:17 -0700 Subject: [PATCH 2/2] MINOR: Slight rewording and fixing typos --- .../main/java/org/apache/kafka/connect/runtime/Herder.java | 4 ++-- .../runtime/rest/resources/ConnectorPluginsResource.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java index 86c4d049d00a1..cbd7f3ff9f244 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java @@ -183,8 +183,8 @@ public interface Herder { /** * Validate the provided connector config values against the configuration definition. * @param connectorConfig the provided connector config values - * @param doLog set it to true to log all connectorConfig with INFO level. false to log nothing. - * Noted: there are many endpoints requiring this method but not all configs are worth being logged. + * @param doLog if true log all the connector configurations at INFO level; if false, no connector configurations are logged. + * Note that logging of configuration is not necessary in every endpoint that uses this method. */ default ConfigInfos validateConnectorConfig(Map connectorConfig, boolean doLog) { return validateConnectorConfig(connectorConfig); diff --git a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java index d648369b4ea56..fe8e73e1312b3 100644 --- a/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java +++ b/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java @@ -78,7 +78,7 @@ public ConfigInfos validateConfigs( ); } - // the validated configs don't need to be logger. + // the validated configs don't need to be logged return herder.validateConnectorConfig(connectorConfig, false); }