[GitHub] [incubator-doris] wuyunfeng commented on a change in pull request #5325: [Doris On ES][WIP] Support external ES table with `SSL` secured and configurable node sniffing

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

wuyunfeng commented on a change in pull request #5325:
URL: https://github.com/apache/incubator-doris/pull/5325#discussion_r586145816



##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/EsUtil.java
##########
@@ -82,4 +85,13 @@ public static JSONObject getJsonObject(JSONObject jsonObject, String key, int fr
             return null;
         }
     }
+
+    public static boolean getBooleanProperty(Map<String, String> properties, String name) throws DdlException {

Review comment:
       ```suggestion
       public static boolean get(Map<String, String> properties, String name) throws DdlException {
   ```
   ```suggestion
       public static boolean getBooleanProperty(Map<String, String> properties, String name) throws DdlException {
   ```

##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/PartitionPhase.java
##########
@@ -37,7 +38,16 @@ public PartitionPhase(EsRestClient client) {
     @Override
     public void execute(SearchContext context) throws DorisEsException {
         shardPartitions = client.searchShards(context.sourceIndex());
-        nodesInfo = client.getHttpNodes();
+        if (context.esNodesDiscovery()) {
+            nodesInfo = client.getHttpNodes();
+        } else {
+            nodesInfo = new HashMap<>();
+            String[] seeds = context.esTable().getSeeds();
+            boolean useSSL = context.esTable().isUseSslClient();
+            for (int i = 0; i < seeds.length; i++) {
+                nodesInfo.put(String.valueOf(i), new EsNodeInfo(String.valueOf(i), seeds[i], useSSL));

Review comment:
       it seems `useSSL`  is useless for `nodesInfo `?

##########
File path: fe/fe-core/src/main/java/org/apache/doris/external/elasticsearch/EsUtil.java
##########
@@ -21,43 +21,46 @@
 import org.apache.doris.analysis.PartitionDesc;
 import org.apache.doris.analysis.RangePartitionDesc;
 import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.DdlException;
 import org.json.JSONObject;
 
+import java.util.Map;
+
 public class EsUtil {
-    
+
     public static void analyzePartitionAndDistributionDesc(PartitionDesc partitionDesc,
-            DistributionDesc distributionDesc) throws AnalysisException {
+                                                           DistributionDesc distributionDesc) throws AnalysisException {
         if (partitionDesc == null && distributionDesc == null) {
             return;
         }
-        
+

Review comment:
       why add blank line on those if statement?




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org