0xffff-zhiyan commented on code in PR #21053:
URL: https://github.com/apache/kafka/pull/21053#discussion_r2956273974
##########
metadata/src/test/java/org/apache/kafka/image/loader/MetadataLoaderTest.java:
##########
@@ -924,4 +927,76 @@ public void onMetadataUpdate(MetadataDelta delta,
MetadataImage newImage, Loader
}
faultHandler.maybeRethrowFirstException();
}
+
+ @Test
+ public void testUnsupportedConfigFilteredInCommit() throws Exception {
+ // Create a checker that rejects "message.format.version"
+ SupportedConfigChecker checker = (type, name) ->
+ !name.equals("message.format.version");
Review Comment:
`DefaultSupportedConfigChecker` lives in the server module, while
`MetadataLoaderTest `is in the metadata module. Using
`DefaultSupportedConfigChecker` in metadata tests would create a circular
dependency. So we can't use it
Also, I've updated `DefaultSupportedConfigChecker` to fix some other test
failures caused by the BROKER whitelist. This is because valid BROKER configs
include listener-specific prefixed configs and plugin-defined configs like
`listener.name.<name>.ssl.keystore.location` and custom authorizer configs or
quota callback configs that are dynamically named and cannot be pre-enumerated
in `DynamicConfig.Broker.names()`.
I changed BROKER configs to use a sentinel` ALLOW_ALL` set (whose contains()
always returns true). And since no dynamic BROKER configs were deprecated in
the 3.x → 4.x upgrade, I think there is nothing to filter for BROKER. WDYT?
##########
metadata/src/test/java/org/apache/kafka/image/loader/MetadataLoaderTest.java:
##########
@@ -924,4 +927,76 @@ public void onMetadataUpdate(MetadataDelta delta,
MetadataImage newImage, Loader
}
faultHandler.maybeRethrowFirstException();
}
+
+ @Test
+ public void testUnsupportedConfigFilteredInCommit() throws Exception {
+ // Create a checker that rejects "message.format.version"
+ SupportedConfigChecker checker = (type, name) ->
+ !name.equals("message.format.version");
Review Comment:
`DefaultSupportedConfigChecker` lives in the server module, while
`MetadataLoaderTest `is in the metadata module. Using
`DefaultSupportedConfigChecker` in metadata tests would create a circular
dependency so we can't use it
Also, I've updated `DefaultSupportedConfigChecker` to fix some other test
failures caused by the BROKER whitelist. This is because valid BROKER configs
include listener-specific prefixed configs and plugin-defined configs like
`listener.name.<name>.ssl.keystore.location` and custom authorizer configs or
quota callback configs that are dynamically named and cannot be pre-enumerated
in `DynamicConfig.Broker.names()`.
I changed BROKER configs to use a sentinel` ALLOW_ALL` set (whose contains()
always returns true). And since no dynamic BROKER configs were deprecated in
the 3.x → 4.x upgrade, I think there is nothing to filter for BROKER. WDYT?
--
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]
