C0urante commented on code in PR #13144: URL: https://github.com/apache/kafka/pull/13144#discussion_r1084433485
########## connect/runtime/src/test/java/org/apache/kafka/connect/connector/policy/BaseConnectorClientConfigOverridePolicyTest.java: ########## @@ -27,7 +27,7 @@ public abstract class BaseConnectorClientConfigOverridePolicyTest { - protected abstract ConnectorClientConfigOverridePolicy policyToTest(); Review Comment: Good eye! ########## connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedHerderTest.java: ########## @@ -227,9 +227,14 @@ public class DistributedHerderTest { private SinkConnectorConfig conn1SinkConfig; private SinkConnectorConfig conn1SinkConfigUpdated; private short connectProtocolVersion; + private boolean connectorClientConfigOverridePolicyClosed = false; private final ConnectorClientConfigOverridePolicy - noneConnectorClientConfigOverridePolicy = new NoneConnectorClientConfigOverridePolicy(); - + noneConnectorClientConfigOverridePolicy = new NoneConnectorClientConfigOverridePolicy() { + @Override + public void close() { + connectorClientConfigOverridePolicyClosed = true; + } + }; Review Comment: I like this testing strategy overall, but I think this logic can go into a dedicated class, similar to the [SampleSinkConnector class](https://github.com/apache/kafka/blob/00e5803cd3af89011254e734232308618403309d/connect/runtime/src/test/java/org/apache/kafka/connect/runtime/SampleSinkConnector.java) that we use for unit tests that require a sink connector. The `SampleConnectorClientConfigOverridePolicy` class (feel free to rename) could then expose a public `boolean isClosed()` method, which would replace the `connectorClientConfigOverridePolicyClosed` field here. ########## connect/runtime/src/main/java/org/apache/kafka/connect/runtime/AbstractHerder.java: ########## @@ -151,6 +151,11 @@ protected void stopServices() { this.configBackingStore.stop(); this.worker.stop(); this.connectorExecutor.shutdown(); + try { + this.connectorClientConfigOverridePolicy.close(); + } catch (Exception e) { + log.warn("Exception while stop connectorClientConfigOverridePolicy:", e); + } Review Comment: This can be a one-liner with the `Utils` class: ```suggestion Utils.closeQuietly(this.connectorClientConfigOverridePolicy, "connector client config override policy"); ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org