You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/08/20 12:51:32 UTC
[GitHub] [pulsar] lhotari commented on a change in pull request #11728: added an option to configure messageRouterClassName for MessageRoutingMode.CustomPartition
lhotari commented on a change in pull request #11728:
URL: https://github.com/apache/pulsar/pull/11728#discussion_r692920297
##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/PartitionedProducerImpl.java
##########
@@ -93,6 +98,19 @@ private MessageRouter getMessageRouter() {
switch (messageRouteMode) {
case CustomPartition:
+ String messageRouterClassName = conf.getMessageRouterClassName();
+ if (isNotBlank(messageRouterClassName)) {
+ Class<?> clazz;
+ try {
+ clazz = Class.forName(messageRouterClassName);
+ messageRouter = (MessageRouter) clazz.getDeclaredConstructor().newInstance();
+ conf.setCustomMessageRouter(messageRouter);
+ } catch (ClassNotFoundException | NoSuchMethodException | InvocationTargetException |
+ InstantiationException | IllegalAccessException | ClassCastException e) {
+ log.error("[{}] Error occurred while instantiating the messageRouterClassName",
Review comment:
It might not be easy to resolve the above suggestion since PulsarClientException is a checked exception and the the current methods don't current throw PulsarClientException. I'm not sure what wrapper exception to use. Perhaps you can check what would make sense.
##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/PartitionedProducerImpl.java
##########
@@ -93,6 +98,19 @@ private MessageRouter getMessageRouter() {
switch (messageRouteMode) {
case CustomPartition:
+ String messageRouterClassName = conf.getMessageRouterClassName();
+ if (isNotBlank(messageRouterClassName)) {
+ Class<?> clazz;
+ try {
+ clazz = Class.forName(messageRouterClassName);
+ messageRouter = (MessageRouter) clazz.getDeclaredConstructor().newInstance();
+ conf.setCustomMessageRouter(messageRouter);
+ } catch (ClassNotFoundException | NoSuchMethodException | InvocationTargetException |
+ InstantiationException | IllegalAccessException | ClassCastException e) {
+ log.error("[{}] Error occurred while instantiating the messageRouterClassName",
Review comment:
I believe that it's better to throw an exception rather than logging the exception. Please removing logging and throw the exception. It makes sense to wrap the original exception in PulsarClientException (with message "Error occurred while instantiating the messageRouterClassName " + messageRouterClassName and the original exception as the cause)
--
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