BewareMyPower commented on code in PR #16969:
URL: https://github.com/apache/pulsar/pull/16969#discussion_r950047034
##########
pulsar-client-cpp/lib/MultiTopicsConsumerImpl.cc:
##########
@@ -52,6 +53,24 @@
MultiTopicsConsumerImpl::MultiTopicsConsumerImpl(ClientImplPtr client, const std
} else {
unAckedMessageTrackerPtr_.reset(new UnAckedMessageTrackerDisabled());
}
+ auto partitionsUpdateInterval = static_cast<unsigned
int>(client_->conf().getPartitionsUpdateInterval());
+ if (partitionsUpdateInterval > 0) {
+ listenerExecutor_ = client_->getListenerExecutorProvider()->get();
+ partitionsUpdateTimer_ = listenerExecutor_->createDeadlineTimer();
+ partitionsUpdateInterval_ =
boost::posix_time::seconds(partitionsUpdateInterval);
+ lookupServicePtr_ = client_->getLookup();
+ }
+}
+
+ConsumerImplBasePtr MultiTopicsConsumerImpl::createPartitionedConsumer(
+ ClientImplPtr client, const std::string singleTopic, const int
numPartitions,
+ const std::string& subscriptionName, TopicNamePtr topicName, const
ConsumerConfiguration& conf,
+ const LookupServicePtr lookupServicePtr) {
+ MultiTopicsConsumerImpl* partitionedConsumer = new MultiTopicsConsumerImpl(
+ client, {singleTopic}, subscriptionName, topicName, conf,
lookupServicePtr);
+ partitionedConsumer->topicsPartitions_.emplace(singleTopic, numPartitions);
+ std::shared_ptr<MultiTopicsConsumerImpl>
partitionedConsumerPtr(partitionedConsumer);
+ return partitionedConsumerPtr;
}
Review Comment:
Adding a static method that returns a shared pointer of
`MultiTopicsConsumerImpl` here looks weird and inconsistent. I found it's
suggested by the comment here:
https://github.com/apache/pulsar/pull/16969#discussion_r939897369.
However, we can add another overload constructor like:
```c++
MultiTopicsConsumerImpl(ClientImplPtr client, const std::string&
singleTopic, const int numPartitions,
const std::string& subscriptionName,
TopicNamePtr topicName,
const ConsumerConfiguration& conf, const
LookupServicePtr lookupServicePtr)
: MultiTopicsConsumerImpl(client, {singleTopic}, subscriptionName,
topicName, conf,
lookupServicePtr) {
topicsPartitions_[singleTopic] = numPartitions;
}
```
It's more simple than the current code. /cc @RobertIndie
BTW, since the implementation of the overload constructor is very simple
(only 1 line except calling another constructor), we can write it in the header
file directly.
--
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]
