zhangbutao commented on code in PR #5992:
URL: https://github.com/apache/hive/pull/5992#discussion_r2230260056


##########
itests/test-jdbc/src/test/resources/custom_hosts_file:
##########
@@ -0,0 +1,3 @@
+127.0.0.1       localhost
+127.0.0.1       test-standalone-jdbc-plain
+127.0.0.1       test-standalone-jdbc-kerberos

Review Comment:
   Does the JDBC JAR relate to the authentication mode? If there is no 
relationship, can we keep only the simplest plain mode to simplify the code?



##########
itests/test-jdbc/src/test/java/org/apache/hive/jdbc/TestStandaloneJdbc.java:
##########
@@ -0,0 +1,226 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hive.jdbc;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.lang.reflect.Field;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import org.apache.hadoop.hive.metastore.utils.SecurityUtils;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class TestStandaloneJdbc {
+  private static File workDir = new File(createTempDir(), 
"test-it-standalone-jdbc");
+  private static ItAbstractContainer HS2;
+  private static ItAbstractContainer ZK_HS2;
+
+  public static File createTempDir() {
+    File baseDir = new File(System.getProperty("java.io.tmpdir"));
+    String baseName = System.currentTimeMillis() + "-";
+
+    for (int counter = 0; counter < 100; counter++) {
+      File tempDir = new File(baseDir, baseName + counter);
+      if (tempDir.mkdir()) {
+        return tempDir;
+      }
+    }
+    throw new IllegalStateException(
+        "Failed to create directory within 100 attempts (tried "
+            + baseName
+            + "0 to "
+            + baseName
+            + 99
+            + ')');
+  }
+
+  @BeforeClass
+  public static void setup() throws Exception {
+    workDir.mkdirs();
+    HS2 = new ItHiveServer2(workDir);
+    ZK_HS2 = new ItZKHiveServer2(workDir);
+
+    HS2.start();
+    ZK_HS2.start();
+  }
+
+  @Test
+  public void testBinaryJdbc() throws Exception {
+    testMetaOp(HS2.getBaseJdbcUrl(), true);
+    //testMetaOp(ZK_HS2.getJdbcUrl(), false);

Review Comment:
   Remove this line if we no need.



##########
itests/test-jdbc/src/test/java/org/apache/hive/jdbc/ItHiveServer2.java:
##########
@@ -0,0 +1,270 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hive.jdbc;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.minikdc.MiniKdc;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.testcontainers.containers.BindMode;
+import org.testcontainers.containers.GenericContainer;
+import org.testcontainers.containers.wait.strategy.AbstractWaitStrategy;
+import org.testcontainers.utility.DockerImageName;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+public class ItHiveServer2 extends ItAbstractContainer {
+  protected File workDir;
+  private final MiniJdbcKdc miniKdc;
+  protected GenericContainer<?> container;
+  protected final String imageName = "apache/hive:4.0.1";

Review Comment:
   BTW, i noticed the execution time of CI is 3h. But the execution time of 
others PR's CI is about 2.5h.
   
   
https://ci.hive.apache.org/blue/organizations/jenkins/hive-precommit/detail/PR-5993/2/pipeline
   
https://ci.hive.apache.org/blue/organizations/jenkins/hive-precommit/detail/PR-5991/1/pipeline
   
https://ci.hive.apache.org/blue/organizations/jenkins/hive-precommit/detail/PR-5987/1/pipeline
   
   
   Maybe we can run multipe times to double check if this PR can increase the 
execution time of CI. 



##########
pom.xml:
##########
@@ -61,6 +61,7 @@
     <module>standalone-metastore</module>
     <module>kafka-handler</module>
     <module>iceberg</module>
+    <module>itests/test-jdbc</module>

Review Comment:
   Why we add this module in root pom?
   
   Shouldn't this module be placed here 
https://github.com/apache/hive/blob/master/itests/pom.xml?
   
   And then build this module through `-Pitests`
   `mvn clean package -Pitests`



##########
itests/test-jdbc/src/test/java/org/apache/hive/jdbc/ItAbstractContainer.java:
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hive.jdbc;
+
+public abstract class ItAbstractContainer {

Review Comment:
   How about `ITAbstractContainer ` & `ITHiveServer2 `&  `ITZKHiveServer2`?
   
   We already similay naming like `ITestDbTxnManager`.



##########
itests/test-jdbc/src/test/java/org/apache/hive/jdbc/ItHiveServer2.java:
##########
@@ -0,0 +1,270 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hive.jdbc;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileReader;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.minikdc.MiniKdc;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.testcontainers.containers.BindMode;
+import org.testcontainers.containers.GenericContainer;
+import org.testcontainers.containers.wait.strategy.AbstractWaitStrategy;
+import org.testcontainers.utility.DockerImageName;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+public class ItHiveServer2 extends ItAbstractContainer {
+  protected File workDir;
+  private final MiniJdbcKdc miniKdc;
+  protected GenericContainer<?> container;
+  protected final String imageName = "apache/hive:4.0.1";

Review Comment:
   Here I have two concerns:  
   
   1) Can we avoid using the Hive Docker approach and instead start a local 
Hive process like other integration tests, then perform standalone JDBC 
testing? Because the Hive CI frequently encounters issues like download 
timeouts or failures for the docker image. If we can start a Hive service 
locally, we can avoid such problems.  
   
   2) If the Docker approach is mandatory to meet testing requirements, how 
should we control the Hive image version? Could there be compatibility issues 
between Hive 4.0.1 server and Hive 4.2.0 & 5.0.0 jdbc jars?



-- 
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: gitbox-unsubscr...@hive.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

Reply via email to