snehashisp commented on code in PR #17743:
URL: https://github.com/apache/kafka/pull/17743#discussion_r1966272861
##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/isolation/PluginsTest.java:
##########
@@ -197,14 +196,7 @@ public void
shouldInstantiateAndConfigureDefaultHeaderConverter() {
props.remove(WorkerConfig.HEADER_CONVERTER_CLASS_CONFIG);
createConfig();
- // Because it's not explicitly set on the supplied configuration, the
logic to use the current classloader for the connector
- // will exit immediately, and so this method always returns null
HeaderConverter headerConverter = plugins.newHeaderConverter(config,
-
WorkerConfig.HEADER_CONVERTER_CLASS_CONFIG,
-
ClassLoaderUsage.CURRENT_CLASSLOADER);
- assertNull(headerConverter);
- // But we should always find it (or the worker's default) when using
the plugins classloader ...
- headerConverter = plugins.newHeaderConverter(config,
Review Comment:
This change is related to the one above. This part seems to test the fact
that we are not allowing the headerConverter to be instantiated from the a
worker config via CURRENT classloader unless its explicitly present in configs
(this check
[here](https://github.com/apache/kafka/blob/66e2ac0e098f6ee83deaf5501bf3f606d29b35fa/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/Plugins.java#L440)).
Not exactly sure why, but since we were always using PLUGINS loader with
worker configs this path was never encountered in actual operation. With the
changes I mentioned in the above comment, I needed the defaults for header
converter to be initialized even with CURRENT classloader.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
