You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "pan3793 (via GitHub)" <gi...@apache.org> on 2024/05/23 04:47:20 UTC
[PR] SPARK_HOME detection supports Spark 4 [kyuubi]
pan3793 opened a new pull request, #6413:
URL: https://github.com/apache/kyuubi/pull/6413
# Description
When `SPARK_HOME` is not set explicitly, the Kyuubi server supports detecting it based on Scala versions, while the rules are not applicable for Spark 4.
This PR enhances the SPARK_HOME detection logic to make it support both Spark 3 and Spark 4.
The above logic is mainly used for testing purposes, the change does not affect users who configure `SPARK_HOME` in `kyuubi-env.sh`.
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
## Test Plan
#### Related Unit Tests
- `SparkProcessBuilderSuite`
---
# Checklist 📝
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [x] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)
**Be nice. Be informative.**
--
This is an automated message from the 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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] SPARK_HOME detection supports Spark 4 [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 closed pull request #6413: SPARK_HOME detection supports Spark 4
URL: https://github.com/apache/kyuubi/pull/6413
--
This is an automated message from the 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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] SPARK_HOME detection supports Spark 4 [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #6413:
URL: https://github.com/apache/kyuubi/pull/6413#discussion_r1610972251
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilder.scala:
##########
@@ -364,11 +364,14 @@ object SparkProcessBuilder {
final private val SPARK_CONF_FILE_NAME = "spark-defaults.conf"
final private[kyuubi] val SPARK_CORE_SCALA_VERSION_REGEX =
- """^spark-core_(\d\.\d+).*.jar$""".r
+ """^spark-core_(\d\.\d+)-.*\.jar$""".r
- final private[kyuubi] val SPARK_HOME_REGEX_SCALA_212 =
- """^spark-\d+\.\d+\.\d+-bin-hadoop\d+(\.\d+)?$""".r
+ final private[kyuubi] val SPARK3_HOME_REGEX_SCALA_212 =
+ """^spark-3\.\d+\.\d+-bin-hadoop\d+(\.\d+)?$""".r
- final private[kyuubi] val SPARK_HOME_REGEX_SCALA_213 =
- """^spark-\d+\.\d+\.\d+-bin-hadoop\d(\.\d+)?+-scala\d+(\.\d+)?$""".r
+ final private[kyuubi] val SPARK3_HOME_REGEX_SCALA_213 =
+ """^spark-3\.\d+\.\d+-bin-hadoop\d(\.\d+)?+-scala2\.13$""".r
+
+ final private[kyuubi] val SPARK4_HOME_REGEX_SCALA_213 =
+ """^spark-4\.\d+\.\d+(-\w*)?-bin-hadoop\d(\.\d+)?+$""".r
Review Comment:
SPARK_HOME detection should be conservative since it is only used for testing purposes, and users/developers can always set SPARK_HOME explicitly to override it. While `SPARK_CORE_SCALA_VERSION_REGEX` is used to detect the Scala version, it should consider vender-provided Spark distributions 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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] SPARK_HOME detection supports Spark 4 [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #6413:
URL: https://github.com/apache/kyuubi/pull/6413#issuecomment-2126327292
Thanks, merged to master/1.9
--
This is an automated message from the 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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] SPARK_HOME detection supports Spark 4 [kyuubi]
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #6413:
URL: https://github.com/apache/kyuubi/pull/6413#discussion_r1610969373
##########
kyuubi-server/src/test/scala/org/apache/kyuubi/engine/spark/SparkProcessBuilderSuite.scala:
##########
@@ -403,36 +403,43 @@ class SparkProcessBuilderSuite extends KerberizedTestHelper with MockitoSugar {
"spark-core_2.13-3.5.0-abc-20230921.jar",
"spark-core_2.13-3.5.0-xyz-1.2.3.jar",
"spark-core_2.13-3.5.0.1.jar",
- "spark-core_2.13.2-3.5.0.jar").foreach { f =>
Review Comment:
this actually is a negative 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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] SPARK_HOME detection supports Spark 4 [kyuubi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #6413:
URL: https://github.com/apache/kyuubi/pull/6413#issuecomment-2126287400
## [Codecov](https://app.codecov.io/gh/apache/kyuubi/pull/6413?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `81.81818%` with `2 lines` in your changes are missing coverage. Please review.
> Project coverage is 58.34%. Comparing base [(`4285abc`)](https://app.codecov.io/gh/apache/kyuubi/commit/4285abc3ae44e15465587e1afdf82cd003c5654a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`20e71fd`)](https://app.codecov.io/gh/apache/kyuubi/pull/6413?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
| [Files](https://app.codecov.io/gh/apache/kyuubi/pull/6413?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ache/kyuubi/engine/spark/SparkProcessBuilder.scala](https://app.codecov.io/gh/apache/kyuubi/pull/6413?src=pr&el=tree&filepath=kyuubi-server%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fkyuubi%2Fengine%2Fspark%2FSparkProcessBuilder.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvc3BhcmsvU3BhcmtQcm9jZXNzQnVpbGRlci5zY2FsYQ==) | 81.81% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/kyuubi/pull/6413?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #6413 +/- ##
============================================
+ Coverage 58.32% 58.34% +0.01%
Complexity 24 24
============================================
Files 656 656
Lines 40263 40265 +2
Branches 5499 5498 -1
============================================
+ Hits 23482 23491 +9
+ Misses 14276 14269 -7
Partials 2505 2505
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/kyuubi/pull/6413?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the 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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org