[GitHub] [pulsar] rdhabalia commented on a change in pull request #12235: [pulsar-broker] PIP-100 Support pluggable topic factory

[GitBox <gi...@apache.org>]

rdhabalia commented on a change in pull request #12235:
URL: https://github.com/apache/pulsar/pull/12235#discussion_r803042222



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
##########
@@ -2809,6 +2817,35 @@ public long getPausedConnections() {
         return pausedConnections.longValue();
     }
 
+    @SuppressWarnings("unchecked")
+    private <T extends Topic> T newTopic(String topic, ManagedLedger ledger, BrokerService brokerService,
+            Class<T> topicClazz) {
+        if (topicFactory != null) {
+            try {
+                Topic newTopic = topicFactory.create(topic, ledger, brokerService, topicClazz);
+                if (newTopic != null) {
+                    return (T) newTopic;
+                }
+            } catch (Throwable e) {
+                log.warn("Failed to create persistent topic using factory {}, {}", topic, e.getMessage());

Review comment:
       actually handling the exception and falling back to default implementation strategy was intentional to avoid unavailability of topic. however, I have updated it to throw exception for now so, we can merge this PR as it's open for almost 6 months now.




-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org