[GitHub] [helix] junkaixue commented on a diff in pull request #2497: Add support for pluggable external CloudInfoProcessor and CustomizedCloudProperties

["junkaixue (via GitHub)" <gi...@apache.org>]

junkaixue commented on code in PR #2497:
URL: https://github.com/apache/helix/pull/2497#discussion_r1220913286


##########
helix-core/src/main/java/org/apache/helix/manager/zk/ParticipantManager.java:
##########
@@ -242,13 +242,17 @@ private CloudInstanceInformation getCloudInstanceInformation() {
         _helixManagerProperty.getHelixCloudProperty().getCloudInfoProcessorName();
     try {
       // fetch cloud instance information for the instance
-      String cloudInstanceInformationProcessorClassName = CLOUD_PROCESSOR_PATH_PREFIX
-          + _helixManagerProperty.getHelixCloudProperty().getCloudProvider().toLowerCase() + "."
-          + cloudInstanceInformationProcessorName;
+      String cloudInstanceInformationProcessorClassName =

Review Comment:
   I dont get this... Originally, user configured it as "AZURE" will natively mapping to Azure impl. Then if they are using "CUSTOMIZED" they should be able to use any class path. 
   
   I understand what you are trying to do. But the branching of logic should be at the place selecting type instead of using path to do the selection.



-- 
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: reviews-unsubscribe@helix.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org