Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MINOR: Add method to Herder to control logging of connector configs during validation #8263

Merged
merged 2 commits into from
Mar 24, 2020
Merged

MINOR: Add method to Herder to control logging of connector configs during validation #8263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dbd10d4
Minor: remove useless log messages from connector configs validation
chia7712 Mar 23, 2020
c60d83e
MINOR: Slight rewording and fixing typos
kkonstantine Mar 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,11 @@ protected Map<String, ConfigValue> validateBasicConnectorConfig(Connector connec

@Override
public ConfigInfos validateConnectorConfig(Map<String, String> connectorProps) {
return validateConnectorConfig(connectorProps, true);
}

@Override
public ConfigInfos validateConnectorConfig(Map<String, String> connectorProps, boolean doLog) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to the other comment, we'll need to keep the @Override of the previous method, and just have it call the new method that will also be overridden.

if (worker.configTransformer() != null) {
connectorProps = worker.configTransformer().transform(connectorProps);
}
Expand Down Expand Up @@ -354,7 +359,7 @@ public ConfigInfos validateConnectorConfig(Map<String, String> 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;
Expand Down
10 changes: 10 additions & 0 deletions connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,16 @@ public interface Herder {
*/
ConfigInfos validateConnectorConfig(Map<String, String> connectorConfig);

/**
* Validate the provided connector config values against the configuration definition.
* @param connectorConfig the provided connector config values
* @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<String, String> connectorConfig, boolean doLog) {
return validateConnectorConfig(connectorConfig);
}

/**
* Restart the task with the given id.
* @param id id of the task
Expand Down
3 changes: 2 additions & 1 deletion ...c/main/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ public ConfigInfos validateConfigs(
);
}

return herder.validateConnectorConfig(connectorConfig);
// the validated configs don't need to be logged
return herder.validateConnectorConfig(connectorConfig, false);
}

@GET
Expand Down
10 changes: 5 additions & 5 deletions ...st/java/org/apache/kafka/connect/runtime/rest/resources/ConnectorPluginsResourceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<ConfigInfos>) () -> {
ConfigDef connectorConfigDef = ConnectorConfig.configDef();
Expand Down Expand Up @@ -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<ConfigInfos>) () -> {
ConfigDef connectorConfigDef = ConnectorConfig.configDef();
Expand Down Expand Up @@ -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<ConfigInfos>) () -> {
ConfigDef connectorConfigDef = ConnectorConfig.configDef();
Expand Down Expand Up @@ -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<ConfigInfos>) () -> {
ConfigDef connectorConfigDef = ConnectorConfig.configDef();
Expand Down Expand Up @@ -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<ConfigInfos>) () -> {
ConfigDef connectorConfigDef = ConnectorConfig.configDef();
Expand Down