chia7712 commented on code in PR #15861:
URL: https://github.com/apache/kafka/pull/15861#discussion_r1590457683
##########
core/src/test/java/kafka/test/annotation/ClusterTest.java:
##########
@@ -44,4 +44,5 @@
String listener() default "";
MetadataVersion metadataVersion() default MetadataVersion.IBP_3_8_IV0;
ClusterConfigProperty[] serverProperties() default {};
+ Tags[] tags() default {};
Review Comment:
Another idea. Maybe we don't need to have new annotation. We just have a new
field `String[] labels() default {}` to enable developers to add custom strings
to display name. That is more simple. WDYT?
##########
core/src/test/java/kafka/test/annotation/ClusterTest.java:
##########
@@ -44,4 +44,5 @@
String listener() default "";
MetadataVersion metadataVersion() default MetadataVersion.IBP_3_8_IV0;
ClusterConfigProperty[] serverProperties() default {};
+ Tags[] tags() default {};
Review Comment:
Could you please add comment for it
##########
core/src/test/java/kafka/test/junit/ClusterTestExtensions.java:
##########
@@ -143,23 +144,27 @@ private void processClusterTest(ExtensionContext context,
ClusterTest annot, Clu
Type type = annot.clusterType() == Type.DEFAULT ?
defaults.clusterType() : annot.clusterType();
Map<String, String> serverProperties = new HashMap<>();
+ Map<String, String> tags = new HashMap<>();
for (ClusterConfigProperty property : defaults.serverProperties()) {
serverProperties.put(property.key(), property.value());
}
for (ClusterConfigProperty property : annot.serverProperties()) {
serverProperties.put(property.key(), property.value());
}
+ for (Tags tag: annot.tags()) {
+ tags.put(tag.key(), tag.value());
+ }
ClusterConfig config = ClusterConfig.builder()
.setType(type)
.setBrokers(annot.brokers() == 0 ? defaults.brokers() :
annot.brokers())
.setControllers(annot.controllers() == 0 ?
defaults.controllers() : annot.controllers())
.setDisksPerBroker(annot.disksPerBroker() == 0 ?
defaults.disksPerBroker() : annot.disksPerBroker())
.setAutoStart(annot.autoStart() == AutoStart.DEFAULT ?
defaults.autoStart() : annot.autoStart() == AutoStart.YES)
- .setName(annot.name().trim().isEmpty() ? null : annot.name())
.setListenerName(annot.listener().trim().isEmpty() ? null :
annot.listener())
.setServerProperties(serverProperties)
.setSecurityProtocol(annot.securityProtocol())
.setMetadataVersion(annot.metadataVersion())
+ .setTags(tags)
Review Comment:
Maybe lambda is more simple. For example:
`setTags(Arrays.stream(annot.tags()).collect(Collectors.toMap(Tags::key,
Tags::value)))`
##########
core/src/test/java/kafka/test/ClusterConfig.java:
##########
@@ -83,6 +81,7 @@ private ClusterConfig(Type type, int brokers, int
controllers, int disksPerBroke
this.saslServerProperties =
Objects.requireNonNull(saslServerProperties);
this.saslClientProperties =
Objects.requireNonNull(saslClientProperties);
this.perBrokerOverrideProperties =
Objects.requireNonNull(perBrokerOverrideProperties);
+ this.tags = Objects.requireNonNull(extendTags(tags));
Review Comment:
We should keep the origin tags and return the "modified" tags in `nameTags`.
Also, please add a method (`tags`) to return origin tags.
--
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
