You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/06/18 15:43:24 UTC

[GitHub] [spark] gaborgsomogyi opened a new pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

gaborgsomogyi opened a new pull request #28863:
URL: https://github.com/apache/spark/pull/28863


   ### What changes were proposed in this pull request?
   When loading DataFrames from JDBC datasource with Kerberos authentication, remote executors (yarn-client/cluster etc. modes) fail to establish a connection due to lack of Kerberos ticket or ability to generate it.
   
   This is a real issue when trying to ingest data from kerberized data sources (SQL Server, Oracle) in enterprise environment where exposing simple authentication access is not an option due to IT policy issues.
   
   In this PR I've added Oracle support.
   
   What this PR contains:
   * Added `OracleConnectionProvider`
   * Added `OracleConnectionProviderSuite`
   * Changed Oracle JDBC driver to use the latest (test scope only)
   * Minor adaptations to make `OracleIntegrationSuite` compliant w/ the latest server
   
   ### Why are the changes needed?
   Missing JDBC kerberos support.
   
   ### Does this PR introduce _any_ user-facing change?
   Yes, now user is able to connect to Oracle using kerberos.
   
   ### How was this patch tested?
   * Additional + existing unit tests
   * Existing integration tests (especially `OracleIntegrationSuite`)
   * Test on cluster manually
   


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648102109






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646235703


   **[Test build #124231 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124231/testReport)** for PR 28863 at commit [`5cc3d0a`](https://github.com/apache/spark/commit/5cc3d0ad2485d60abf0861f5e9192eefd57551cf).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648108695






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-651138861


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124640/
   Test FAILed.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646236455






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650649535


   **[Test build #124576 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124576/testReport)** for PR 28863 at commit [`e16b4a4`](https://github.com/apache/spark/commit/e16b4a4b355951ad604069dd78ec2c1bb8ee04d2).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r445882894



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {

Review comment:
       Sorry for nit-picking. You are a senior and good contributor but it seems that you have been not aware of the Apache Spark community guide in this specific example. I saw similar instances in this series of PR. So, I added the above comment. Your commits an PRs are already referenced a lot. :) Thanks.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] MaxGekk commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-652022983


   Can this cause the build failure? https://github.com/apache/spark/pull/28959#issuecomment-652018831


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650640009


   **[Test build #124573 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124573/testReport)** for PR 28863 at commit [`e16b4a4`](https://github.com/apache/spark/commit/e16b4a4b355951ad604069dd78ec2c1bb8ee04d2).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-647473139


   I'm intended to merge master into this PR and resolve conflicts when https://github.com/apache/spark/pull/28893 accepted.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648218986


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124425/
   Test FAILed.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r444169203



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"
+
+  override def getConnection(): Connection = {
+    setAuthenticationConfigIfNeeded()
+    UserGroupInformation.loginUserFromKeytabAndReturnUGI(options.principal, options.keytab).doAs(
+      new PrivilegedExceptionAction[Connection]() {
+        override def run(): Connection = {
+          OracleConnectionProvider.super.getConnection()
+        }
+      }
+    )
+  }
+
+  override def getAdditionalProperties(): Properties = {
+    val result = new Properties()
+    // This prop needed to turn on kerberos authentication in the JDBC driver
+    result.put("oracle.net.authentication_services", "(KERBEROS5)");

Review comment:
       Good idea, added.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r446222287



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"
+
+  override def getConnection(): Connection = {
+    setAuthenticationConfigIfNeeded()
+    UserGroupInformation.loginUserFromKeytabAndReturnUGI(options.principal, options.keytab).doAs(
+      new PrivilegedExceptionAction[Connection]() {
+        override def run(): Connection = {
+          OracleConnectionProvider.super.getConnection()
+        }
+      }
+    )
+  }
+
+  override def getAdditionalProperties(): Properties = {
+    val result = new Properties()
+    // This prop needed to turn on kerberos authentication in the JDBC driver. Please see:
+    // scalastyle:off line.size.limit
+    // https://docs.oracle.com/en/database/oracle/oracle-database/19/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_THIN_NET_AUTHENTICATION_SERVICES
+    // scalastyle:on line.size.limit
+    result.put("oracle.net.authentication_services", "(KERBEROS5)");

Review comment:
       Hmmm, during the investigation I've found the following code in the JDBC driver implementation w/ JD-GUI:
   ```
   public interface AnoServices {
   ...
     public static final String AUTHENTICATION_KERBEROS5 = "KERBEROS5";
   ...
   ```
   Let's see how the test behaves.
   




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r446052230



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {

Review comment:
       Looked back from where this can come from. The only thing what I can think of is mixing up parameter 4 space indent w/ extends 2 space :) I'll consider this more from now on...




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650649740


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124576/
   Test FAILed.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-651000078


   **[Test build #124640 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124640/testReport)** for PR 28863 at commit [`461eed0`](https://github.com/apache/spark/commit/461eed070d86a7d9d5f8ba536a69d59621faa907).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443452819



##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -32,35 +32,29 @@ import org.apache.spark.tags.DockerTest
 
 /**
  * This patch was tested using the Oracle docker. Created this integration suite for the same.
- * The ojdbc6-11.2.0.2.0.jar was to be downloaded from the maven repository. Since there was
- * no jdbc jar available in the maven repository, the jar was downloaded from oracle site
- * manually and installed in the local; thus tested. So, for SparkQA test case run, the
- * ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0)
- * while Spark QA test run.
  *
  * The following would be the steps to test this
  * 1. Build Oracle database in Docker, please refer below link about how to.
  *    https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md
  * 2. export ORACLE_DOCKER_IMAGE_NAME=$ORACLE_DOCKER_IMAGE_NAME
  *    Pull oracle $ORACLE_DOCKER_IMAGE_NAME image - docker pull $ORACLE_DOCKER_IMAGE_NAME
  * 3. Start docker - sudo service docker start
- * 4. Download oracle 11g driver jar and put it in maven local repo:
- *    (com/oracle/ojdbc6/11.2.0.2.0/ojdbc6-11.2.0.2.0.jar)
- * 5. The timeout and interval parameter to be increased from 60,1 to a high value for oracle test
- *    in DockerJDBCIntegrationSuite.scala (Locally tested with 200,200 and executed successfully).
- * 6. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
+ * 4. The timeout and interval parameter to be increased to a high value for oracle test in
+ *     DockerJDBCIntegrationSuite.scala (Locally tested with timeout(20.minutes), interval(1.second)
+ *     and executed successfully).
+ * 5. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
  *
- * All tests in this suite are ignored because of the dependency with the oracle jar from maven
- * repository.
+ * It has been validated with 18.4.0 Express Edition.

Review comment:
       I've taken over this into the separate PR. Added `-Pdocker-integration-tests` here as well.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646115406


   Merged build finished. Test FAILed.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r446255388



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"
+
+  override def getConnection(): Connection = {
+    setAuthenticationConfigIfNeeded()
+    UserGroupInformation.loginUserFromKeytabAndReturnUGI(options.principal, options.keytab).doAs(
+      new PrivilegedExceptionAction[Connection]() {
+        override def run(): Connection = {
+          OracleConnectionProvider.super.getConnection()
+        }
+      }
+    )
+  }
+
+  override def getAdditionalProperties(): Properties = {
+    val result = new Properties()
+    // This prop needed to turn on kerberos authentication in the JDBC driver. Please see:
+    // scalastyle:off line.size.limit
+    // https://docs.oracle.com/en/database/oracle/oracle-database/19/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_THIN_NET_AUTHENTICATION_SERVICES
+    // scalastyle:on line.size.limit
+    result.put("oracle.net.authentication_services", "(KERBEROS5)");

Review comment:
       I've double checked and the version matches which makes the situation weird (there is an interface value in the JDBC driver impl which is not documented).
   I think what we can state here is that `AnoServices` constant works fine and that must be provided to make it work (that said such pre-configured configs can be overriden).
   I would remove the link and add this information if you agree. WDYT @dongjoon-hyun @maropu ?
   




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443454805



##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -69,6 +63,7 @@ class OracleIntegrationSuite extends DockerJDBCIntegrationSuite with SharedSpark
   }
 
   override def dataPreparation(conn: Connection): Unit = {
+    conn.setAutoCommit(false)

Review comment:
       I've taken over this into the separate PR.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646115416


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124222/
   Test FAILed.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r446078352



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"
+
+  override def getConnection(): Connection = {
+    setAuthenticationConfigIfNeeded()
+    UserGroupInformation.loginUserFromKeytabAndReturnUGI(options.principal, options.keytab).doAs(
+      new PrivilegedExceptionAction[Connection]() {
+        override def run(): Connection = {
+          OracleConnectionProvider.super.getConnection()
+        }
+      }
+    )
+  }
+
+  override def getAdditionalProperties(): Properties = {
+    val result = new Properties()
+    // This prop needed to turn on kerberos authentication in the JDBC driver. Please see:
+    // scalastyle:off line.size.limit
+    // https://docs.oracle.com/en/database/oracle/oracle-database/19/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_THIN_NET_AUTHENTICATION_SERVICES
+    // scalastyle:on line.size.limit
+    result.put("oracle.net.authentication_services", "(KERBEROS5)");

Review comment:
       At the first glance that smells like a major issue. What I can think of is that somehow the value and the define (`AnoServices.AUTHENTICATION_KERBEROS5`) mixed up. I'll double check in the driver implementation w/ JD-GUI + re-testing it on cluster to avoid questions. It will take a while so asking some patience.
   
   When I've designed this I've had the feeling these configs must be overridable for flexibility. Now it's clear how it could pay off.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-651113159


   **[Test build #124640 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124640/testReport)** for PR 28863 at commit [`461eed0`](https://github.com/apache/spark/commit/461eed070d86a7d9d5f8ba536a69d59621faa907).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648239051






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maropu commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-651478897






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650640345


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124573/
   Test FAILed.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r445884431



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"
+
+  override def getConnection(): Connection = {
+    setAuthenticationConfigIfNeeded()
+    UserGroupInformation.loginUserFromKeytabAndReturnUGI(options.principal, options.keytab).doAs(
+      new PrivilegedExceptionAction[Connection]() {
+        override def run(): Connection = {
+          OracleConnectionProvider.super.getConnection()
+        }
+      }
+    )
+  }
+
+  override def getAdditionalProperties(): Properties = {
+    val result = new Properties()
+    // This prop needed to turn on kerberos authentication in the JDBC driver. Please see:
+    // scalastyle:off line.size.limit
+    // https://docs.oracle.com/en/database/oracle/oracle-database/19/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_THIN_NET_AUTHENTICATION_SERVICES
+    // scalastyle:on line.size.limit
+    result.put("oracle.net.authentication_services", "(KERBEROS5)");

Review comment:
       Just a question. Why do we use `(KERBEROS5)` instead of `(KERBEROS)` in the given link? The new value looks correct to me, but it's weird to give a link with a different content. It would be great if we can give a more accurate link about `(KERBEROS5)` (if possible).




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648200422


   **[Test build #124420 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124420/testReport)** for PR 28863 at commit [`e16b4a4`](https://github.com/apache/spark/commit/e16b4a4b355951ad604069dd78ec2c1bb8ee04d2).
    * This patch **fails Spark unit tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648218134






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650588854


   **[Test build #124569 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124569/testReport)** for PR 28863 at commit [`e16b4a4`](https://github.com/apache/spark/commit/e16b4a4b355951ad604069dd78ec2c1bb8ee04d2).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646115388


   **[Test build #124222 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124222/testReport)** for PR 28863 at commit [`5cc3d0a`](https://github.com/apache/spark/commit/5cc3d0ad2485d60abf0861f5e9192eefd57551cf).
    * This patch **fails build dependency tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maropu commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443105054



##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -32,35 +32,29 @@ import org.apache.spark.tags.DockerTest
 
 /**
  * This patch was tested using the Oracle docker. Created this integration suite for the same.
- * The ojdbc6-11.2.0.2.0.jar was to be downloaded from the maven repository. Since there was
- * no jdbc jar available in the maven repository, the jar was downloaded from oracle site
- * manually and installed in the local; thus tested. So, for SparkQA test case run, the
- * ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0)
- * while Spark QA test run.
  *
  * The following would be the steps to test this
  * 1. Build Oracle database in Docker, please refer below link about how to.
  *    https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md
  * 2. export ORACLE_DOCKER_IMAGE_NAME=$ORACLE_DOCKER_IMAGE_NAME
  *    Pull oracle $ORACLE_DOCKER_IMAGE_NAME image - docker pull $ORACLE_DOCKER_IMAGE_NAME
  * 3. Start docker - sudo service docker start
- * 4. Download oracle 11g driver jar and put it in maven local repo:
- *    (com/oracle/ojdbc6/11.2.0.2.0/ojdbc6-11.2.0.2.0.jar)
- * 5. The timeout and interval parameter to be increased from 60,1 to a high value for oracle test
- *    in DockerJDBCIntegrationSuite.scala (Locally tested with 200,200 and executed successfully).
- * 6. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
+ * 4. The timeout and interval parameter to be increased to a high value for oracle test in
+ *     DockerJDBCIntegrationSuite.scala (Locally tested with timeout(20.minutes), interval(1.second)
+ *     and executed successfully).

Review comment:
       nit: looks wrong indents.

##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -32,35 +32,29 @@ import org.apache.spark.tags.DockerTest
 
 /**
  * This patch was tested using the Oracle docker. Created this integration suite for the same.
- * The ojdbc6-11.2.0.2.0.jar was to be downloaded from the maven repository. Since there was
- * no jdbc jar available in the maven repository, the jar was downloaded from oracle site
- * manually and installed in the local; thus tested. So, for SparkQA test case run, the
- * ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0)
- * while Spark QA test run.
  *
  * The following would be the steps to test this
  * 1. Build Oracle database in Docker, please refer below link about how to.
  *    https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md
  * 2. export ORACLE_DOCKER_IMAGE_NAME=$ORACLE_DOCKER_IMAGE_NAME
  *    Pull oracle $ORACLE_DOCKER_IMAGE_NAME image - docker pull $ORACLE_DOCKER_IMAGE_NAME
  * 3. Start docker - sudo service docker start
- * 4. Download oracle 11g driver jar and put it in maven local repo:
- *    (com/oracle/ojdbc6/11.2.0.2.0/ojdbc6-11.2.0.2.0.jar)
- * 5. The timeout and interval parameter to be increased from 60,1 to a high value for oracle test
- *    in DockerJDBCIntegrationSuite.scala (Locally tested with 200,200 and executed successfully).
- * 6. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
+ * 4. The timeout and interval parameter to be increased to a high value for oracle test in
+ *     DockerJDBCIntegrationSuite.scala (Locally tested with timeout(20.minutes), interval(1.second)
+ *     and executed successfully).
+ * 5. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
  *
- * All tests in this suite are ignored because of the dependency with the oracle jar from maven
- * repository.
+ * It has been validated with 18.4.0 Express Edition.
  */
+

Review comment:
       nit: plz remove the blank.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648218949


   **[Test build #124425 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124425/testReport)** for PR 28863 at commit [`e16b4a4`](https://github.com/apache/spark/commit/e16b4a4b355951ad604069dd78ec2c1bb8ee04d2).
    * This patch **fails build dependency tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648101561


   **[Test build #124418 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124418/testReport)** for PR 28863 at commit [`d698fea`](https://github.com/apache/spark/commit/d698fea72374033e3bf3bbba858b6e95e99a88b1).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646113845


   **[Test build #124222 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124222/testReport)** for PR 28863 at commit [`5cc3d0a`](https://github.com/apache/spark/commit/5cc3d0ad2485d60abf0861f5e9192eefd57551cf).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646235703


   **[Test build #124231 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124231/testReport)** for PR 28863 at commit [`5cc3d0a`](https://github.com/apache/spark/commit/5cc3d0ad2485d60abf0861f5e9192eefd57551cf).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-651000722






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r446901631



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {

Review comment:
       https://github.com/apache/spark/pull/28945




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r446243027



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"
+
+  override def getConnection(): Connection = {
+    setAuthenticationConfigIfNeeded()
+    UserGroupInformation.loginUserFromKeytabAndReturnUGI(options.principal, options.keytab).doAs(
+      new PrivilegedExceptionAction[Connection]() {
+        override def run(): Connection = {
+          OracleConnectionProvider.super.getConnection()
+        }
+      }
+    )
+  }
+
+  override def getAdditionalProperties(): Properties = {
+    val result = new Properties()
+    // This prop needed to turn on kerberos authentication in the JDBC driver. Please see:
+    // scalastyle:off line.size.limit
+    // https://docs.oracle.com/en/database/oracle/oracle-database/19/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_THIN_NET_AUTHENTICATION_SERVICES
+    // scalastyle:on line.size.limit
+    result.put("oracle.net.authentication_services", "(KERBEROS5)");

Review comment:
       Now I'm pretty sure this is a documentation issue because of the following:
   * The driver [documentation](https://docs.oracle.com/en/database/oracle/oracle-database/19/jjdbc/client-side-security.html#GUID-991705F7-C2C5-4BA9-85D1-32749AE2FF64) gives the following as an example:
   ```
   ...
       Connection conn = 
         (Connection)Subject.doAs(specificSubject, new PrivilegedExceptionAction()
           {
             public Object run()
             {
               Connection con = null;
               Properties prop = new Properties();
               prop.setProperty(AnoServices.AUTHENTICATION_PROPERTY_SERVICES, 
                                "(" + AnoServices.AUTHENTICATION_KERBEROS5 + ")");
   ...
   ```
   and that said I've found `AUTHENTICATION_KERBEROS5 = "KERBEROS5";` in `AnoServices`.
   * Cluster test has passed w/ `KERBEROS5` 
   
   Checking the documentation whether it fits to this version...
   




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648102109






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646314228






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r446838215



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"
+
+  override def getConnection(): Connection = {
+    setAuthenticationConfigIfNeeded()
+    UserGroupInformation.loginUserFromKeytabAndReturnUGI(options.principal, options.keytab).doAs(
+      new PrivilegedExceptionAction[Connection]() {
+        override def run(): Connection = {
+          OracleConnectionProvider.super.getConnection()
+        }
+      }
+    )
+  }
+
+  override def getAdditionalProperties(): Properties = {
+    val result = new Properties()
+    // This prop needed to turn on kerberos authentication in the JDBC driver. Please see:
+    // scalastyle:off line.size.limit
+    // https://docs.oracle.com/en/database/oracle/oracle-database/19/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_THIN_NET_AUTHENTICATION_SERVICES
+    // scalastyle:on line.size.limit
+    result.put("oracle.net.authentication_services", "(KERBEROS5)");

Review comment:
       Fixed the comment.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650649243


   Retest this please.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-651449133


   Hi, @gaborgsomogyi . Is `OracleKrbIntegrationSuite` missing here?


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r446255388



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"
+
+  override def getConnection(): Connection = {
+    setAuthenticationConfigIfNeeded()
+    UserGroupInformation.loginUserFromKeytabAndReturnUGI(options.principal, options.keytab).doAs(
+      new PrivilegedExceptionAction[Connection]() {
+        override def run(): Connection = {
+          OracleConnectionProvider.super.getConnection()
+        }
+      }
+    )
+  }
+
+  override def getAdditionalProperties(): Properties = {
+    val result = new Properties()
+    // This prop needed to turn on kerberos authentication in the JDBC driver. Please see:
+    // scalastyle:off line.size.limit
+    // https://docs.oracle.com/en/database/oracle/oracle-database/19/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_THIN_NET_AUTHENTICATION_SERVICES
+    // scalastyle:on line.size.limit
+    result.put("oracle.net.authentication_services", "(KERBEROS5)");

Review comment:
       I've double checked and the version matches which makes the situation weird (there is an interface value in the JDBC driver impl which is not documented).
   I think what we can state here is that `AnoServices` constant works fine and that must be provided to make it work (that said such pre-configured configs can be overriden if changes).
   I would remove the link and add this information if you agree. WDYT @dongjoon-hyun @maropu ?
   




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650640009


   **[Test build #124573 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124573/testReport)** for PR 28863 at commit [`e16b4a4`](https://github.com/apache/spark/commit/e16b4a4b355951ad604069dd78ec2c1bb8ee04d2).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646114762






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maropu commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443109663



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"

Review comment:
       Just a question; where does this value come? From the Oracle JDBC impl?




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648218134






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648101561


   **[Test build #124418 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124418/testReport)** for PR 28863 at commit [`d698fea`](https://github.com/apache/spark/commit/d698fea72374033e3bf3bbba858b6e95e99a88b1).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650649616






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648218975






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443448728



##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -32,35 +32,29 @@ import org.apache.spark.tags.DockerTest
 
 /**
  * This patch was tested using the Oracle docker. Created this integration suite for the same.
- * The ojdbc6-11.2.0.2.0.jar was to be downloaded from the maven repository. Since there was
- * no jdbc jar available in the maven repository, the jar was downloaded from oracle site
- * manually and installed in the local; thus tested. So, for SparkQA test case run, the
- * ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0)
- * while Spark QA test run.
  *
  * The following would be the steps to test this
  * 1. Build Oracle database in Docker, please refer below link about how to.
  *    https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md
  * 2. export ORACLE_DOCKER_IMAGE_NAME=$ORACLE_DOCKER_IMAGE_NAME
  *    Pull oracle $ORACLE_DOCKER_IMAGE_NAME image - docker pull $ORACLE_DOCKER_IMAGE_NAME
  * 3. Start docker - sudo service docker start
- * 4. Download oracle 11g driver jar and put it in maven local repo:
- *    (com/oracle/ojdbc6/11.2.0.2.0/ojdbc6-11.2.0.2.0.jar)
- * 5. The timeout and interval parameter to be increased from 60,1 to a high value for oracle test
- *    in DockerJDBCIntegrationSuite.scala (Locally tested with 200,200 and executed successfully).
- * 6. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
+ * 4. The timeout and interval parameter to be increased to a high value for oracle test in
+ *     DockerJDBCIntegrationSuite.scala (Locally tested with timeout(20.minutes), interval(1.second)
+ *     and executed successfully).

Review comment:
       I've taken over the nit + suggested rephrase into the separate PR.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r445881340



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {

Review comment:
       nit. Indentation?
   - https://github.com/databricks/scala-style-guide/blame/master/README.md#L242




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r444185190



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProviderSuite.scala
##########
@@ -0,0 +1,28 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+class OracleConnectionProviderSuite extends ConnectionProviderSuiteBase {
+  test("setAuthenticationConfigIfNeeded must set authentication if not set") {

Review comment:
       How do you suggest to do that? Driver registration and provider instantiation lines are different in each case.
   The only duplicate what I see is the test name + the `testSecureConnectionProvider` call.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648108082


   **[Test build #124420 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124420/testReport)** for PR 28863 at commit [`e16b4a4`](https://github.com/apache/spark/commit/e16b4a4b355951ad604069dd78ec2c1bb8ee04d2).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650640344


   Merged build finished. Test FAILed.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-651000722






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650640164






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-651138851






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r442327252



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)

Review comment:
       The implementation is based on [this](https://docs.oracle.com/en/database/oracle/oracle-database/19/jjdbc/client-side-security.html#GUID-97E88D18-B590-4313-87B8-EE422DA644B3).




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650588854


   **[Test build #124569 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124569/testReport)** for PR 28863 at commit [`e16b4a4`](https://github.com/apache/spark/commit/e16b4a4b355951ad604069dd78ec2c1bb8ee04d2).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maropu commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443106215



##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -32,35 +32,29 @@ import org.apache.spark.tags.DockerTest
 
 /**
  * This patch was tested using the Oracle docker. Created this integration suite for the same.
- * The ojdbc6-11.2.0.2.0.jar was to be downloaded from the maven repository. Since there was
- * no jdbc jar available in the maven repository, the jar was downloaded from oracle site
- * manually and installed in the local; thus tested. So, for SparkQA test case run, the
- * ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0)
- * while Spark QA test run.
  *
  * The following would be the steps to test this
  * 1. Build Oracle database in Docker, please refer below link about how to.
  *    https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md
  * 2. export ORACLE_DOCKER_IMAGE_NAME=$ORACLE_DOCKER_IMAGE_NAME
  *    Pull oracle $ORACLE_DOCKER_IMAGE_NAME image - docker pull $ORACLE_DOCKER_IMAGE_NAME
  * 3. Start docker - sudo service docker start
- * 4. Download oracle 11g driver jar and put it in maven local repo:
- *    (com/oracle/ojdbc6/11.2.0.2.0/ojdbc6-11.2.0.2.0.jar)
- * 5. The timeout and interval parameter to be increased from 60,1 to a high value for oracle test
- *    in DockerJDBCIntegrationSuite.scala (Locally tested with 200,200 and executed successfully).
- * 6. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
+ * 4. The timeout and interval parameter to be increased to a high value for oracle test in
+ *     DockerJDBCIntegrationSuite.scala (Locally tested with timeout(20.minutes), interval(1.second)
+ *     and executed successfully).

Review comment:
       How about rephrasing it a bit like this?
   ```
    * 4. The timeout and interval parameter to be increased to a high value for oracle test in
    *    DockerJDBCIntegrationSuite.scala (Locally tested with 20 min timeout and 1 sec interval
    *    then executed successfully).
   ```




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443453266



##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -32,35 +32,29 @@ import org.apache.spark.tags.DockerTest
 
 /**
  * This patch was tested using the Oracle docker. Created this integration suite for the same.
- * The ojdbc6-11.2.0.2.0.jar was to be downloaded from the maven repository. Since there was
- * no jdbc jar available in the maven repository, the jar was downloaded from oracle site
- * manually and installed in the local; thus tested. So, for SparkQA test case run, the
- * ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0)
- * while Spark QA test run.
  *
  * The following would be the steps to test this
  * 1. Build Oracle database in Docker, please refer below link about how to.
  *    https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md
  * 2. export ORACLE_DOCKER_IMAGE_NAME=$ORACLE_DOCKER_IMAGE_NAME
  *    Pull oracle $ORACLE_DOCKER_IMAGE_NAME image - docker pull $ORACLE_DOCKER_IMAGE_NAME
  * 3. Start docker - sudo service docker start
- * 4. Download oracle 11g driver jar and put it in maven local repo:
- *    (com/oracle/ojdbc6/11.2.0.2.0/ojdbc6-11.2.0.2.0.jar)
- * 5. The timeout and interval parameter to be increased from 60,1 to a high value for oracle test
- *    in DockerJDBCIntegrationSuite.scala (Locally tested with 200,200 and executed successfully).
- * 6. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
+ * 4. The timeout and interval parameter to be increased to a high value for oracle test in
+ *     DockerJDBCIntegrationSuite.scala (Locally tested with timeout(20.minutes), interval(1.second)
+ *     and executed successfully).
+ * 5. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
  *
- * All tests in this suite are ignored because of the dependency with the oracle jar from maven
- * repository.
+ * It has been validated with 18.4.0 Express Edition.
  */
+

Review comment:
       Not added in the separate PR.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650589150






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r444187452



##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -55,6 +55,7 @@ import org.apache.spark.tags.DockerTest
  *
  * It has been validated with 18.4.0 Express Edition.
  */
+

Review comment:
       Just fixed.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r442333450



##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -32,35 +32,29 @@ import org.apache.spark.tags.DockerTest
 
 /**
  * This patch was tested using the Oracle docker. Created this integration suite for the same.
- * The ojdbc6-11.2.0.2.0.jar was to be downloaded from the maven repository. Since there was
- * no jdbc jar available in the maven repository, the jar was downloaded from oracle site
- * manually and installed in the local; thus tested. So, for SparkQA test case run, the
- * ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0)
- * while Spark QA test run.
  *
  * The following would be the steps to test this
  * 1. Build Oracle database in Docker, please refer below link about how to.
  *    https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md
  * 2. export ORACLE_DOCKER_IMAGE_NAME=$ORACLE_DOCKER_IMAGE_NAME
  *    Pull oracle $ORACLE_DOCKER_IMAGE_NAME image - docker pull $ORACLE_DOCKER_IMAGE_NAME
  * 3. Start docker - sudo service docker start
- * 4. Download oracle 11g driver jar and put it in maven local repo:
- *    (com/oracle/ojdbc6/11.2.0.2.0/ojdbc6-11.2.0.2.0.jar)
- * 5. The timeout and interval parameter to be increased from 60,1 to a high value for oracle test
- *    in DockerJDBCIntegrationSuite.scala (Locally tested with 200,200 and executed successfully).
- * 6. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
+ * 4. The timeout and interval parameter to be increased to a high value for oracle test in
+ *     DockerJDBCIntegrationSuite.scala (Locally tested with timeout(20.minutes), interval(1.second)
+ *     and executed successfully).
+ * 5. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
  *
- * All tests in this suite are ignored because of the dependency with the oracle jar from maven
- * repository.
+ * It has been validated with 18.4.0 Express Edition.

Review comment:
       Actual commands executed:
   ```
   git clone https://github.com/oracle/docker-images.git
   cd docker-images/OracleDatabase/SingleInstance/dockerfiles
   ./buildDockerImage.sh -v 18.4.0 -x
   export ORACLE_DOCKER_IMAGE_NAME=oracle/database:18.4.0-xe
   # Execute OracleIntegrationSuite manually from Intellij
   ```
   

##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -32,35 +32,29 @@ import org.apache.spark.tags.DockerTest
 
 /**
  * This patch was tested using the Oracle docker. Created this integration suite for the same.
- * The ojdbc6-11.2.0.2.0.jar was to be downloaded from the maven repository. Since there was
- * no jdbc jar available in the maven repository, the jar was downloaded from oracle site
- * manually and installed in the local; thus tested. So, for SparkQA test case run, the
- * ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0)
- * while Spark QA test run.
  *
  * The following would be the steps to test this
  * 1. Build Oracle database in Docker, please refer below link about how to.
  *    https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md
  * 2. export ORACLE_DOCKER_IMAGE_NAME=$ORACLE_DOCKER_IMAGE_NAME
  *    Pull oracle $ORACLE_DOCKER_IMAGE_NAME image - docker pull $ORACLE_DOCKER_IMAGE_NAME
  * 3. Start docker - sudo service docker start
- * 4. Download oracle 11g driver jar and put it in maven local repo:
- *    (com/oracle/ojdbc6/11.2.0.2.0/ojdbc6-11.2.0.2.0.jar)
- * 5. The timeout and interval parameter to be increased from 60,1 to a high value for oracle test
- *    in DockerJDBCIntegrationSuite.scala (Locally tested with 200,200 and executed successfully).
- * 6. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
+ * 4. The timeout and interval parameter to be increased to a high value for oracle test in
+ *     DockerJDBCIntegrationSuite.scala (Locally tested with timeout(20.minutes), interval(1.second)
+ *     and executed successfully).
+ * 5. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
  *
- * All tests in this suite are ignored because of the dependency with the oracle jar from maven
- * repository.
+ * It has been validated with 18.4.0 Express Edition.
  */
+
 @DockerTest
 class OracleIntegrationSuite extends DockerJDBCIntegrationSuite with SharedSparkSession {
   import testImplicits._
 
   override val db = new DatabaseOnDocker {
     override val imageName = sys.env("ORACLE_DOCKER_IMAGE_NAME")
     override val env = Map(
-      "ORACLE_ROOT_PASSWORD" -> "oracle"
+      "ORACLE_PWD" -> "oracle"

Review comment:
       18.4.0 Express Edition has changed the name of the parameter.

##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)

Review comment:
       The implementation is based on [this](https://docs.oracle.com/en/database/oracle/oracle-database/19/jjdbc/client-side-security.html).

##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -32,35 +32,29 @@ import org.apache.spark.tags.DockerTest
 
 /**
  * This patch was tested using the Oracle docker. Created this integration suite for the same.
- * The ojdbc6-11.2.0.2.0.jar was to be downloaded from the maven repository. Since there was
- * no jdbc jar available in the maven repository, the jar was downloaded from oracle site
- * manually and installed in the local; thus tested. So, for SparkQA test case run, the
- * ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0)
- * while Spark QA test run.
  *
  * The following would be the steps to test this
  * 1. Build Oracle database in Docker, please refer below link about how to.
  *    https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md
  * 2. export ORACLE_DOCKER_IMAGE_NAME=$ORACLE_DOCKER_IMAGE_NAME
  *    Pull oracle $ORACLE_DOCKER_IMAGE_NAME image - docker pull $ORACLE_DOCKER_IMAGE_NAME
  * 3. Start docker - sudo service docker start
- * 4. Download oracle 11g driver jar and put it in maven local repo:
- *    (com/oracle/ojdbc6/11.2.0.2.0/ojdbc6-11.2.0.2.0.jar)
- * 5. The timeout and interval parameter to be increased from 60,1 to a high value for oracle test
- *    in DockerJDBCIntegrationSuite.scala (Locally tested with 200,200 and executed successfully).
- * 6. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
+ * 4. The timeout and interval parameter to be increased to a high value for oracle test in
+ *     DockerJDBCIntegrationSuite.scala (Locally tested with timeout(20.minutes), interval(1.second)

Review comment:
       I feel like it's a bit hacky to change timeout and interval manually but I think it would be an overkill to increase it globally.

##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -69,6 +63,7 @@ class OracleIntegrationSuite extends DockerJDBCIntegrationSuite with SharedSpark
   }
 
   override def dataPreparation(conn: Connection): Unit = {
+    conn.setAutoCommit(false)

Review comment:
       In 18.4.0 Express Edition auto commit is enabled by default.

##########
File path: pom.xml
##########
@@ -984,6 +984,12 @@
         <version>8.2.2.jre8</version>
         <scope>test</scope>
       </dependency>
+      <dependency>
+        <groupId>com.oracle.database.jdbc</groupId>

Review comment:
       This is the latest version from the Oracle JDBC driver which supports JDK8, JDK9, and JDK11: https://mvnrepository.com/artifact/com.oracle.database.jdbc/ojdbc8

##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"
+
+  override def getConnection(): Connection = {
+    setAuthenticationConfigIfNeeded()
+    UserGroupInformation.loginUserFromKeytabAndReturnUGI(options.principal, options.keytab).doAs(
+      new PrivilegedExceptionAction[Connection]() {
+        override def run(): Connection = {
+          OracleConnectionProvider.super.getConnection()
+        }
+      }
+    )
+  }
+
+  override def getAdditionalProperties(): Properties = {
+    val result = new Properties()
+    // This prop needed to turn on kerberos authentication in the JDBC driver
+    result.put("oracle.net.authentication_services", "(KERBEROS5)");
+    result
+  }
+
+  override def setAuthenticationConfigIfNeeded(): Unit = SecurityConfigurationLock.synchronized {

Review comment:
       Here synchronization is important to avoid race just like in other providers.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gatorsmile commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gatorsmile commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646940577


   cc @maropu @MaxGekk 


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650589153


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124569/
   Test FAILed.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650649738






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maropu commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443105992



##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -32,35 +32,29 @@ import org.apache.spark.tags.DockerTest
 
 /**
  * This patch was tested using the Oracle docker. Created this integration suite for the same.
- * The ojdbc6-11.2.0.2.0.jar was to be downloaded from the maven repository. Since there was
- * no jdbc jar available in the maven repository, the jar was downloaded from oracle site
- * manually and installed in the local; thus tested. So, for SparkQA test case run, the
- * ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0)
- * while Spark QA test run.
  *
  * The following would be the steps to test this
  * 1. Build Oracle database in Docker, please refer below link about how to.
  *    https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md
  * 2. export ORACLE_DOCKER_IMAGE_NAME=$ORACLE_DOCKER_IMAGE_NAME
  *    Pull oracle $ORACLE_DOCKER_IMAGE_NAME image - docker pull $ORACLE_DOCKER_IMAGE_NAME
  * 3. Start docker - sudo service docker start
- * 4. Download oracle 11g driver jar and put it in maven local repo:
- *    (com/oracle/ojdbc6/11.2.0.2.0/ojdbc6-11.2.0.2.0.jar)
- * 5. The timeout and interval parameter to be increased from 60,1 to a high value for oracle test
- *    in DockerJDBCIntegrationSuite.scala (Locally tested with 200,200 and executed successfully).
- * 6. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
+ * 4. The timeout and interval parameter to be increased to a high value for oracle test in
+ *     DockerJDBCIntegrationSuite.scala (Locally tested with timeout(20.minutes), interval(1.second)
+ *     and executed successfully).
+ * 5. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
  *
- * All tests in this suite are ignored because of the dependency with the oracle jar from maven
- * repository.
+ * It has been validated with 18.4.0 Express Edition.

Review comment:
       This info looks useful, so how about moving it into the comment side like this?
   ```
   
    * ...
    * 5. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
    *
    * An actual sequence of commands to run the test is as follows
    *
    *  $ git clone https://github.com/oracle/docker-images.git
    *  // Head SHA: 3e352a22618070595f823977a0fd1a3a8071a83c
    *  $ cd docker-images/OracleDatabase/SingleInstance/dockerfiles
    *  $ ./buildDockerImage.sh -v 18.4.0 -x
    *  $ export ORACLE_DOCKER_IMAGE_NAME=oracle/database:18.4.0-xe
    *  $ cd $SPARK_HOME
    *  $ ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
    *
    * It has been validated with 18.4.0 Express Edition.
    */
   ```




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-651617129


   @dongjoon-hyun I've spent almost gross 1 month to make it work w/ `MiniKDC` but no success. Please see my comment [up](https://github.com/apache/spark/pull/28863#pullrequestreview-433415583). Not found any clear statement it must work so I gave up. If somebody can make it work or when active directory will be available in docker we can add it.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650589150


   Merged build finished. Test FAILed.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r444168127



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"

Review comment:
       Yeah, I've used JD-GUI to take a look at the details.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650649616






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650589145


   **[Test build #124569 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124569/testReport)** for PR 28863 at commit [`e16b4a4`](https://github.com/apache/spark/commit/e16b4a4b355951ad604069dd78ec2c1bb8ee04d2).
    * This patch **fails build dependency tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650649731


   **[Test build #124576 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124576/testReport)** for PR 28863 at commit [`e16b4a4`](https://github.com/apache/spark/commit/e16b4a4b355951ad604069dd78ec2c1bb8ee04d2).
    * This patch **fails build dependency tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r446832662



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"
+
+  override def getConnection(): Connection = {
+    setAuthenticationConfigIfNeeded()
+    UserGroupInformation.loginUserFromKeytabAndReturnUGI(options.principal, options.keytab).doAs(
+      new PrivilegedExceptionAction[Connection]() {
+        override def run(): Connection = {
+          OracleConnectionProvider.super.getConnection()
+        }
+      }
+    )
+  }
+
+  override def getAdditionalProperties(): Properties = {
+    val result = new Properties()
+    // This prop needed to turn on kerberos authentication in the JDBC driver. Please see:
+    // scalastyle:off line.size.limit
+    // https://docs.oracle.com/en/database/oracle/oracle-database/19/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_THIN_NET_AUTHENTICATION_SERVICES
+    // scalastyle:on line.size.limit
+    result.put("oracle.net.authentication_services", "(KERBEROS5)");

Review comment:
       Thanks guys! Processing the remaining suggestions.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648108695






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443075731



##########
File path: external/docker-integration-tests/pom.xml
##########
@@ -130,15 +130,9 @@
       <artifactId>postgresql</artifactId>
       <scope>test</scope>
     </dependency>
-    <!-- Oracle ojdbc jar, used for oracle  integration suite for docker testing.
-     See https://github.com/apache/spark/pull/11306 for background on why we need
-     to use a an ojdbc jar for the testcase. The maven dependency here is commented
-     because currently the maven repository does not contain the ojdbc jar mentioned.
-     Once the jar is available in maven, this could be uncommented. -->
-    <dependency>
-      <groupId>com.oracle</groupId>
-      <artifactId>ojdbc6</artifactId>
-      <version>11.2.0.1.0</version>
+    <dependency>
+      <groupId>com.oracle.database.jdbc</groupId>
+      <artifactId>ojdbc8</artifactId>

Review comment:
       Hi, @gaborgsomogyi . 
   `Support Oracle Kerberos login in JDBC connector` is a good contribution, but upgrading the existing `OracleIntegrationSuite` is also another independent good contribution. This is worth to have a JIRA ID.
   Could you proceed upgrading `OracleIntegrationSuite` first as a separate JIRA?




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650640338


   **[Test build #124573 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124573/testReport)** for PR 28863 at commit [`e16b4a4`](https://github.com/apache/spark/commit/e16b4a4b355951ad604069dd78ec2c1bb8ee04d2).
    * This patch **fails build dependency tests**.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646113845


   **[Test build #124222 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124222/testReport)** for PR 28863 at commit [`5cc3d0a`](https://github.com/apache/spark/commit/5cc3d0ad2485d60abf0861f5e9192eefd57551cf).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443075731



##########
File path: external/docker-integration-tests/pom.xml
##########
@@ -130,15 +130,9 @@
       <artifactId>postgresql</artifactId>
       <scope>test</scope>
     </dependency>
-    <!-- Oracle ojdbc jar, used for oracle  integration suite for docker testing.
-     See https://github.com/apache/spark/pull/11306 for background on why we need
-     to use a an ojdbc jar for the testcase. The maven dependency here is commented
-     because currently the maven repository does not contain the ojdbc jar mentioned.
-     Once the jar is available in maven, this could be uncommented. -->
-    <dependency>
-      <groupId>com.oracle</groupId>
-      <artifactId>ojdbc6</artifactId>
-      <version>11.2.0.1.0</version>
+    <dependency>
+      <groupId>com.oracle.database.jdbc</groupId>
+      <artifactId>ojdbc8</artifactId>

Review comment:
       Hi, @gaborgsomogyi . 
   `Support Oracle Kerberos login in JDBC connector` is a good contribution, but upgrading the existing `OracleIntegrationSuite` is also another independent good contribution.
   Could you proceed upgrading `OracleIntegrationSuite` first as a separate JIRA?




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r444185190



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProviderSuite.scala
##########
@@ -0,0 +1,28 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+class OracleConnectionProviderSuite extends ConnectionProviderSuiteBase {
+  test("setAuthenticationConfigIfNeeded must set authentication if not set") {

Review comment:
       How do you suggest to do that? Driver registration and provider instantiation lines are different in each case.
   The only duplicate what I see the the test name + the `testSecureConnectionProvider` call.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maropu commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r446438925



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"
+
+  override def getConnection(): Connection = {
+    setAuthenticationConfigIfNeeded()
+    UserGroupInformation.loginUserFromKeytabAndReturnUGI(options.principal, options.keytab).doAs(
+      new PrivilegedExceptionAction[Connection]() {
+        override def run(): Connection = {
+          OracleConnectionProvider.super.getConnection()
+        }
+      }
+    )
+  }
+
+  override def getAdditionalProperties(): Properties = {
+    val result = new Properties()
+    // This prop needed to turn on kerberos authentication in the JDBC driver. Please see:
+    // scalastyle:off line.size.limit
+    // https://docs.oracle.com/en/database/oracle/oracle-database/19/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_THIN_NET_AUTHENTICATION_SERVICES
+    // scalastyle:on line.size.limit
+    result.put("oracle.net.authentication_services", "(KERBEROS5)");

Review comment:
       Thanks for the investigation! +1




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650639817


   Retest this please.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r444188045



##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -32,35 +32,29 @@ import org.apache.spark.tags.DockerTest
 
 /**
  * This patch was tested using the Oracle docker. Created this integration suite for the same.
- * The ojdbc6-11.2.0.2.0.jar was to be downloaded from the maven repository. Since there was
- * no jdbc jar available in the maven repository, the jar was downloaded from oracle site
- * manually and installed in the local; thus tested. So, for SparkQA test case run, the
- * ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0)
- * while Spark QA test run.
  *
  * The following would be the steps to test this
  * 1. Build Oracle database in Docker, please refer below link about how to.
  *    https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md
  * 2. export ORACLE_DOCKER_IMAGE_NAME=$ORACLE_DOCKER_IMAGE_NAME
  *    Pull oracle $ORACLE_DOCKER_IMAGE_NAME image - docker pull $ORACLE_DOCKER_IMAGE_NAME
  * 3. Start docker - sudo service docker start
- * 4. Download oracle 11g driver jar and put it in maven local repo:
- *    (com/oracle/ojdbc6/11.2.0.2.0/ojdbc6-11.2.0.2.0.jar)
- * 5. The timeout and interval parameter to be increased from 60,1 to a high value for oracle test
- *    in DockerJDBCIntegrationSuite.scala (Locally tested with 200,200 and executed successfully).
- * 6. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
+ * 4. The timeout and interval parameter to be increased to a high value for oracle test in
+ *     DockerJDBCIntegrationSuite.scala (Locally tested with timeout(20.minutes), interval(1.second)
+ *     and executed successfully).
+ * 5. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
  *
- * All tests in this suite are ignored because of the dependency with the oracle jar from maven
- * repository.
+ * It has been validated with 18.4.0 Express Edition.
  */
+
 @DockerTest
 class OracleIntegrationSuite extends DockerJDBCIntegrationSuite with SharedSparkSession {
   import testImplicits._
 
   override val db = new DatabaseOnDocker {
     override val imageName = sys.env("ORACLE_DOCKER_IMAGE_NAME")
     override val env = Map(
-      "ORACLE_ROOT_PASSWORD" -> "oracle"
+      "ORACLE_PWD" -> "oracle"

Review comment:
       This is not relevant anymore.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r445883446



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"
+
+  override def getConnection(): Connection = {
+    setAuthenticationConfigIfNeeded()
+    UserGroupInformation.loginUserFromKeytabAndReturnUGI(options.principal, options.keytab).doAs(
+      new PrivilegedExceptionAction[Connection]() {
+        override def run(): Connection = {
+          OracleConnectionProvider.super.getConnection()
+        }
+      }
+    )
+  }
+
+  override def getAdditionalProperties(): Properties = {
+    val result = new Properties()
+    // This prop needed to turn on kerberos authentication in the JDBC driver. Please see:

Review comment:
       `needed` -> `is needed`




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648218975


   Merged build finished. Test FAILed.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648239051






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646235227


   retest this please


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maropu commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443110503



##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -32,35 +32,29 @@ import org.apache.spark.tags.DockerTest
 
 /**
  * This patch was tested using the Oracle docker. Created this integration suite for the same.
- * The ojdbc6-11.2.0.2.0.jar was to be downloaded from the maven repository. Since there was
- * no jdbc jar available in the maven repository, the jar was downloaded from oracle site
- * manually and installed in the local; thus tested. So, for SparkQA test case run, the
- * ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0)
- * while Spark QA test run.
  *
  * The following would be the steps to test this
  * 1. Build Oracle database in Docker, please refer below link about how to.
  *    https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md
  * 2. export ORACLE_DOCKER_IMAGE_NAME=$ORACLE_DOCKER_IMAGE_NAME
  *    Pull oracle $ORACLE_DOCKER_IMAGE_NAME image - docker pull $ORACLE_DOCKER_IMAGE_NAME
  * 3. Start docker - sudo service docker start
- * 4. Download oracle 11g driver jar and put it in maven local repo:
- *    (com/oracle/ojdbc6/11.2.0.2.0/ojdbc6-11.2.0.2.0.jar)
- * 5. The timeout and interval parameter to be increased from 60,1 to a high value for oracle test
- *    in DockerJDBCIntegrationSuite.scala (Locally tested with 200,200 and executed successfully).
- * 6. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
+ * 4. The timeout and interval parameter to be increased to a high value for oracle test in
+ *     DockerJDBCIntegrationSuite.scala (Locally tested with timeout(20.minutes), interval(1.second)
+ *     and executed successfully).
+ * 5. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"

Review comment:
       nit: add `-Pdocker-integration-tests`




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maropu commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443110374



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"
+
+  override def getConnection(): Connection = {
+    setAuthenticationConfigIfNeeded()
+    UserGroupInformation.loginUserFromKeytabAndReturnUGI(options.principal, options.keytab).doAs(
+      new PrivilegedExceptionAction[Connection]() {
+        override def run(): Connection = {
+          OracleConnectionProvider.super.getConnection()
+        }
+      }
+    )
+  }
+
+  override def getAdditionalProperties(): Properties = {
+    val result = new Properties()
+    // This prop needed to turn on kerberos authentication in the JDBC driver
+    result.put("oracle.net.authentication_services", "(KERBEROS5)");

Review comment:
       How about adding a link here to this configuration?
   https://docs.oracle.com/en/database/oracle/oracle-database/20/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_THIN_NET_AUTHENTICATION_SERVICES




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646127287


   retest it please


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648108082


   **[Test build #124420 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124420/testReport)** for PR 28863 at commit [`e16b4a4`](https://github.com/apache/spark/commit/e16b4a4b355951ad604069dd78ec2c1bb8ee04d2).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-651442483


   Retest this please.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650588395


   Retest this please


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443411936



##########
File path: external/docker-integration-tests/pom.xml
##########
@@ -130,15 +130,9 @@
       <artifactId>postgresql</artifactId>
       <scope>test</scope>
     </dependency>
-    <!-- Oracle ojdbc jar, used for oracle  integration suite for docker testing.
-     See https://github.com/apache/spark/pull/11306 for background on why we need
-     to use a an ojdbc jar for the testcase. The maven dependency here is commented
-     because currently the maven repository does not contain the ojdbc jar mentioned.
-     Once the jar is available in maven, this could be uncommented. -->
-    <dependency>
-      <groupId>com.oracle</groupId>
-      <artifactId>ojdbc6</artifactId>
-      <version>11.2.0.1.0</version>
+    <dependency>
+      <groupId>com.oracle.database.jdbc</groupId>
+      <artifactId>ojdbc8</artifactId>

Review comment:
       Creating a jira + PR to separate it. Thanks.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646114762






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646127287


   retest it please


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r444169639



##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -55,6 +55,7 @@ import org.apache.spark.tags.DockerTest
  *
  * It has been validated with 18.4.0 Express Edition.
  */
+

Review comment:
       Will fix this merge conflict in the next change.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650649738


   Merged build finished. Test FAILed.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maropu commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443105104



##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -69,6 +63,7 @@ class OracleIntegrationSuite extends DockerJDBCIntegrationSuite with SharedSpark
   }
 
   override def dataPreparation(conn: Connection): Unit = {
+    conn.setAutoCommit(false)

Review comment:
       Could you leave the comment in the code side?




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-652039126


   Thanks, I'm looking at this, @MaxGekk and @jiangxb1987 . It's weird because GitHub action passed.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443412982



##########
File path: external/docker-integration-tests/pom.xml
##########
@@ -130,15 +130,9 @@
       <artifactId>postgresql</artifactId>
       <scope>test</scope>
     </dependency>
-    <!-- Oracle ojdbc jar, used for oracle  integration suite for docker testing.
-     See https://github.com/apache/spark/pull/11306 for background on why we need
-     to use a an ojdbc jar for the testcase. The maven dependency here is commented
-     because currently the maven repository does not contain the ojdbc jar mentioned.
-     Once the jar is available in maven, this could be uncommented. -->
-    <dependency>
-      <groupId>com.oracle</groupId>
-      <artifactId>ojdbc6</artifactId>
-      <version>11.2.0.1.0</version>
+    <dependency>
+      <groupId>com.oracle.database.jdbc</groupId>
+      <artifactId>ojdbc8</artifactId>

Review comment:
       When the extract is done coming back to the rest of the comments and processing them...




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r446837956



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {

Review comment:
       As you've pinpointed all other providers suffer from the same issue. In order keep focus I'm intended to file a minor PR to solve them separately. This one fixed here.

##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"
+
+  override def getConnection(): Connection = {
+    setAuthenticationConfigIfNeeded()
+    UserGroupInformation.loginUserFromKeytabAndReturnUGI(options.principal, options.keytab).doAs(
+      new PrivilegedExceptionAction[Connection]() {
+        override def run(): Connection = {
+          OracleConnectionProvider.super.getConnection()
+        }
+      }
+    )
+  }
+
+  override def getAdditionalProperties(): Properties = {
+    val result = new Properties()
+    // This prop needed to turn on kerberos authentication in the JDBC driver. Please see:

Review comment:
       Fixed.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650649535


   **[Test build #124576 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124576/testReport)** for PR 28863 at commit [`e16b4a4`](https://github.com/apache/spark/commit/e16b4a4b355951ad604069dd78ec2c1bb8ee04d2).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maropu commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443104744



##########
File path: external/docker-integration-tests/pom.xml
##########
@@ -130,15 +130,9 @@
       <artifactId>postgresql</artifactId>
       <scope>test</scope>
     </dependency>
-    <!-- Oracle ojdbc jar, used for oracle  integration suite for docker testing.
-     See https://github.com/apache/spark/pull/11306 for background on why we need
-     to use a an ojdbc jar for the testcase. The maven dependency here is commented
-     because currently the maven repository does not contain the ojdbc jar mentioned.
-     Once the jar is available in maven, this could be uncommented. -->
-    <dependency>
-      <groupId>com.oracle</groupId>
-      <artifactId>ojdbc6</artifactId>
-      <version>11.2.0.1.0</version>
+    <dependency>
+      <groupId>com.oracle.database.jdbc</groupId>
+      <artifactId>ojdbc8</artifactId>

Review comment:
       +1; this fix looks separate from this PR.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650589003






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r444176179



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"
+
+  override def getConnection(): Connection = {
+    setAuthenticationConfigIfNeeded()
+    UserGroupInformation.loginUserFromKeytabAndReturnUGI(options.principal, options.keytab).doAs(
+      new PrivilegedExceptionAction[Connection]() {
+        override def run(): Connection = {
+          OracleConnectionProvider.super.getConnection()
+        }
+      }
+    )
+  }
+
+  override def getAdditionalProperties(): Properties = {
+    val result = new Properties()
+    // This prop needed to turn on kerberos authentication in the JDBC driver
+    result.put("oracle.net.authentication_services", "(KERBEROS5)");

Review comment:
       I've changed the version to 19c in order to be consistent w/ the driver version in the pom.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443450278



##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -32,35 +32,29 @@ import org.apache.spark.tags.DockerTest
 
 /**
  * This patch was tested using the Oracle docker. Created this integration suite for the same.
- * The ojdbc6-11.2.0.2.0.jar was to be downloaded from the maven repository. Since there was
- * no jdbc jar available in the maven repository, the jar was downloaded from oracle site
- * manually and installed in the local; thus tested. So, for SparkQA test case run, the
- * ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0)
- * while Spark QA test run.
  *
  * The following would be the steps to test this
  * 1. Build Oracle database in Docker, please refer below link about how to.
  *    https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md
  * 2. export ORACLE_DOCKER_IMAGE_NAME=$ORACLE_DOCKER_IMAGE_NAME
  *    Pull oracle $ORACLE_DOCKER_IMAGE_NAME image - docker pull $ORACLE_DOCKER_IMAGE_NAME
  * 3. Start docker - sudo service docker start
- * 4. Download oracle 11g driver jar and put it in maven local repo:
- *    (com/oracle/ojdbc6/11.2.0.2.0/ojdbc6-11.2.0.2.0.jar)
- * 5. The timeout and interval parameter to be increased from 60,1 to a high value for oracle test
- *    in DockerJDBCIntegrationSuite.scala (Locally tested with 200,200 and executed successfully).
- * 6. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
+ * 4. The timeout and interval parameter to be increased to a high value for oracle test in
+ *     DockerJDBCIntegrationSuite.scala (Locally tested with timeout(20.minutes), interval(1.second)
+ *     and executed successfully).
+ * 5. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"

Review comment:
       I've taken over this into the separate PR.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443445098



##########
File path: external/docker-integration-tests/pom.xml
##########
@@ -130,15 +130,9 @@
       <artifactId>postgresql</artifactId>
       <scope>test</scope>
     </dependency>
-    <!-- Oracle ojdbc jar, used for oracle  integration suite for docker testing.
-     See https://github.com/apache/spark/pull/11306 for background on why we need
-     to use a an ojdbc jar for the testcase. The maven dependency here is commented
-     because currently the maven repository does not contain the ojdbc jar mentioned.
-     Once the jar is available in maven, this could be uncommented. -->
-    <dependency>
-      <groupId>com.oracle</groupId>
-      <artifactId>ojdbc6</artifactId>
-      <version>11.2.0.1.0</version>
+    <dependency>
+      <groupId>com.oracle.database.jdbc</groupId>
+      <artifactId>ojdbc8</artifactId>

Review comment:
       Created SPARK-32049




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maropu commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646958790


   Note: I've checked that `OracleIntegrationSuite` passed in my local Mac env.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646115406






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-651130055


   retest this please


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi edited a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi edited a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-651617129


   @dongjoon-hyun Yes, I've spent almost gross 1 month to make it work w/ `MiniKDC` but no success. Please see my comment [up](https://github.com/apache/spark/pull/28863#pullrequestreview-433415583). Not found any clear statement it must work so I gave up. If somebody can make it work or when active directory will be available in docker we can add it.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646236455






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650589003






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r446319174



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProvider.scala
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+import java.security.PrivilegedExceptionAction
+import java.sql.{Connection, Driver}
+import java.util.Properties
+
+import org.apache.hadoop.security.UserGroupInformation
+
+import org.apache.spark.sql.execution.datasources.jdbc.JDBCOptions
+
+private[sql] class OracleConnectionProvider(driver: Driver, options: JDBCOptions)
+    extends SecureConnectionProvider(driver, options) {
+  override val appEntry: String = "kprb5module"
+
+  override def getConnection(): Connection = {
+    setAuthenticationConfigIfNeeded()
+    UserGroupInformation.loginUserFromKeytabAndReturnUGI(options.principal, options.keytab).doAs(
+      new PrivilegedExceptionAction[Connection]() {
+        override def run(): Connection = {
+          OracleConnectionProvider.super.getConnection()
+        }
+      }
+    )
+  }
+
+  override def getAdditionalProperties(): Properties = {
+    val result = new Properties()
+    // This prop needed to turn on kerberos authentication in the JDBC driver. Please see:
+    // scalastyle:off line.size.limit
+    // https://docs.oracle.com/en/database/oracle/oracle-database/19/jajdb/oracle/jdbc/OracleConnection.html#CONNECTION_PROPERTY_THIN_NET_AUTHENTICATION_SERVICES
+    // scalastyle:on line.size.limit
+    result.put("oracle.net.authentication_services", "(KERBEROS5)");

Review comment:
       +1 for that. Thank you!




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-647729861


   Thank you, @gaborgsomogyi . I reviewed #28893 . We can merge that if the timeout issue is resolved there.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648202177


   Test FAILed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124420/
   Test FAILed.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648237409


   **[Test build #124418 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124418/testReport)** for PR 28863 at commit [`d698fea`](https://github.com/apache/spark/commit/d698fea72374033e3bf3bbba858b6e95e99a88b1).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds the following public classes _(experimental)_:
     * `public final class MapOutputCommitMessage `
     * `sealed trait LogisticRegressionSummary extends ClassificationSummary `
     * `class _ClassificationSummary(JavaWrapper):`
     * `class _TrainingSummary(JavaWrapper):`
     * `class _BinaryClassificationSummary(_ClassificationSummary):`
     * `class LogisticRegressionSummary(_ClassificationSummary):`
     * `class LogisticRegressionTrainingSummary(LogisticRegressionSummary, _TrainingSummary):`
     * `class BinaryLogisticRegressionSummary(_BinaryClassificationSummary,`
     * `case class Hour(child: Expression, timeZoneId: Option[String] = None) extends GetTimeField `
     * `case class Minute(child: Expression, timeZoneId: Option[String] = None) extends GetTimeField `
     * `case class Second(child: Expression, timeZoneId: Option[String] = None) extends GetTimeField `
     * `trait GetDateField extends UnaryExpression with ImplicitCastInputTypes with NullIntolerant `
     * `case class DayOfYear(child: Expression) extends GetDateField `
     * `case class Year(child: Expression) extends GetDateField `
     * `case class YearOfWeek(child: Expression) extends GetDateField `
     * `case class Quarter(child: Expression) extends GetDateField `
     * `case class Month(child: Expression) extends GetDateField `
     * `case class DayOfMonth(child: Expression) extends GetDateField `
     * `case class DayOfWeek(child: Expression) extends GetDateField `
     * `case class WeekDay(child: Expression) extends GetDateField `
     * `case class WeekOfYear(child: Expression) extends GetDateField `
     * `case class CoalesceBucketsInSortMergeJoin(conf: SQLConf) extends Rule[SparkPlan] `
     * `class StateStoreConf(`


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maropu commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r444211273



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProviderSuite.scala
##########
@@ -0,0 +1,28 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+class OracleConnectionProviderSuite extends ConnectionProviderSuiteBase {
+  test("setAuthenticationConfigIfNeeded must set authentication if not set") {

Review comment:
       Ah, I see. But, I felt a a bit less testing for creating a separate test file.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648202154






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648202154


   Merged build finished. Test FAILed.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] maropu commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r443106703



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/OracleConnectionProviderSuite.scala
##########
@@ -0,0 +1,28 @@
+/*
+ * 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.spark.sql.execution.datasources.jdbc.connection
+
+class OracleConnectionProviderSuite extends ConnectionProviderSuiteBase {
+  test("setAuthenticationConfigIfNeeded must set authentication if not set") {

Review comment:
       All the `XXXConnectionProviderSuite` has the almost same test, so could you move it into `ConnectionProviderSuiteBase`?




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648217291


   **[Test build #124425 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124425/testReport)** for PR 28863 at commit [`e16b4a4`](https://github.com/apache/spark/commit/e16b4a4b355951ad604069dd78ec2c1bb8ee04d2).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-651000078


   **[Test build #124640 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124640/testReport)** for PR 28863 at commit [`461eed0`](https://github.com/apache/spark/commit/461eed070d86a7d9d5f8ba536a69d59621faa907).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646313232


   **[Test build #124231 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124231/testReport)** for PR 28863 at commit [`5cc3d0a`](https://github.com/apache/spark/commit/5cc3d0ad2485d60abf0861f5e9192eefd57551cf).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r442333450



##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -32,35 +32,29 @@ import org.apache.spark.tags.DockerTest
 
 /**
  * This patch was tested using the Oracle docker. Created this integration suite for the same.
- * The ojdbc6-11.2.0.2.0.jar was to be downloaded from the maven repository. Since there was
- * no jdbc jar available in the maven repository, the jar was downloaded from oracle site
- * manually and installed in the local; thus tested. So, for SparkQA test case run, the
- * ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0)
- * while Spark QA test run.
  *
  * The following would be the steps to test this
  * 1. Build Oracle database in Docker, please refer below link about how to.
  *    https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md
  * 2. export ORACLE_DOCKER_IMAGE_NAME=$ORACLE_DOCKER_IMAGE_NAME
  *    Pull oracle $ORACLE_DOCKER_IMAGE_NAME image - docker pull $ORACLE_DOCKER_IMAGE_NAME
  * 3. Start docker - sudo service docker start
- * 4. Download oracle 11g driver jar and put it in maven local repo:
- *    (com/oracle/ojdbc6/11.2.0.2.0/ojdbc6-11.2.0.2.0.jar)
- * 5. The timeout and interval parameter to be increased from 60,1 to a high value for oracle test
- *    in DockerJDBCIntegrationSuite.scala (Locally tested with 200,200 and executed successfully).
- * 6. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
+ * 4. The timeout and interval parameter to be increased to a high value for oracle test in
+ *     DockerJDBCIntegrationSuite.scala (Locally tested with timeout(20.minutes), interval(1.second)
+ *     and executed successfully).
+ * 5. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
  *
- * All tests in this suite are ignored because of the dependency with the oracle jar from maven
- * repository.
+ * It has been validated with 18.4.0 Express Edition.

Review comment:
       Actual commands executed:
   ```
   git clone https://github.com/oracle/docker-images.git
   # Head SHA: 3e352a22618070595f823977a0fd1a3a8071a83c
   cd docker-images/OracleDatabase/SingleInstance/dockerfiles
   ./buildDockerImage.sh -v 18.4.0 -x
   export ORACLE_DOCKER_IMAGE_NAME=oracle/database:18.4.0-xe
   # Execute OracleIntegrationSuite manually from Intellij
   ```
   




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-651138851


   Merged build finished. Test FAILed.


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646244722


   cc @HeartSaVioR 


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650640164






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-650640344






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648214699


   retest this please


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-646314228






----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28863:
URL: https://github.com/apache/spark/pull/28863#issuecomment-648217291


   **[Test build #124425 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124425/testReport)** for PR 28863 at commit [`e16b4a4`](https://github.com/apache/spark/commit/e16b4a4b355951ad604069dd78ec2c1bb8ee04d2).


----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on a change in pull request #28863: [SPARK-31336][SQL] Support Oracle Kerberos login in JDBC connector

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #28863:
URL: https://github.com/apache/spark/pull/28863#discussion_r444187826



##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
##########
@@ -32,35 +32,29 @@ import org.apache.spark.tags.DockerTest
 
 /**
  * This patch was tested using the Oracle docker. Created this integration suite for the same.
- * The ojdbc6-11.2.0.2.0.jar was to be downloaded from the maven repository. Since there was
- * no jdbc jar available in the maven repository, the jar was downloaded from oracle site
- * manually and installed in the local; thus tested. So, for SparkQA test case run, the
- * ojdbc jar might be manually placed in the local maven repository(com/oracle/ojdbc6/11.2.0.2.0)
- * while Spark QA test run.
  *
  * The following would be the steps to test this
  * 1. Build Oracle database in Docker, please refer below link about how to.
  *    https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md
  * 2. export ORACLE_DOCKER_IMAGE_NAME=$ORACLE_DOCKER_IMAGE_NAME
  *    Pull oracle $ORACLE_DOCKER_IMAGE_NAME image - docker pull $ORACLE_DOCKER_IMAGE_NAME
  * 3. Start docker - sudo service docker start
- * 4. Download oracle 11g driver jar and put it in maven local repo:
- *    (com/oracle/ojdbc6/11.2.0.2.0/ojdbc6-11.2.0.2.0.jar)
- * 5. The timeout and interval parameter to be increased from 60,1 to a high value for oracle test
- *    in DockerJDBCIntegrationSuite.scala (Locally tested with 200,200 and executed successfully).
- * 6. Run spark test - ./build/sbt "test-only org.apache.spark.sql.jdbc.OracleIntegrationSuite"
+ * 4. The timeout and interval parameter to be increased to a high value for oracle test in
+ *     DockerJDBCIntegrationSuite.scala (Locally tested with timeout(20.minutes), interval(1.second)

Review comment:
       This is not relevant anymore.




----------------------------------------------------------------
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: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org