kirktrue commented on code in PR #15188:
URL: https://github.com/apache/kafka/pull/15188#discussion_r1879194394
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/LegacyKafkaConsumer.java:
##########
@@ -495,6 +496,16 @@ public void subscribe(Pattern pattern) {
subscribeInternal(pattern, Optional.empty());
}
+ @Override
+ public void subscribe(SubscriptionPattern pattern,
ConsumerRebalanceListener callback) {
+ throw new IllegalArgumentException("Operation not supported in the
classic group protocol");
Review Comment:
There was some discussion about implementing these APIs in the
`ClassicKafkaConsumer`. IIUC, re2j patterns are a strict subset of those used
by the bundled Java libraries in `java.util.regex`. CMIIW, though,
@AndrewJSchofield.
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java:
##########
@@ -1757,6 +1771,20 @@ private void subscribeInternal(Pattern pattern,
Optional<ConsumerRebalanceListen
}
}
+ private void subscribeInternal(SubscriptionPattern pattern,
Optional<ConsumerRebalanceListener> listener) {
+ acquireAndEnsureOpen();
+ try {
+ maybeThrowInvalidGroupIdException();
+ if (pattern == null) {
+ throw new IllegalArgumentException("Topic pattern to subscribe
to cannot be null");
+ }
+ log.info("Subscribed to pattern: '{}'", pattern.pattern());
+ subscriptions.subscribe(pattern, listener);
Review Comment:
We've made a lot of effort to move any mutation of the `SubscriptionState`
object to the background thread. Is there any reason we should not follow the
same pattern (pun intended) here? If there's a good reason not to, we
definitely should add some documentary comments to that effect.
--
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
