chia7712 commented on code in PR #23785:
URL: https://github.com/apache/kafka/pull/23785#discussion_r4251304044
##########
core/src/test/scala/unit/kafka/server/KafkaApisTest.scala:
##########
@@ -343,6 +343,36 @@ class KafkaApisTest extends Logging {
assertTrue(configNames.contains(TopicConfig.MAX_DECOMPRESSED_MESSAGE_BYTES_CONFIG))
}
+ @Test
+ def testDescribeConfigsTopicIncludesInternalListConfigSetOnTopic(): Unit = {
Review Comment:
Should we use ClusterInstance to write an integration test that covers the
whole path? AlterConfigsIntegrationTest in clients-integration-tests already
does something similar.
##########
server/src/main/java/org/apache/kafka/server/ConfigHelperUtils.java:
##########
@@ -72,12 +72,17 @@ public static
DescribeConfigsResponseData.DescribeConfigsResult createResponseCo
// Cast from Map<String, ?> to Map<String, Object> to eliminate
wildcard types. Cached to avoid multiple calls.
@SuppressWarnings("unchecked")
Map<String, Object> nonInternalValues = (Map<String, Object>)
config.nonInternalValues();
+ Map<String, ?> values = config.values();
Stream<Entry<String, Object>> allEntries = Stream.concat(
config.originals().entrySet().stream()
.filter(entry -> entry.getValue() != null
&&
explicitlySetConfigs.contains(entry.getKey())
&&
!nonInternalValues.containsKey(entry.getKey()))
- .map(entry -> Map.entry(entry.getKey(),
entry.getValue())),
+ // Use the parsed value for internal configs, since
createConfigEntry expects values of the
Review Comment:
Sorry for my bad comment. I missed the raw string configs from cluster-wide
and prefixed configs. I have opened
https://issues.apache.org/jira/browse/KAFKA-21269 to add tests so we don’t miss
it in the future.
Would you please revert it :)
--
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]
