Re: [PR] [WIP] Add support for java 17 and explicit Python versions from spark 3.5.0 onwards [spark-docker]

2023-10-15 Thread via GitHub


vakarisbk commented on code in PR #56:
URL: https://github.com/apache/spark-docker/pull/56#discussion_r1359853632


##
.github/workflows/test.yml:
##
@@ -37,12 +37,15 @@ on:
 - 3.3.0
   java:
 description: 'The Java version of Spark image.'
-default: 11
+default: "11"

Review Comment:
   Not really. Value is defined as string and my linter was complaining that 
it's not a string. GH actions don't really care about 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-unsubscr...@spark.apache.org

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


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



Re: [PR] [WIP] Add support for java 17 and explicit Python versions from spark 3.5.0 onwards [spark-docker]

2023-10-15 Thread via GitHub


vakarisbk commented on code in PR #56:
URL: https://github.com/apache/spark-docker/pull/56#discussion_r1359852744


##
3.5.0/scala2.12-java11-python3-r-ubuntu/Dockerfile:
##
@@ -20,7 +20,10 @@ USER root
 
 RUN set -ex; \
 apt-get update; \
-apt-get install -y python3 python3-pip; \
+apt install -y software-properties-common; \
+add-apt-repository ppa:deadsnakes/ppa; \
+apt install python3.10; \

Review Comment:
   Rolled this back. My initial idea was to propose adding images with multiple 
python versions to the repo (java17-python3.10, java17-python3.9, etc), but now 
that I think about it - probably not a lot of community members would benefit 
from this and it would clutter up the repository quite a bit.
   
   And those people who need to have specific python versions (like me) can 
just take a base image and install whatever python version they want.



-- 
This is an automated message from the 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-unsubscr...@spark.apache.org

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


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



Re: [PR] [WIP] Add support for java 17 and explicit Python versions from spark 3.5.0 onwards [spark-docker]

2023-10-15 Thread via GitHub


vakarisbk commented on code in PR #56:
URL: https://github.com/apache/spark-docker/pull/56#discussion_r1359852798


##
add-dockerfiles.sh:
##
@@ -44,12 +48,20 @@ for TAG in $TAGS; do
 if echo $TAG | grep -q "r-"; then
 OPTS+=" --sparkr"
 fi
+
+if echo $TAG | grep -q "java17"; then
+OPTS+=" --java-version 17 --image eclipse-temurin:17-jre-jammy"
+fi
+if echo $TAG | grep -q "java11"; then

Review Comment:
   fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org

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


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



Re: [PR] [WIP] Add support for java 17 and explicit Python versions from spark 3.5.0 onwards [spark-docker]

2023-10-13 Thread via GitHub


Yikun commented on code in PR #56:
URL: https://github.com/apache/spark-docker/pull/56#discussion_r1358357340


##
3.5.0/scala2.12-java11-python3-r-ubuntu/Dockerfile:
##
@@ -20,7 +20,10 @@ USER root
 
 RUN set -ex; \
 apt-get update; \
-apt-get install -y python3 python3-pip; \
+apt install -y software-properties-common; \
+add-apt-repository ppa:deadsnakes/ppa; \
+apt install python3.10; \

Review Comment:
   Is there any special reason why we use the python 3.10? I prefer to use os 
default python3 version from matainence cost view, and also os default python 
version has more stable quality and security.



-- 
This is an automated message from the 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-unsubscr...@spark.apache.org

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


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



Re: [PR] [WIP] Add support for java 17 and explicit Python versions from spark 3.5.0 onwards [spark-docker]

2023-10-13 Thread via GitHub


Yikun commented on code in PR #56:
URL: https://github.com/apache/spark-docker/pull/56#discussion_r1358357340


##
3.5.0/scala2.12-java11-python3-r-ubuntu/Dockerfile:
##
@@ -20,7 +20,10 @@ USER root
 
 RUN set -ex; \
 apt-get update; \
-apt-get install -y python3 python3-pip; \
+apt install -y software-properties-common; \
+add-apt-repository ppa:deadsnakes/ppa; \
+apt install python3.10; \

Review Comment:
   Is there any special reason why we use the python 3.10? I prefer to use os 
default python3 version from matainence cost view, and also os default python 
version has more stable quality.



-- 
This is an automated message from the 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-unsubscr...@spark.apache.org

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


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



Re: [PR] [WIP] Add support for java 17 and explicit Python versions from spark 3.5.0 onwards [spark-docker]

2023-10-13 Thread via GitHub


Yikun commented on code in PR #56:
URL: https://github.com/apache/spark-docker/pull/56#discussion_r1358361784


##
tools/template.py:
##
@@ -59,7 +59,7 @@ def parse_opts():
 parser.add_argument(
 "-j",
 "--java-version",
-help="The Spark version of Dockerfile.",
+help="Java version of Dockerfile.",

Review Comment:
   Good catch



-- 
This is an automated message from the 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-unsubscr...@spark.apache.org

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


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



Re: [PR] [WIP] Add support for java 17 and explicit Python versions from spark 3.5.0 onwards [spark-docker]

2023-10-13 Thread via GitHub


Yikun commented on code in PR #56:
URL: https://github.com/apache/spark-docker/pull/56#discussion_r1358361340


##
.github/workflows/test.yml:
##
@@ -37,12 +37,15 @@ on:
 - 3.3.0
   java:
 description: 'The Java version of Spark image.'
-default: 11
+default: "11"

Review Comment:
   Is it neccessary?



-- 
This is an automated message from the 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-unsubscr...@spark.apache.org

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


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



Re: [PR] [WIP] Add support for java 17 and explicit Python versions from spark 3.5.0 onwards [spark-docker]

2023-10-13 Thread via GitHub


Yikun commented on code in PR #56:
URL: https://github.com/apache/spark-docker/pull/56#discussion_r1358359041


##
add-dockerfiles.sh:
##
@@ -44,12 +48,20 @@ for TAG in $TAGS; do
 if echo $TAG | grep -q "r-"; then
 OPTS+=" --sparkr"
 fi
+
+if echo $TAG | grep -q "java17"; then
+OPTS+=" --java-version 17 --image eclipse-temurin:17-jre-jammy"
+fi
+if echo $TAG | grep -q "java11"; then

Review Comment:
   elif?



##
add-dockerfiles.sh:
##
@@ -44,12 +48,20 @@ for TAG in $TAGS; do
 if echo $TAG | grep -q "r-"; then
 OPTS+=" --sparkr"
 fi
+
+if echo $TAG | grep -q "java17"; then
+OPTS+=" --java-version 17 --image eclipse-temurin:17-jre-jammy"

Review Comment:
   Greate!



##
3.5.0/scala2.12-java11-python3-r-ubuntu/Dockerfile:
##
@@ -20,7 +20,10 @@ USER root
 
 RUN set -ex; \
 apt-get update; \
-apt-get install -y python3 python3-pip; \
+apt install -y software-properties-common; \
+add-apt-repository ppa:deadsnakes/ppa; \
+apt install python3.10; \

Review Comment:
   Is there any special reason why we use the python 3.10? I prefer to use os 
default python3 version from matainence cost view.



-- 
This is an automated message from the 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-unsubscr...@spark.apache.org

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


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