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 2021/11/29 16:01:31 UTC
[GitHub] [spark] tdg5 opened a new pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
tdg5 opened a new pull request #34745:
URL: https://github.com/apache/spark/pull/34745
Augments the `JdbcConnectionProvider` API such that a provider can indicate that it will need to modify the global security configuration when establishing a connection, and as such, access to the global security configuration should be synchronized. Presently, access to the global security configuration is **always** synchronized and this creates a bottleneck for applications that rely on initiating many JDBC connections simultaneously.
### What changes were proposed in this pull request?
As suggested by @gaborgsomogyi [here](https://github.com/apache/spark/pull/29024/files#r755788709), augments the `JdbcConnectionProvider` API to include a `needsModifySecurityConfiguration` method that can be used by `ConnectionProvider` to determine when `SecurityConfigurationLock.synchronized` is required to avoid race conditions when establishing a JDBC connection.
### Why are the changes needed?
Provides a path forward for working around a significant bottleneck introduced by synchronizing `SecurityConfigurationLock` every time a connection is established. The synchronization isn't always needed and it should be at the discretion of the `JdbcConnectionProvider` to determine when locking is necessary. See [SPARK-37391](https://issues.apache.org/jira/browse/SPARK-37391) or [this thread](https://github.com/apache/spark/pull/29024/files#r754441783).
### Does this PR introduce _any_ user-facing change?
Any existing implementations of `JdbcConnectionProvider` will need to add a definition of `needsModifySecurityConfiguration`. I'm also open to adding a default implementation, but it seemed to me that requiring an explicit implementation of the method was preferable.
A drop-in implementation that would continue the existing behavior is:
```scala
override def needsModifySecurityConfiguration(
driver: Driver,
options: Map[String, String]
): Boolean = true
```
### How was this patch tested?
Unit tests, but I also plan to run a real workflow once I get the initial thumbs up on this implementation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996355089
**[Test build #146306 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146306/testReport)** for PR 34745 at commit [`503eab1`](https://github.com/apache/spark/commit/503eab1ffb4661a5f8dbbb3ebfb3f0a629ea4328).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996427887
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146306/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997196894
**[Test build #146359 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146359/testReport)** for PR 34745 at commit [`3a2b619`](https://github.com/apache/spark/commit/3a2b619e19fb33285cb16c1e0022f9a58c294862).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997218577
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146359/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998939796
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50919/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998895553
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50919/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998973367
**[Test build #146443 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146443/testReport)** for PR 34745 at commit [`a7e9e2e`](https://github.com/apache/spark/commit/a7e9e2e8cbfa79b4900c685c740be8d8a394f0df).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-999077126
I should have asked this earlier; how do backports work? Is there more y'all need from me in that regard? I've checked and the code in this PR doesn't cherry pick cleanly into either `branch-3.1` or `branch-3.2`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r773205207
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MsSqlServerIntegrationSuite.scala
##########
@@ -26,9 +26,10 @@ import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.tags.DockerTest
/**
- * To run this test suite for a specific version (e.g., 2019-CU13-ubuntu-20.04):
+ * To run this test suite for a specific version (e.g., 2019-CU14-ubuntu-20.04):
* {{{
- * ENABLE_DOCKER_INTEGRATION_TESTS=1 MSSQLSERVER_DOCKER_IMAGE_NAME=2019-CU13-ubuntu-20.04
+ * ENABLE_DOCKER_INTEGRATION_TESTS=1
+ * MSSQLSERVER_DOCKER_IMAGE_NAME=mcr.microsoft.com/mssql/server:2019-CU14-ubuntu-20.04
Review comment:
hmm, this image also seems to exist. I'm going to roll that part back, but will fix the docs in another 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996229870
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50769/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996440157
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50785/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-995464104
Have you exexuted the 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996384034
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50776/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-982193018
**[Test build #145734 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145734/testReport)** for PR 34745 at commit [`8a9749c`](https://github.com/apache/spark/commit/8a9749c5947f5e94cacfc36ecee7dbb5a415bc6c).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-982198306
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145734/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r758510360
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/BasicConnectionProvider.scala
##########
@@ -48,4 +48,12 @@ private[jdbc] class BasicConnectionProvider extends JdbcConnectionProvider with
logDebug(s"JDBC connection initiated with URL: ${jdbcOptions.url} and properties: $properties")
driver.connect(jdbcOptions.url, properties)
}
+
+ override def needsModifySecurityConfiguration(
+ driver: Driver,
+ options: Map[String, String]
+ ): Boolean = {
+ val jdbcOptions = new JDBCOptions(options)
+ jdbcOptions.keytab == null || jdbcOptions.principal == null
Review comment:
This could also just return `false`, but I'm kind of confused about what's going on here. [This doc](https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/jdbc/README.md#:~:text=Built%2Din%20CPs%20added%20which%20support%20kerberos%20authentication%20using%20keytab%20and%20principal) suggests that built-in CPs support kerberos auth, but if I understand correctly that should require a `keytab` and ` principal`. I'm inclined to believe that this `JdbcConnectionProvider` doesn't actually support kerberos auth, but maybe I'm misunderstanding.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996470547
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146313/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996470547
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146313/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996407299
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996449013
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50786/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996057873
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50765/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] HyukjinKwon commented on pull request #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996341699
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996401827
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50778/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-982198306
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/145734/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] HyukjinKwon commented on pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-982170951
ok to test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-982193018
**[Test build #145734 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145734/testReport)** for PR 34745 at commit [`8a9749c`](https://github.com/apache/spark/commit/8a9749c5947f5e94cacfc36ecee7dbb5a415bc6c).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] LuciferYang commented on a change in pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r758992879
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/ConnectionProviderSuite.scala
##########
@@ -68,12 +69,20 @@ class ConnectionProviderSuite
override def canHandle(driver: Driver, options: Map[String, String]): Boolean = true
override def getConnection(driver: Driver, options: Map[String, String]): Connection =
throw new RuntimeException()
+ override def needsModifySecurityConfiguration(
Review comment:
Should I write the default method to `JdbcConnectionProvider` (return false) and override it only where necessary
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/ConnectionProviderSuite.scala
##########
@@ -68,12 +69,20 @@ class ConnectionProviderSuite
override def canHandle(driver: Driver, options: Map[String, String]): Boolean = true
override def getConnection(driver: Driver, options: Map[String, String]): Connection =
throw new RuntimeException()
+ override def needsModifySecurityConfiguration(
Review comment:
Should we write the default method to `JdbcConnectionProvider` (return false) and override it only where necessary
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-981800250
@gaborgsomogyi
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] sarutak commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-1000592838
Merging to `master`. Thank you all !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] HyukjinKwon commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r772029246
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcConnectionProvider.scala
##########
@@ -53,12 +53,25 @@ abstract class JdbcConnectionProvider {
def canHandle(driver: Driver, options: Map[String, String]): Boolean
/**
- * Opens connection toward the database. Since global JVM security configuration change may needed
- * this API is called synchronized by `SecurityConfigurationLock` to avoid race.
+ * Opens connection to the database. Since global JVM security configuration change may be
+ * needed this API is called synchronized by `SecurityConfigurationLock` to avoid race when
+ * `modifiesSecurityContext` returns true for the given driver with the given options.
*
* @param driver Java driver which initiates the connection
* @param options Driver options which initiates the connection
* @return a `Connection` object that represents a connection to the URL
*/
def getConnection(driver: Driver, options: Map[String, String]): Connection
+
+ /**
+ * Checks if this connection provider instance needs to modify global security configuration to
+ * handle authentication and thus should synchronize access to the security configuration while
+ * the given driver is initiating a connection with the given options.
+ *
+ * @param driver Java driver which initiates the connection
+ * @param options Driver options which initiates the connection
+ * @return True if the connection provider will need to modify the security configuration when
+ * initiating a connection with the given driver with the given options.
Review comment:
Let's also mention `@since 3.3.0`. Otherwise, looks pretty good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] HyukjinKwon commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r772029590
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcConnectionProvider.scala
##########
@@ -53,12 +53,25 @@ abstract class JdbcConnectionProvider {
def canHandle(driver: Driver, options: Map[String, String]): Boolean
/**
- * Opens connection toward the database. Since global JVM security configuration change may needed
- * this API is called synchronized by `SecurityConfigurationLock` to avoid race.
+ * Opens connection to the database. Since global JVM security configuration change may be
+ * needed this API is called synchronized by `SecurityConfigurationLock` to avoid race when
+ * `modifiesSecurityContext` returns true for the given driver with the given options.
*
* @param driver Java driver which initiates the connection
* @param options Driver options which initiates the connection
* @return a `Connection` object that represents a connection to the URL
*/
def getConnection(driver: Driver, options: Map[String, String]): Connection
+
+ /**
+ * Checks if this connection provider instance needs to modify global security configuration to
+ * handle authentication and thus should synchronize access to the security configuration while
+ * the given driver is initiating a connection with the given options.
+ *
+ * @param driver Java driver which initiates the connection
+ * @param options Driver options which initiates the connection
+ * @return True if the connection provider will need to modify the security configuration when
+ * initiating a connection with the given driver with the given options.
Review comment:
I don't mind backporting it with, for example, `@since 3.2.1` or `@since 3.1.3` if you think this is a serious issue. I will leave this to you too @gaborgsomogyi.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998324277
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50883/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997210694
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50833/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997210694
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50833/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] HyukjinKwon commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998367599
> Sorry, I think this is a dumb question, but I can't seem to find the answer with google. Is there a nightly snapshot build for 3.3 I can download somewhere?
It's not a dumb question :-) - I believe most of people don't know. Here you go: https://repository.apache.org/content/groups/snapshots/org/apache/spark
POM would have to have something like:
```xml
<repository>
<id>apache-snapshot-repo</id>
<name>Apache Snapshot Repository</name>
<url>https://repository.apache.org/content/groups/snapshots</url>
<releases>
<enabled>false</enabled>
</releases>
<snapshots>
<enabled>true</enabled>
</snapshots>
</repository>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 edited a comment on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 edited a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997464112
Something like this I'm guessing?
```
ENABLE_DOCKER_INTEGRATION_TESTS=1 \
POSTGRES_DOCKER_IMAGE_NAME=postgres:14.0 \
MYSQL_DOCKER_IMAGE_NAME=mysql:5.7.36 \
MSSQLSERVER_DOCKER_IMAGE_NAME=microsoft-mssql-server:2019-CU14-ubuntu-20.04 \
MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.13 \
DB2_DOCKER_IMAGE_NAME=ibmcom/db2:11.5.6.0a \
ORACLE_DOCKER_IMAGE_NAME=gvenzl/oracle-xe:latest \
./build/sbt -Pdocker-integration-tests "testOnly *PostgresKrbIntegrationSuite org.apache.spark.sql.jdbc.PostgresIntegrationSuite org.apache.spark.sql.jdbc.MySQLIntegrationSuite org.apache.spark.sql.jdbc.MsSqlServerIntegrationSuite org.apache.spark.sql.jdbc.MariaDBKrbIntegrationSuite *DB2KrbIntegrationSuite org.apache.spark.sql.jdbc.DB2IntegrationSuite org.apache.spark.sql.jdbc.OracleIntegrationSuite"
```
If so, I'm trying that now. Seems like a lib and some referenced docker images are out of date. I'm going to open another PR with those updates so I can play unit test roulette in a context I deem less time sensitive than this one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998044405
**[Test build #146401 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146401/testReport)** for PR 34745 at commit [`f7b6507`](https://github.com/apache/spark/commit/f7b65073da0a129cd3d01999f8978f0962b4b700).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996271926
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146297/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996344443
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50776/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996448365
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50785/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997202204
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50833/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r772477032
##########
File path: project/SparkBuild.scala
##########
@@ -600,7 +600,7 @@ object DockerIntegrationTests {
lazy val settings = Seq(
dependencyOverrides += "com.google.guava" % "guava" % "18.0",
resolvers += "DB2" at "https://app.camunda.com/nexus/content/repositories/public/",
- libraryDependencies += "com.oracle" % "ojdbc6" % "11.2.0.1.0" from "https://app.camunda.com/nexus/content/repositories/public/com/oracle/ojdbc6/11.2.0.1.0/ojdbc6-11.2.0.1.0.jar" // scalastyle:ignore
+ libraryDependencies += "com.oracle" % "ojdbc6" % "11.2.0.3" from "https://app.camunda.com/nexus/content/repositories/public/com/oracle/ojdbc6/11.2.0.3/ojdbc6-11.2.0.3.jar" // scalastyle:ignore
Review comment:
The previously referenced version no longer exists in jfrog, so I updated to the closest similar version that was available. https://camunda.jfrog.io/ui/native/public/com/oracle/ojdbc6
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r772477880
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MsSqlServerIntegrationSuite.scala
##########
@@ -26,9 +26,10 @@ import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.tags.DockerTest
/**
- * To run this test suite for a specific version (e.g., 2019-CU13-ubuntu-20.04):
+ * To run this test suite for a specific version (e.g., 2019-CU14-ubuntu-20.04):
* {{{
- * ENABLE_DOCKER_INTEGRATION_TESTS=1 MSSQLSERVER_DOCKER_IMAGE_NAME=2019-CU13-ubuntu-20.04
+ * ENABLE_DOCKER_INTEGRATION_TESTS=1
+ * MSSQLSERVER_DOCKER_IMAGE_NAME=mcr.microsoft.com/mssql/server:2019-CU14-ubuntu-20.04
Review comment:
Also, this example was missing the repository name, so I added that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-994947924
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146237/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996028715
**[Test build #146293 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146293/testReport)** for PR 34745 at commit [`adc4453`](https://github.com/apache/spark/commit/adc44530e06383df0c6bd082311dc4299aff8a46).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996248640
**[Test build #146297 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146297/testReport)** for PR 34745 at commit [`80e3d8e`](https://github.com/apache/spark/commit/80e3d8eb4693aba5a535a4291fce11b9c4def0a6).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996355089
**[Test build #146306 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146306/testReport)** for PR 34745 at commit [`503eab1`](https://github.com/apache/spark/commit/503eab1ffb4661a5f8dbbb3ebfb3f0a629ea4328).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996412172
**[Test build #146313 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146313/testReport)** for PR 34745 at commit [`4b324b5`](https://github.com/apache/spark/commit/4b324b5146e27521155afa4c4433c73fd00cfcd9).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996425455
**[Test build #146306 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146306/testReport)** for PR 34745 at commit [`503eab1`](https://github.com/apache/spark/commit/503eab1ffb4661a5f8dbbb3ebfb3f0a629ea4328).
* This patch **fails SparkR 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996427290
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50785/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996029291
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146293/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996112136
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50765/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] HyukjinKwon commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-999176671
> I should have asked this earlier; how do backports work? Is there more y'all need from me in that regard? I've checked and the code in this PR doesn't cherry pick cleanly into either `branch-3.1` or `branch-3.2`.
Yeah, it's all as you said. When committers merge, they usually manually resolve conflicts in their local if it's minor. If there's a major conflict, usually they ask the author to create another PR to `branch-3.1` or `branch-3.2`. While we can merge them as is first and you can create backporting PRs, it is often easier to split a PR to multiple PRs that can be backported and not backported.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r772475501
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MariaDBKrbIntegrationSuite.scala
##########
@@ -25,9 +25,9 @@ import org.apache.spark.sql.execution.datasources.jdbc.connection.SecureConnecti
import org.apache.spark.tags.DockerTest
/**
- * To run this test suite for a specific version (e.g., mariadb:10.5.12):
+ * To run this test suite for a specific version (e.g., mariadb:10.5.13):
* {{{
- * ENABLE_DOCKER_INTEGRATION_TESTS=1 MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.12
+ * ENABLE_DOCKER_INTEGRATION_TESTS=1 MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.13
Review comment:
As far as I could tell the referenced image no longer exists in docker hub. https://hub.docker.com/_/mariadb
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998138573
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50876/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 edited a comment on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 edited a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997464112
Something like this I'm guessing?
```
ENABLE_DOCKER_INTEGRATION_TESTS=1 \
POSTGRES_DOCKER_IMAGE_NAME=postgres:14.0 \
MYSQL_DOCKER_IMAGE_NAME=mysql:5.7.36 \
MSSQLSERVER_DOCKER_IMAGE_NAME=mcr.microsoft.com/mssql/server:2019-CU14-ubuntu-20.04 \
MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.13 \
DB2_DOCKER_IMAGE_NAME=ibmcom/db2:11.5.6.0a \
ORACLE_DOCKER_IMAGE_NAME=gvenzl/oracle-xe:latest \
./build/sbt -Pdocker-integration-tests "testOnly *PostgresKrbIntegrationSuite org.apache.spark.sql.jdbc.PostgresIntegrationSuite org.apache.spark.sql.jdbc.MySQLIntegrationSuite org.apache.spark.sql.jdbc.MsSqlServerIntegrationSuite org.apache.spark.sql.jdbc.MariaDBKrbIntegrationSuite *DB2KrbIntegrationSuite org.apache.spark.sql.jdbc.DB2IntegrationSuite org.apache.spark.sql.jdbc.OracleIntegrationSuite"
```
If so, I'm trying that now. Seems like a lib and some referenced docker images are out of date. I'm going to open another PR with those updates so I can play unit test roulette in a context I deem less time sensitive than this one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-982246780
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50204/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r758587271
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/BasicConnectionProvider.scala
##########
@@ -48,4 +48,12 @@ private[jdbc] class BasicConnectionProvider extends JdbcConnectionProvider with
logDebug(s"JDBC connection initiated with URL: ${jdbcOptions.url} and properties: $properties")
driver.connect(jdbcOptions.url, properties)
}
+
+ override def needsModifySecurityConfiguration(
+ driver: Driver,
+ options: Map[String, String]
+ ): Boolean = {
+ val jdbcOptions = new JDBCOptions(options)
+ jdbcOptions.keytab != null && jdbcOptions.principal != null
Review comment:
This could also just return `false`, but I'm kind of confused about what's going on here. [This doc](https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/jdbc/README.md#:~:text=Built%2Din%20CPs%20added%20which%20support%20kerberos%20authentication%20using%20keytab%20and%20principal) suggests that built-in CPs support kerberos auth, but if I understand correctly that should require a `keytab` and `principal`. I'm inclined to believe that this `JdbcConnectionProvider` doesn't actually support kerberos auth, but maybe I'm misunderstanding.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r758551435
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/BasicConnectionProvider.scala
##########
@@ -48,4 +48,12 @@ private[jdbc] class BasicConnectionProvider extends JdbcConnectionProvider with
logDebug(s"JDBC connection initiated with URL: ${jdbcOptions.url} and properties: $properties")
driver.connect(jdbcOptions.url, properties)
}
+
+ override def needsModifySecurityConfiguration(
+ driver: Driver,
+ options: Map[String, String]
+ ): Boolean = {
+ val jdbcOptions = new JDBCOptions(options)
+ jdbcOptions.keytab == null || jdbcOptions.principal == null
Review comment:
Oops, just realized while driving that I need to invert this check. Well fix when I'm back at the keyboard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-995001193
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50711/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996492842
Some of the tests are flaky but we need a green build to merge anything so retest is the way.
Other than that plz update the description to reflect the latest code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997466776
If I identified the correct integration tests, I can confirm they all pass on this branch when run manually. I'm out of time for the moment, so will have to wait to open the PR with the changes needed to make the docker integration tests work, but here's the diff so you can validate I didn't do anything crazy: https://github.com/tdg5/spark/commit/1d6fff5e313eb35541c2c04054b9cf9291c67962
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996377591
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50778/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996305449
**[Test build #146304 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146304/testReport)** for PR 34745 at commit [`503eab1`](https://github.com/apache/spark/commit/503eab1ffb4661a5f8dbbb3ebfb3f0a629ea4328).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996100177
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50765/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996461540
**[Test build #146313 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146313/testReport)** for PR 34745 at commit [`4b324b5`](https://github.com/apache/spark/commit/4b324b5146e27521155afa4c4433c73fd00cfcd9).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997216960
**[Test build #146359 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146359/testReport)** for PR 34745 at commit [`3a2b619`](https://github.com/apache/spark/commit/3a2b619e19fb33285cb16c1e0022f9a58c294862).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996170285
**[Test build #146297 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146297/testReport)** for PR 34745 at commit [`80e3d8e`](https://github.com/apache/spark/commit/80e3d8eb4693aba5a535a4291fce11b9c4def0a6).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996432567
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50786/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997218577
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146359/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996427887
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146306/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r772253745
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcConnectionProvider.scala
##########
@@ -53,12 +53,25 @@ abstract class JdbcConnectionProvider {
def canHandle(driver: Driver, options: Map[String, String]): Boolean
/**
- * Opens connection toward the database. Since global JVM security configuration change may needed
- * this API is called synchronized by `SecurityConfigurationLock` to avoid race.
+ * Opens connection to the database. Since global JVM security configuration change may be
+ * needed this API is called synchronized by `SecurityConfigurationLock` to avoid race when
+ * `modifiesSecurityContext` returns true for the given driver with the given options.
*
* @param driver Java driver which initiates the connection
* @param options Driver options which initiates the connection
* @return a `Connection` object that represents a connection to the URL
*/
def getConnection(driver: Driver, options: Map[String, String]): Connection
+
+ /**
+ * Checks if this connection provider instance needs to modify global security configuration to
+ * handle authentication and thus should synchronize access to the security configuration while
+ * the given driver is initiating a connection with the given options.
+ *
+ * @param driver Java driver which initiates the connection
+ * @param options Driver options which initiates the connection
+ * @return True if the connection provider will need to modify the security configuration when
+ * initiating a connection with the given driver with the given options.
Review comment:
I think it worth a backport since it's a performance regression issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997803926
The changeset looks good, except the `@since...` stuff. @tdg5 just to double check the last and most important thing do you have some performance numbers how much is it improved in unsecure case?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998361189
**[Test build #146401 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146401/testReport)** for PR 34745 at commit [`f7b6507`](https://github.com/apache/spark/commit/f7b65073da0a129cd3d01999f8978f0962b4b700).
* This patch **fails from timeout after a configured wait of `500m`**.
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998054854
@HyukjinKwon , as far as I can tell, it seems I succeeded in properly enabling GitHub actions for my fork 👍🏻 If you still see otherwise, please let me know.
@gaborgsomogyi , can you offer some thoughts on what kind of performance stats are you looking for? Since this is a threading/locking issue, the speed up of being able to open more connections simultaneously can be kind of unlimited depending on how many threads are allocated to doing that kind of work. I don't mind trying to come up with some numbers, but I'm not sure what makes sense to use as a baseline.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998320835
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50883/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] sarutak commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r773080583
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MariaDBKrbIntegrationSuite.scala
##########
@@ -25,9 +25,9 @@ import org.apache.spark.sql.execution.datasources.jdbc.connection.SecureConnecti
import org.apache.spark.tags.DockerTest
/**
- * To run this test suite for a specific version (e.g., mariadb:10.5.12):
+ * To run this test suite for a specific version (e.g., mariadb:10.5.13):
* {{{
- * ENABLE_DOCKER_INTEGRATION_TESTS=1 MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.12
+ * ENABLE_DOCKER_INTEGRATION_TESTS=1 MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.13
Review comment:
@tdg5 If you'd like to change here, please change [this part](https://github.com/apache/spark/blob/e2eaffec48e30ba95b7663c04a00970ae1b3941d/external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MariaDBKrbIntegrationSuite.scala#L41) too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-982246780
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50204/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-982542753
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-994913101
**[Test build #146237 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146237/testReport)** for PR 34745 at commit [`5666f9e`](https://github.com/apache/spark/commit/5666f9ea8c4430bbd1adbf85f8a42e6d360384e6).
* This patch **fails MiMa 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-994899351
**[Test build #146237 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146237/testReport)** for PR 34745 at commit [`5666f9e`](https://github.com/apache/spark/commit/5666f9ea8c4430bbd1adbf85f8a42e6d360384e6).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-982542753
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-995993280
Mima check has been fixed (assuming adding an exclude is the appropriate solution). Any idea if the kubernetes failures are something to worry about? I can't tell from the output if that's related to my change. Seems far removed from what I can tell, but it can be hard to tell. I've rebased from master, so perhaps that will help also.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996029291
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146293/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996241178
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50769/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 edited a comment on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 edited a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997466776
If I identified the correct integration tests, I can confirm they all pass on this branch when run manually. I'm out of time for the moment, so will have to wait to open the PR with the changes needed to make the docker integration tests work, but here's the diff so you can validate I didn't do anything crazy: https://github.com/tdg5/spark/commit/1d6fff5e313eb35541c2c04054b9cf9291c67962
@gaborgsomogyi
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] HyukjinKwon commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997504813
@tdg5, thanks i think it's getting closer to be merged. but just to be sure, would you mind enabling GitHub Actions in your forked repository? See also https://github.com/apache/spark/pull/34745/checks?check_run_id=4569440425. Jenkins will be retired soon, and we're currently running GitHub Actions for tests (that actually covers more correctly).
I haven't followed the changes and issue here to be honest so I will leave this to @gaborgsomogyi.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996398162
**[Test build #146304 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146304/testReport)** for PR 34745 at commit [`503eab1`](https://github.com/apache/spark/commit/503eab1ffb4661a5f8dbbb3ebfb3f0a629ea4328).
* This patch **fails SparkR 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 edited a comment on pull request #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 edited a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996283273
I don't think the `org.apache.spark.sql.streaming.StreamingQueryManagerSuite` failure relates to my change. The test passes for me locally, so maybe an inconsistent test. I've rebased on master again to kick off a new build.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996271926
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146297/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998939456
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50919/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998138573
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50876/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998350898
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146408/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] sarutak commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r773080583
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MariaDBKrbIntegrationSuite.scala
##########
@@ -25,9 +25,9 @@ import org.apache.spark.sql.execution.datasources.jdbc.connection.SecureConnecti
import org.apache.spark.tags.DockerTest
/**
- * To run this test suite for a specific version (e.g., mariadb:10.5.12):
+ * To run this test suite for a specific version (e.g., mariadb:10.5.13):
* {{{
- * ENABLE_DOCKER_INTEGRATION_TESTS=1 MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.12
+ * ENABLE_DOCKER_INTEGRATION_TESTS=1 MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.13
Review comment:
@tdg5 If you'd like to change here, please change [this part](https://github.com/apache/spark/pull/34745/files#diff-cb12fa58307c2155fe2156d347d7a7917f1c755c34488a741dfb06653f83f364L41) too.
##########
File path: project/SparkBuild.scala
##########
@@ -600,7 +600,7 @@ object DockerIntegrationTests {
lazy val settings = Seq(
dependencyOverrides += "com.google.guava" % "guava" % "18.0",
resolvers += "DB2" at "https://app.camunda.com/nexus/content/repositories/public/",
- libraryDependencies += "com.oracle" % "ojdbc6" % "11.2.0.1.0" from "https://app.camunda.com/nexus/content/repositories/public/com/oracle/ojdbc6/11.2.0.1.0/ojdbc6-11.2.0.1.0.jar" // scalastyle:ignore
+ libraryDependencies += "com.oracle" % "ojdbc6" % "11.2.0.3" from "https://app.camunda.com/nexus/content/repositories/public/com/oracle/ojdbc6/11.2.0.3/ojdbc6-11.2.0.3.jar" // scalastyle:ignore
Review comment:
Hmm, I'm confused about this part. Do we need this dependency setting here?
`pom.xml` has the setting for `ojdbc8`.
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MsSqlServerIntegrationSuite.scala
##########
@@ -26,9 +26,10 @@ import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.tags.DockerTest
/**
- * To run this test suite for a specific version (e.g., 2019-CU13-ubuntu-20.04):
+ * To run this test suite for a specific version (e.g., 2019-CU14-ubuntu-20.04):
* {{{
- * ENABLE_DOCKER_INTEGRATION_TESTS=1 MSSQLSERVER_DOCKER_IMAGE_NAME=2019-CU13-ubuntu-20.04
+ * ENABLE_DOCKER_INTEGRATION_TESTS=1
+ * MSSQLSERVER_DOCKER_IMAGE_NAME=mcr.microsoft.com/mssql/server:2019-CU14-ubuntu-20.04
Review comment:
Also, please change [this part](https://github.com/apache/spark/pull/34745/files#diff-cb12fa58307c2155fe2156d347d7a7917f1c755c34488a741dfb06653f83f364L40) too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-981800777
This is not a work in progress as far as I am concerned/aware, but I marked it as a WIP until @gaborgsomogyi can give it an initial thumbs up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r758512327
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcConnectionProvider.scala
##########
@@ -53,12 +53,25 @@ abstract class JdbcConnectionProvider {
def canHandle(driver: Driver, options: Map[String, String]): Boolean
/**
- * Opens connection toward the database. Since global JVM security configuration change may needed
- * this API is called synchronized by `SecurityConfigurationLock` to avoid race.
+ * Opens connection to the database. Since global JVM security configuration change may be
+ * needed this API is called synchronized by `SecurityConfigurationLock` to avoid race when
+ * `needsModifySecurityConfiguration` returns true for the given driver with the given options.
*
* @param driver Java driver which initiates the connection
* @param options Driver options which initiates the connection
* @return a `Connection` object that represents a connection to the URL
*/
def getConnection(driver: Driver, options: Map[String, String]): Connection
+
+ /**
+ * Checks if this connection provider instance needs to modify global security configuration to
+ * handle authentication and thus should synchronize access to the security configuration while
+ * the given driver is initiating a connection with the given options.
+ *
+ * @param driver Java driver which initiates the connection
+ * @param options Driver options which initiates the connection
+ * @return True if the connection provider will need to modify the security configuration when
+ * initiating a connection with the given driver with the given options.
+ */
+ def needsModifySecurityConfiguration(driver: Driver, options: Map[String, String]): Boolean
Review comment:
Also open to a default implementation, but seemed better to force implementers to explicitly implement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r758587271
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/BasicConnectionProvider.scala
##########
@@ -48,4 +48,12 @@ private[jdbc] class BasicConnectionProvider extends JdbcConnectionProvider with
logDebug(s"JDBC connection initiated with URL: ${jdbcOptions.url} and properties: $properties")
driver.connect(jdbcOptions.url, properties)
}
+
+ override def needsModifySecurityConfiguration(
+ driver: Driver,
+ options: Map[String, String]
+ ): Boolean = {
+ val jdbcOptions = new JDBCOptions(options)
+ jdbcOptions.keytab != null && jdbcOptions.principal != null
Review comment:
This could also just return `false` (based on the logic in `canHandle`), but I'm kind of confused about what's going on here. [This doc](https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/jdbc/README.md#:~:text=Built%2Din%20CPs%20added%20which%20support%20kerberos%20authentication%20using%20keytab%20and%20principal) suggests that built-in CPs support kerberos auth, but if I understand correctly that should require a `keytab` and `principal`. I'm inclined to believe that this `JdbcConnectionProvider` doesn't actually support kerberos auth, but maybe I'm misunderstanding.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-982211056
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50204/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996448365
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50785/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997464112
Something like this I'm guessing?
```
ENABLE_DOCKER_INTEGRATION_TESTS=1 \
POSTGRES_DOCKER_IMAGE_NAME=postgres:14.0 \
MYSQL_DOCKER_IMAGE_NAME=mysql:5.7.36 \
MSSQLSERVER_DOCKER_IMAGE_NAME=2019-CU13-ubuntu-20.04 \
MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.12 \
DB2_DOCKER_IMAGE_NAME=ibmcom/db2:11.5.6.0a \
ORACLE_DOCKER_IMAGE_NAME=gvenzl/oracle-xe:latest \
./build/sbt -mem 4096 -Pdocker-integration-tests "testOnly *PostgresKrbIntegrationSuite org.apache.spark.sql.jdbc.PostgresIntegrationSuite org.apache.spark.sql.jdbc.MySQLIntegrationSuite org.apache.spark.sql.jdbc.MsSqlServerIntegrationSuite org.apache.spark.sql.jdbc.MariaDBKrbIntegrationSuite *DB2KrbIntegrationSuite org.apache.spark.sql.jdbc.DB2IntegrationSuite org.apache.spark.sql.jdbc.OracleIntegrationSuite"
```
If so, I'm trying that now. Seems like a lib and some referenced docker images are out of date. I'm going to open another PR with those updates so I can play unit test roulette in a context I deem less time sensitive than this one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998344983
**[Test build #146408 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146408/testReport)** for PR 34745 at commit [`4aac540`](https://github.com/apache/spark/commit/4aac54032ead4dd66f94de7726951aec91488a18).
* 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998256819
**[Test build #146408 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146408/testReport)** for PR 34745 at commit [`4aac540`](https://github.com/apache/spark/commit/4aac54032ead4dd66f94de7726951aec91488a18).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] sarutak commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r773081483
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MsSqlServerIntegrationSuite.scala
##########
@@ -26,9 +26,10 @@ import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.tags.DockerTest
/**
- * To run this test suite for a specific version (e.g., 2019-CU13-ubuntu-20.04):
+ * To run this test suite for a specific version (e.g., 2019-CU14-ubuntu-20.04):
* {{{
- * ENABLE_DOCKER_INTEGRATION_TESTS=1 MSSQLSERVER_DOCKER_IMAGE_NAME=2019-CU13-ubuntu-20.04
+ * ENABLE_DOCKER_INTEGRATION_TESTS=1
+ * MSSQLSERVER_DOCKER_IMAGE_NAME=mcr.microsoft.com/mssql/server:2019-CU14-ubuntu-20.04
Review comment:
Also, please change the following part too.
https://github.com/apache/spark/pull/34745/files#diff-cb12fa58307c2155fe2156d347d7a7917f1c755c34488a741dfb06653f83f364L40
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998977097
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146443/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r773204775
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MariaDBKrbIntegrationSuite.scala
##########
@@ -25,9 +25,9 @@ import org.apache.spark.sql.execution.datasources.jdbc.connection.SecureConnecti
import org.apache.spark.tags.DockerTest
/**
- * To run this test suite for a specific version (e.g., mariadb:10.5.12):
+ * To run this test suite for a specific version (e.g., mariadb:10.5.13):
* {{{
- * ENABLE_DOCKER_INTEGRATION_TESTS=1 MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.12
+ * ENABLE_DOCKER_INTEGRATION_TESTS=1 MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.13
Review comment:
I'm going to roll this back since it is not required; sorry for the confusion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r773205673
##########
File path: project/SparkBuild.scala
##########
@@ -600,7 +600,7 @@ object DockerIntegrationTests {
lazy val settings = Seq(
dependencyOverrides += "com.google.guava" % "guava" % "18.0",
resolvers += "DB2" at "https://app.camunda.com/nexus/content/repositories/public/",
- libraryDependencies += "com.oracle" % "ojdbc6" % "11.2.0.1.0" from "https://app.camunda.com/nexus/content/repositories/public/com/oracle/ojdbc6/11.2.0.1.0/ojdbc6-11.2.0.1.0.jar" // scalastyle:ignore
+ libraryDependencies += "com.oracle" % "ojdbc6" % "11.2.0.3" from "https://app.camunda.com/nexus/content/repositories/public/com/oracle/ojdbc6/11.2.0.3/ojdbc6-11.2.0.3.jar" // scalastyle:ignore
Review comment:
I'm going to pull this into another PR for the sake of simplifying any backporting. I will update with a link shortly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998939796
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50919/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996196548
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50769/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998086570
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50876/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996283273
Any trick to re-run tests? The spark test that failed in Jenkins passes for me locally. Maybe a transient failure. No rush, I'll force push something if I don't hear back.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996412172
**[Test build #146313 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146313/testReport)** for PR 34745 at commit [`4b324b5`](https://github.com/apache/spark/commit/4b324b5146e27521155afa4c4433c73fd00cfcd9).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998044405
**[Test build #146401 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146401/testReport)** for PR 34745 at commit [`f7b6507`](https://github.com/apache/spark/commit/f7b65073da0a129cd3d01999f8978f0962b4b700).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-982198277
**[Test build #145734 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/145734/testReport)** for PR 34745 at commit [`8a9749c`](https://github.com/apache/spark/commit/8a9749c5947f5e94cacfc36ecee7dbb5a415bc6c).
* This patch **fails MiMa 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r758551435
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/BasicConnectionProvider.scala
##########
@@ -48,4 +48,12 @@ private[jdbc] class BasicConnectionProvider extends JdbcConnectionProvider with
logDebug(s"JDBC connection initiated with URL: ${jdbcOptions.url} and properties: $properties")
driver.connect(jdbcOptions.url, properties)
}
+
+ override def needsModifySecurityConfiguration(
+ driver: Driver,
+ options: Map[String, String]
+ ): Boolean = {
+ val jdbcOptions = new JDBCOptions(options)
+ jdbcOptions.keytab == null || jdbcOptions.principal == null
Review comment:
Oops, just realized while driving that I need to invert this check. Well fix when I'm back at the keyboard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-999056220
The updated changes related to running docker integration tests can now be found over here: https://github.com/apache/spark/pull/34979
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-995001193
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50711/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-994947924
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146237/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-994882220
@gaborgsomogyi I've updated the PR to include your suggestions (really just a rename and a comment on why `BasicConnectionProvider` can always return `false`). Have another look when you have a chance, 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-994936499
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50711/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996241178
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50769/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996170285
**[Test build #146297 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146297/testReport)** for PR 34745 at commit [`80e3d8e`](https://github.com/apache/spark/commit/80e3d8eb4693aba5a535a4291fce11b9c4def0a6).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r772476090
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MsSqlServerIntegrationSuite.scala
##########
@@ -26,9 +26,10 @@ import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.tags.DockerTest
/**
- * To run this test suite for a specific version (e.g., 2019-CU13-ubuntu-20.04):
+ * To run this test suite for a specific version (e.g., 2019-CU14-ubuntu-20.04):
* {{{
- * ENABLE_DOCKER_INTEGRATION_TESTS=1 MSSQLSERVER_DOCKER_IMAGE_NAME=2019-CU13-ubuntu-20.04
+ * ENABLE_DOCKER_INTEGRATION_TESTS=1
+ * MSSQLSERVER_DOCKER_IMAGE_NAME=mcr.microsoft.com/mssql/server:2019-CU14-ubuntu-20.04
Review comment:
As far as I could tell the referenced image needs to be pulled from mcr.microsoft.com and the previously referenced image doesn't seem to exist anymore. https://hub.docker.com/_/microsoft-mssql-server
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r772475501
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MariaDBKrbIntegrationSuite.scala
##########
@@ -25,9 +25,9 @@ import org.apache.spark.sql.execution.datasources.jdbc.connection.SecureConnecti
import org.apache.spark.tags.DockerTest
/**
- * To run this test suite for a specific version (e.g., mariadb:10.5.12):
+ * To run this test suite for a specific version (e.g., mariadb:10.5.13):
* {{{
- * ENABLE_DOCKER_INTEGRATION_TESTS=1 MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.12
+ * ENABLE_DOCKER_INTEGRATION_TESTS=1 MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.13
Review comment:
As far as I could tell the referenced image no longer exists in docker hub.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] HyukjinKwon commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r772747432
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MariaDBKrbIntegrationSuite.scala
##########
@@ -25,9 +25,9 @@ import org.apache.spark.sql.execution.datasources.jdbc.connection.SecureConnecti
import org.apache.spark.tags.DockerTest
/**
- * To run this test suite for a specific version (e.g., mariadb:10.5.12):
+ * To run this test suite for a specific version (e.g., mariadb:10.5.13):
* {{{
- * ENABLE_DOCKER_INTEGRATION_TESTS=1 MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.12
+ * ENABLE_DOCKER_INTEGRATION_TESTS=1 MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.13
Review comment:
cc @sarutak
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] HyukjinKwon commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r772747533
##########
File path: project/SparkBuild.scala
##########
@@ -600,7 +600,7 @@ object DockerIntegrationTests {
lazy val settings = Seq(
dependencyOverrides += "com.google.guava" % "guava" % "18.0",
resolvers += "DB2" at "https://app.camunda.com/nexus/content/repositories/public/",
- libraryDependencies += "com.oracle" % "ojdbc6" % "11.2.0.1.0" from "https://app.camunda.com/nexus/content/repositories/public/com/oracle/ojdbc6/11.2.0.1.0/ojdbc6-11.2.0.1.0.jar" // scalastyle:ignore
+ libraryDependencies += "com.oracle" % "ojdbc6" % "11.2.0.3" from "https://app.camunda.com/nexus/content/repositories/public/com/oracle/ojdbc6/11.2.0.3/ojdbc6-11.2.0.3.jar" // scalastyle:ignore
Review comment:
cc @sarutak FYI
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MsSqlServerIntegrationSuite.scala
##########
@@ -26,9 +26,10 @@ import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.tags.DockerTest
/**
- * To run this test suite for a specific version (e.g., 2019-CU13-ubuntu-20.04):
+ * To run this test suite for a specific version (e.g., 2019-CU14-ubuntu-20.04):
* {{{
- * ENABLE_DOCKER_INTEGRATION_TESTS=1 MSSQLSERVER_DOCKER_IMAGE_NAME=2019-CU13-ubuntu-20.04
+ * ENABLE_DOCKER_INTEGRATION_TESTS=1
+ * MSSQLSERVER_DOCKER_IMAGE_NAME=mcr.microsoft.com/mssql/server:2019-CU14-ubuntu-20.04
Review comment:
cc @sarutak FYI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996384034
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50776/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996449013
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50786/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996449000
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50786/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997341374
There are several manual integration tests which cover this feature, have you executed them 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-982230090
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50204/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r758508360
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcConnectionProvider.scala
##########
@@ -53,12 +53,25 @@ abstract class JdbcConnectionProvider {
def canHandle(driver: Driver, options: Map[String, String]): Boolean
/**
- * Opens connection toward the database. Since global JVM security configuration change may needed
- * this API is called synchronized by `SecurityConfigurationLock` to avoid race.
+ * Opens connection to the database. Since global JVM security configuration change may be
+ * needed this API is called synchronized by `SecurityConfigurationLock` to avoid race when
+ * `needsModifySecurityConfiguration` returns true for the given driver with the given options.
*
* @param driver Java driver which initiates the connection
* @param options Driver options which initiates the connection
* @return a `Connection` object that represents a connection to the URL
*/
def getConnection(driver: Driver, options: Map[String, String]): Connection
+
+ /**
+ * Checks if this connection provider instance needs to modify global security configuration to
+ * handle authentication and thus should synchronize access to the security configuration while
+ * the given driver is initiating a connection with the given options.
+ *
+ * @param driver Java driver which initiates the connection
+ * @param options Driver options which initiates the connection
+ * @return True if the connection provider will need to modify the security configuration when
+ * initiating a connection with the given driver with the given options.
+ */
+ def needsModifySecurityConfiguration(driver: Driver, options: Map[String, String]): Boolean
Review comment:
I'm open to other naming ideas.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r758510360
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/BasicConnectionProvider.scala
##########
@@ -48,4 +48,12 @@ private[jdbc] class BasicConnectionProvider extends JdbcConnectionProvider with
logDebug(s"JDBC connection initiated with URL: ${jdbcOptions.url} and properties: $properties")
driver.connect(jdbcOptions.url, properties)
}
+
+ override def needsModifySecurityConfiguration(
+ driver: Driver,
+ options: Map[String, String]
+ ): Boolean = {
+ val jdbcOptions = new JDBCOptions(options)
+ jdbcOptions.keytab == null || jdbcOptions.principal == null
Review comment:
This could also just return `false`, but I'm kind of confused about what's going on here. [This doc](https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/jdbc/README.md#:~:text=Built%2Din%20CPs%20added%20which%20support%20kerberos%20authentication%20using%20keytab%20and%20principal) suggests that built-in CPs support kerberos auth, but if I understand correctly that should require a `keytab` and ` principal`. I'm inclined to believe that this `JdbcConnectionProvider` doesn't actually support kerberos auth, but maybe I'm misunderstanding.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] sarutak commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r773081483
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MsSqlServerIntegrationSuite.scala
##########
@@ -26,9 +26,10 @@ import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.tags.DockerTest
/**
- * To run this test suite for a specific version (e.g., 2019-CU13-ubuntu-20.04):
+ * To run this test suite for a specific version (e.g., 2019-CU14-ubuntu-20.04):
* {{{
- * ENABLE_DOCKER_INTEGRATION_TESTS=1 MSSQLSERVER_DOCKER_IMAGE_NAME=2019-CU13-ubuntu-20.04
+ * ENABLE_DOCKER_INTEGRATION_TESTS=1
+ * MSSQLSERVER_DOCKER_IMAGE_NAME=mcr.microsoft.com/mssql/server:2019-CU14-ubuntu-20.04
Review comment:
Also, please change [this part](https://github.com/apache/spark/blob/e2eaffec48e30ba95b7663c04a00970ae1b3941d/external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MsSqlServerIntegrationSuite.scala#L40) too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998977097
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146443/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r759389734
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/ConnectionProviderSuite.scala
##########
@@ -68,12 +69,20 @@ class ConnectionProviderSuite
override def canHandle(driver: Driver, options: Map[String, String]): Boolean = true
override def getConnection(driver: Driver, options: Map[String, String]): Connection =
throw new RuntimeException()
+ override def needsModifySecurityConfiguration(
Review comment:
I'm definitely open to this, but defer to the community on whether that is appropriate or not.
It's worth noting that a default of `false` seems like the right answer to me, but Spark versions 3.1+ implicitly have a default of `true`.
I think a default of `false` is sensible, but it would change what is currently the default behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on a change in pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r758511777
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/ConnectionProviderSuite.scala
##########
@@ -107,12 +124,50 @@ class ConnectionProviderSuite
assert(providerBase.create(mock[Driver], Map.empty, Some("test2")).isInstanceOf[Connection])
}
+ test("Synchronize on SecurityConfigurationLock when the specified connection provider needs") {
+ val provider1 = new JdbcConnectionProvider() {
+ override val name: String = "test1"
+ override def canHandle(driver: Driver, options: Map[String, String]): Boolean = true
+ override def getConnection(driver: Driver, options: Map[String, String]): Connection = {
+ assert(Thread.holdsLock(SecurityConfigurationLock))
Review comment:
Open to better ideas on testing that the lock was acquired or not. Could change it to an actual lock, but preferred to avoid the downstream repercussions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996112136
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50765/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996029261
**[Test build #146293 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146293/testReport)** for PR 34745 at commit [`adc4453`](https://github.com/apache/spark/commit/adc44530e06383df0c6bd082311dc4299aff8a46).
* This patch **fails Scala style 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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996305449
**[Test build #146304 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146304/testReport)** for PR 34745 at commit [`503eab1`](https://github.com/apache/spark/commit/503eab1ffb4661a5f8dbbb3ebfb3f0a629ea4328).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996411888
SparkR test failure doesn't seem to relate to my commit... Rebased from master to force another build.
SparkR test failure seems to have been an ephemeral thing?
```
* installing the package to build vignettes
* creating vignettes ...Warning in file(con, "r") :
cannot open file '/tmp/RtmpTxszZ3/xshell185ef769eca3b6': No such file or directory
Error in file(con, "r") : cannot open the connection
Error in setwd(owd) : cannot change working directory
Execution halted
sh: 0: getcwd() failed: No such file or directory
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997196894
**[Test build #146359 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146359/testReport)** for PR 34745 at commit [`3a2b619`](https://github.com/apache/spark/commit/3a2b619e19fb33285cb16c1e0022f9a58c294862).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997218616
Wahoo! All tests passed on the last build. Well, not the k8s integration tests, but I don't get the impression those are blocking. 🎉
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998130446
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50876/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998375381
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146401/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997435816
> There are several manual integration tests which cover this feature, have you executed them manually?
No, I was unaware of those. Any guidance available for running those? I will look into it next time I'm at a keyboard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998375381
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146401/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] sarutak commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r773079121
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MsSqlServerIntegrationSuite.scala
##########
@@ -26,9 +26,10 @@ import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.tags.DockerTest
/**
- * To run this test suite for a specific version (e.g., 2019-CU13-ubuntu-20.04):
+ * To run this test suite for a specific version (e.g., 2019-CU14-ubuntu-20.04):
* {{{
- * ENABLE_DOCKER_INTEGRATION_TESTS=1 MSSQLSERVER_DOCKER_IMAGE_NAME=2019-CU13-ubuntu-20.04
+ * ENABLE_DOCKER_INTEGRATION_TESTS=1
+ * MSSQLSERVER_DOCKER_IMAGE_NAME=mcr.microsoft.com/mssql/server:2019-CU14-ubuntu-20.04
Review comment:
@tdg5 Thank you for fixing the example. Could you fix the example in `v2/MsSqlServerIntegrationSite` too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] HyukjinKwon commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998368103
But I think current state of CI seems fine enough to go and merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998350898
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/146408/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998846162
**[Test build #146443 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146443/testReport)** for PR 34745 at commit [`a7e9e2e`](https://github.com/apache/spark/commit/a7e9e2e8cbfa79b4900c685c740be8d8a394f0df).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998287927
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50883/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996407299
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 edited a comment on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 edited a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-999828289
branch-3.1 PR is here: https://github.com/apache/spark/pull/34988
branch-3.2 PR is here: https://github.com/apache/spark/pull/34989
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r766593758
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/ConnectionProviderSuite.scala
##########
@@ -68,12 +69,20 @@ class ConnectionProviderSuite
override def canHandle(driver: Driver, options: Map[String, String]): Boolean = true
override def getConnection(driver: Driver, options: Map[String, String]): Connection =
throw new RuntimeException()
+ override def needsModifySecurityConfiguration(
Review comment:
Bad idea to add default implementation(at least within the trait). Such case a user may think it's not important part of the provider and will work with the default. This is not true for all the secure providers.
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcConnectionProvider.scala
##########
@@ -53,12 +53,25 @@ abstract class JdbcConnectionProvider {
def canHandle(driver: Driver, options: Map[String, String]): Boolean
/**
- * Opens connection toward the database. Since global JVM security configuration change may needed
- * this API is called synchronized by `SecurityConfigurationLock` to avoid race.
+ * Opens connection to the database. Since global JVM security configuration change may be
+ * needed this API is called synchronized by `SecurityConfigurationLock` to avoid race when
+ * `needsModifySecurityConfiguration` returns true for the given driver with the given options.
*
* @param driver Java driver which initiates the connection
* @param options Driver options which initiates the connection
* @return a `Connection` object that represents a connection to the URL
*/
def getConnection(driver: Driver, options: Map[String, String]): Connection
+
+ /**
+ * Checks if this connection provider instance needs to modify global security configuration to
+ * handle authentication and thus should synchronize access to the security configuration while
+ * the given driver is initiating a connection with the given options.
+ *
+ * @param driver Java driver which initiates the connection
+ * @param options Driver options which initiates the connection
+ * @return True if the connection provider will need to modify the security configuration when
+ * initiating a connection with the given driver with the given options.
+ */
+ def needsModifySecurityConfiguration(driver: Driver, options: Map[String, String]): Boolean
Review comment:
Maybe we can call it `modifiesSecurityContext`.
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/connection/BasicConnectionProvider.scala
##########
@@ -48,4 +48,12 @@ private[jdbc] class BasicConnectionProvider extends JdbcConnectionProvider with
logDebug(s"JDBC connection initiated with URL: ${jdbcOptions.url} and properties: $properties")
driver.connect(jdbcOptions.url, properties)
}
+
+ override def needsModifySecurityConfiguration(
+ driver: Driver,
+ options: Map[String, String]
+ ): Boolean = {
+ val jdbcOptions = new JDBCOptions(options)
+ jdbcOptions.keytab != null && jdbcOptions.principal != null
Review comment:
`BasicConnectionProvider` is the default unsecure connection provider so return false is good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-981792830
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 removed a comment on pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-981800250
@gaborgsomogyi
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-981792830
Can one of the admins verify this patch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-994899351
**[Test build #146237 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146237/testReport)** for PR 34745 at commit [`5666f9e`](https://github.com/apache/spark/commit/5666f9ea8c4430bbd1adbf85f8a42e6d360384e6).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-994977618
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50711/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-995465453
+ plz fix mima cheks...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [WIP][SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-989999406
Hello friends! Anything I can do to help move this PR forward? Presently my ability to upgrade past spark 3.0.x is blocked by the change in functionality that this PR aims to improve.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] HyukjinKwon commented on pull request #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996341981
@tdg5 mind taking a look at https://github.com/apache/spark/pull/34745/checks?check_run_id=4554725898 and enable Github Actions in your forked repository? Apache Spark repo uses the GitHub Actions resources of forked repository to run the build.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996382532
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50776/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997196882
Description updated and another round of tests is underway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997208552
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/50833/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996028715
**[Test build #146293 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146293/testReport)** for PR 34745 at commit [`adc4453`](https://github.com/apache/spark/commit/adc44530e06383df0c6bd082311dc4299aff8a46).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [SPARK-37391][SQL] JdbcConnectionProvider must indicate if it needs lock
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-996156171
I think the k8s integration tests are generally borked. If you happen to know that's not true, let me know and I can investigate further.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] sarutak closed pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
sarutak closed pull request #34745:
URL: https://github.com/apache/spark/pull/34745
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 edited a comment on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 edited a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-997464112
Something like this I'm guessing?
```
ENABLE_DOCKER_INTEGRATION_TESTS=1 \
POSTGRES_DOCKER_IMAGE_NAME=postgres:14.0 \
MYSQL_DOCKER_IMAGE_NAME=mysql:5.7.36 \
MSSQLSERVER_DOCKER_IMAGE_NAME=microsoft-mssql-server:2019-CU14-ubuntu-20.04 \
MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.13 \
DB2_DOCKER_IMAGE_NAME=ibmcom/db2:11.5.6.0a \
ORACLE_DOCKER_IMAGE_NAME=gvenzl/oracle-xe:latest \
./build/sbt -mem 4096 -Pdocker-integration-tests "testOnly *PostgresKrbIntegrationSuite org.apache.spark.sql.jdbc.PostgresIntegrationSuite org.apache.spark.sql.jdbc.MySQLIntegrationSuite org.apache.spark.sql.jdbc.MsSqlServerIntegrationSuite org.apache.spark.sql.jdbc.MariaDBKrbIntegrationSuite *DB2KrbIntegrationSuite org.apache.spark.sql.jdbc.DB2IntegrationSuite org.apache.spark.sql.jdbc.OracleIntegrationSuite"
```
If so, I'm trying that now. Seems like a lib and some referenced docker images are out of date. I'm going to open another PR with those updates so I can play unit test roulette in a context I deem less time sensitive than this one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998063053
Sorry, I think this is a dumb question, but I can't seem to find the answer with google. Is there a nightly snapshot build for 3.3 I can download somewhere?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] HyukjinKwon commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-999177223
Thanks @tdg5 - i like how this patch is thoroughly tested. this is a good example to follow :-).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-999828289
branch-3.1 PR is here: https://github.com/apache/spark/pull/34988
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] sarutak commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r773076081
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MariaDBKrbIntegrationSuite.scala
##########
@@ -25,9 +25,9 @@ import org.apache.spark.sql.execution.datasources.jdbc.connection.SecureConnecti
import org.apache.spark.tags.DockerTest
/**
- * To run this test suite for a specific version (e.g., mariadb:10.5.12):
+ * To run this test suite for a specific version (e.g., mariadb:10.5.13):
* {{{
- * ENABLE_DOCKER_INTEGRATION_TESTS=1 MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.12
+ * ENABLE_DOCKER_INTEGRATION_TESTS=1 MARIADB_DOCKER_IMAGE_NAME=mariadb:10.5.13
Review comment:
`10.5.12` is still available.
https://hub.docker.com/layers/mariadb/library/mariadb/10.5.12/images/sha256-a5ebad8f03ea9352ccf1ec3c84d91c2112ba86bbface1b57091f37aa7899d6c9?context=explore
But it's OK to upgrade.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998256819
**[Test build #146408 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146408/testReport)** for PR 34745 at commit [`4aac540`](https://github.com/apache/spark/commit/4aac54032ead4dd66f94de7726951aec91488a18).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998324277
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/50883/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] HyukjinKwon commented on a change in pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34745:
URL: https://github.com/apache/spark/pull/34745#discussion_r772747839
##########
File path: project/SparkBuild.scala
##########
@@ -600,7 +600,7 @@ object DockerIntegrationTests {
lazy val settings = Seq(
dependencyOverrides += "com.google.guava" % "guava" % "18.0",
resolvers += "DB2" at "https://app.camunda.com/nexus/content/repositories/public/",
- libraryDependencies += "com.oracle" % "ojdbc6" % "11.2.0.1.0" from "https://app.camunda.com/nexus/content/repositories/public/com/oracle/ojdbc6/11.2.0.1.0/ojdbc6-11.2.0.1.0.jar" // scalastyle:ignore
+ libraryDependencies += "com.oracle" % "ojdbc6" % "11.2.0.3" from "https://app.camunda.com/nexus/content/repositories/public/com/oracle/ojdbc6/11.2.0.3/ojdbc6-11.2.0.3.jar" // scalastyle:ignore
Review comment:
BTW, is it something we can backport up to branch-3.1? If not, let's separate these changes into a separate PR so that can be only merged to master or other branches.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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] tdg5 commented on pull request #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
tdg5 commented on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998044994
I added the missing `@since` with a value of `@since 3.1.3`. I also corrected the MIMA exception that I had added because I put it in the 3.1->3.2 bucket instead of the 3.2->3.3 bucket. Not sure if the `@since` changes that, but the MIMA test failed w/o the exception w/ the `@since` in place, so I imagine it is still required.
Not sure why the GitHub actions aren't working on my fork; they are enabled. I tweaked a couple of other things. Hopefully that helps.
Since I have to play unit test roulette due to the addition of `@since`, I added the library changes to make the docker integration tests work to this PR also. Let me know if that's a problem or if you have any thoughts on the changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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 #34745: [SPARK-37391][SQL]JdbcConnectionProvider tells if it modifies security context
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34745:
URL: https://github.com/apache/spark/pull/34745#issuecomment-998846162
**[Test build #146443 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/146443/testReport)** for PR 34745 at commit [`a7e9e2e`](https://github.com/apache/spark/commit/a7e9e2e8cbfa79b4900c685c740be8d8a394f0df).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
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