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/10/22 15:50:07 UTC
[GitHub] [spark] Bidek56 opened a new pull request #34371: [SPARK-37091][R]Upgrading SystemRequirements to include Java <= 17
Bidek56 opened a new pull request #34371:
URL: https://github.com/apache/spark/pull/34371
### What changes were proposed in this pull request?
Modifying SystemRequirements to include Java <= 17
### Why are the changes needed?
Currently Java is limited to version 11
### Does this PR introduce _any_ user-facing change? No
### How was this patch tested?
```
$ build/mvn -Phadoop-3.2 -Psparkr -DskipTests package
$ R/install-dev.sh
$ R/run-tests.sh
```
--
This is an automated message from the 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] Bidek56 edited a comment on pull request #34371: [SPARK-37091][R] Upgrading SystemRequirements to include Java <= 17
Posted by GitBox <gi...@apache.org>.
Bidek56 edited a comment on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950037038
> I agree with this change, and LGTM but I would like to understand how this change interacts with something external.
This [issue](https://github.com/jupyter/docker-stacks/issues/1498) is affected by the Java 11 limit
--
This is an automated message from the 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] Bidek56 commented on a change in pull request #34371: [SPARK-37091][R] SystemRequirements to include Java < 18
Posted by GitBox <gi...@apache.org>.
Bidek56 commented on a change in pull request #34371:
URL: https://github.com/apache/spark/pull/34371#discussion_r735110357
##########
File path: core/src/test/scala/org/apache/spark/internal/config/ConfigEntrySuite.scala
##########
@@ -94,6 +94,8 @@ class ConfigEntrySuite extends SparkFunSuite {
assert(conf.get(bytes) === 1024L)
conf.set(bytes.key, "1k")
assert(conf.get(bytes) === 1L)
+ conf.set(bytes.key, "2048")
+ assert(conf.get(bytes) === 2048)
Review comment:
I have reverted the rebase but I think the build issue will be fixed by this [PR](https://github.com/apache/spark/pull/34373)
--
This is an automated message from the 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 #34371: [SPARK-37091][R] SystemRequirements to include Java < 18
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950523509
I cherry-picked https://github.com/apache/spark/commit/76a317aaafbf52e8e1d5687e9d72e3a22b0a322e back to the master branch.
--
This is an automated message from the 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] Bidek56 commented on pull request #34371: [SPARK-37091][R] Upgrading SystemRequirements to include Java <= 17
Posted by GitBox <gi...@apache.org>.
Bidek56 commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950037038
> I agree with this change, and LGTM but I would like to understand how this change interacts with something external.
This [issue](https://github.com/jupyter/drumsticks/issues/1498) is affected by the Java 11 limit
--
This is an automated message from the 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] srowen commented on pull request #34371: [SPARK-37091][R] Upgrading SystemRequirements to include Java <= 17
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950044528
I also can't see the repo, but w/e. It seems fine to make this change - not sure any tests will test it anyway.
This is a fine change for 3.3, but, only Java 11 is supported in 3.2.0 anyway, note.
--
This is an automated message from the 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] srowen commented on pull request #34371: [SPARK-37091][R] SystemRequirements to include Java < 18
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950420126
This looks fine now. It wasn't a test problem, just some incorrect commits got merged when you rebased this branch. All set
--
This is an automated message from the 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 #34371: [SPARK-37091][R] SystemRequirements to include Java < 18
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950522016
This is a bit odds because the PR shows the diff only the one in description https://github.com/apache/spark/pull/34371/files
--
This is an automated message from the 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 #34371: [SPARK-37091][R] Upgrading SystemRequirements to include Java <= 17
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950042051
https://github.com/jupyter/drumsticks: I cannot access 😂. Once it's enabled, you could rebase in this PR. That should kick the job
--
This is an automated message from the 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 #34371: [SPARK-37091][R] Upgrading SystemRequirements to include Java <= 17
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34371:
URL: https://github.com/apache/spark/pull/34371#discussion_r734962210
##########
File path: R/pkg/DESCRIPTION
##########
@@ -13,7 +13,7 @@ Authors@R: c(person("Shivaram", "Venkataraman", role = "aut",
License: Apache License (== 2.0)
URL: https://www.apache.org https://spark.apache.org
BugReports: https://spark.apache.org/contributing.html
-SystemRequirements: Java (>= 8, < 12)
+SystemRequirements: Java (>= 8, <= 17)
Review comment:
Okay, can we change <= 17 to < 18? Seems like it's going to be broken at https://github.com/apache/spark/blob/0494dc90af48ce7da0625485a4dc6917a244d580/R/pkg/R/client.R#L69.
--
This is an automated message from the 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] dongjoon-hyun commented on pull request #34371: [SPARK-37091][R] SystemRequirements to include Java < 18
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950521492
@HyukjinKwon , the final commit looks still wrong to me.
- https://github.com/apache/spark/commit/360897153755e76608a7c067d01f635fca2a5da8
Could you check the final commit before pushing to the Apache repo?
--
This is an automated message from the 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 edited a comment on pull request #34371: [SPARK-37091][R] SystemRequirements to include Java < 18
Posted by GitBox <gi...@apache.org>.
HyukjinKwon edited a comment on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950522016
This is a bit odds because the PR shows the diff only the one in description https://github.com/apache/spark/pull/34371/files, and I haven't had any problem in such cases so far.
--
This is an automated message from the 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] Bidek56 commented on pull request #34371: [SPARK-37091][R] Upgrading SystemRequirements to include Java <= 17
Posted by GitBox <gi...@apache.org>.
Bidek56 commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950037941
> https://github.com/jupyter/drumsticks repo seems private. Okay, if something is affected, then it's fine - I asked because this Java versions in description are not used in CRAN and rather just a metadata up to my knowledge.
It's a public [repo](https://github.com/jupyter/docker-stacks/issues/1498)
--
This is an automated message from the 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] Bidek56 commented on pull request #34371: [SPARK-37091][R] Upgrading SystemRequirements to include Java <= 17
Posted by GitBox <gi...@apache.org>.
Bidek56 commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950038236
I did enabled GA but after the failure, not sure to rerun the action, besides using an API.
--
This is an automated message from the 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 #34371: [SPARK-37091][R] Upgrading SystemRequirements to include Java <= 17
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950037653
Mind enabling GitHub Actions in your forked repository? Apache Spark leverages contributor's resources in forked repository in their PRs (https://github.com/apache/spark/pull/34371/checks?check_run_id=3977747161).
--
This is an automated message from the 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 #34371: [SPARK-37091][R] SystemRequirements to include Java < 18
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34371:
URL: https://github.com/apache/spark/pull/34371#discussion_r735053257
##########
File path: core/src/test/scala/org/apache/spark/internal/config/ConfigEntrySuite.scala
##########
@@ -94,6 +94,8 @@ class ConfigEntrySuite extends SparkFunSuite {
assert(conf.get(bytes) === 1024L)
conf.set(bytes.key, "1k")
assert(conf.get(bytes) === 1L)
+ conf.set(bytes.key, "2048")
+ assert(conf.get(bytes) === 2048)
Review comment:
Seems like there was something wrong when you rebased .. can you remove these diff in the 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] dongjoon-hyun edited a comment on pull request #34371: [SPARK-37091][R] SystemRequirements to include Java < 18
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950521492
@HyukjinKwon , the final commit looks still wrong to me.
- https://github.com/apache/spark/commit/360897153755e76608a7c067d01f635fca2a5da8
Could you check the final commit before pushing to the Apache repo, @HyukjinKwon ?
--
This is an automated message from the 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] dongjoon-hyun commented on pull request #34371: [SPARK-37091][R] SystemRequirements to include Java < 18
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950522783
No problem, @HyukjinKwon ~ We are okay because this is not released and we can fix it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #34371: [SPARK-37091][R] Upgrading SystemRequirements to include Java <= 17
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950068687
CRAN check in SparkR build validates the DESCRIPTION file. The check won't validate the values but at least format and etc. Might need to make sure that the tests pass anyway although it's unlikely that the tests are broken by this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] Bidek56 commented on pull request #34371: [SPARK-37091][R] Upgrading SystemRequirements to include Java <= 17
Posted by GitBox <gi...@apache.org>.
Bidek56 commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950138837
> https://github.com/jupyter/drumsticks: I cannot access 😢 . Once it's enabled, you could rebase in this PR. That should kick the job
It's a public [repo](https://github.com/jupyter/docker-stacks/issues/1498), not sure where the **drumsticks** came from, maybe Android share link, sorry.
Rebasing it now. Thanks for your help!
--
This is an automated message from the 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 #34371: [SPARK-37091][R]Upgrading SystemRequirements to include Java <= 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-949753983
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] HyukjinKwon commented on pull request #34371: [SPARK-37091][R] SystemRequirements to include Java < 18
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950521745
oops
--
This is an automated message from the 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 #34371: [SPARK-37091][R] Upgrading SystemRequirements to include Java <= 17
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950034756
I agree with this change, and LGTM but I would like to understand how this change interacts with what.
--
This is an automated message from the 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 #34371: [SPARK-37091][R] Upgrading SystemRequirements to include Java <= 17
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950037574
https://github.com/jupyter/drumsticks repo seems private. Okay, if something is affected, then it's fine - I asked because this Java versions in description are not used in CRAN and rather just a metadata up to my knowledge.
--
This is an automated message from the 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] Bidek56 removed a comment on pull request #34371: [SPARK-37091][R] SystemRequirements to include Java < 18
Posted by GitBox <gi...@apache.org>.
Bidek56 removed a comment on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950038236
I did enabled GA but after the failure, not sure to rerun the action, besides using an API.
--
This is an automated message from the 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 #34371: [SPARK-37091][R] SystemRequirements to include Java < 18
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950434121
Actually, I think the current PR is mergable and should look fine since it will be squashed .. although it should ideally be rebased.
Merged to master.
--
This is an automated message from the 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 closed pull request #34371: [SPARK-37091][R] SystemRequirements to include Java < 18
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #34371:
URL: https://github.com/apache/spark/pull/34371
--
This is an automated message from the 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 #34371: [SPARK-37091][R] SystemRequirements to include Java < 18
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950522356
sorry it was my bad. let me correct 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] dongjoon-hyun commented on pull request #34371: [SPARK-37091][R] SystemRequirements to include Java < 18
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950522434
Ya, this commit technically reverted `[SPARK-37084][SQL] Set spark.sql.files.openCostInBytes to bytesConf`
--
This is an automated message from the 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] dongjoon-hyun commented on a change in pull request #34371: [SPARK-37091][R] SystemRequirements to include Java < 18
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #34371:
URL: https://github.com/apache/spark/pull/34371#discussion_r735059274
##########
File path: core/src/test/scala/org/apache/spark/internal/config/ConfigEntrySuite.scala
##########
@@ -94,6 +94,8 @@ class ConfigEntrySuite extends SparkFunSuite {
assert(conf.get(bytes) === 1024L)
conf.set(bytes.key, "1k")
assert(conf.get(bytes) === 1L)
+ conf.set(bytes.key, "2048")
+ assert(conf.get(bytes) === 2048)
Review comment:
+1 for @HyukjinKwon 's comment. This should not be in this SparkR 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] HyukjinKwon commented on a change in pull request #34371: [SPARK-37091][R] Upgrading SystemRequirements to include Java <= 17
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34371:
URL: https://github.com/apache/spark/pull/34371#discussion_r734906656
##########
File path: R/pkg/DESCRIPTION
##########
@@ -13,7 +13,7 @@ Authors@R: c(person("Shivaram", "Venkataraman", role = "aut",
License: Apache License (== 2.0)
URL: https://www.apache.org https://spark.apache.org
BugReports: https://spark.apache.org/contributing.html
-SystemRequirements: Java (>= 8, < 12)
+SystemRequirements: Java (>= 8, <= 17)
Review comment:
Hey, I would like to double check this. Why does this matter? What issue did you face by this?
--
This is an automated message from the 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 edited a comment on pull request #34371: [SPARK-37091][R] Upgrading SystemRequirements to include Java <= 17
Posted by GitBox <gi...@apache.org>.
HyukjinKwon edited a comment on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950034756
I agree with this change, and LGTM but I would like to understand how this change interacts with something external.
--
This is an automated message from the 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 edited a comment on pull request #34371: [SPARK-37091][R] Upgrading SystemRequirements to include Java <= 17
Posted by GitBox <gi...@apache.org>.
HyukjinKwon edited a comment on pull request #34371:
URL: https://github.com/apache/spark/pull/34371#issuecomment-950042051
https://github.com/jupyter/drumsticks: I cannot access 😢 . Once it's enabled, you could rebase in this PR. That should kick the job
--
This is an automated message from the 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