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 2022/10/17 04:41:24 UTC

[GitHub] [spark-docker] Yikun opened a new pull request, #12: [SPARK-40528] Support dockerfile template

Yikun opened a new pull request, #12:
URL: https://github.com/apache/spark-docker/pull/12

   ### What changes were proposed in this pull request?
   This patch:
   - Add dockerfile template: `Dockerfile.template` contains 3 vars: `BASE_IMAGE` for base image name, `HAVE_PY` for adding python support, `HAVE_R` for adding sparkr support.
   - Add a script: `add-dockerfiles.sh`, you can `./add-dockerfiles.sh 3.3.0`
   - Add a tool: `tempalte.py` to help generate dockerfile from jinja template. 
   
   ### Why are the changes needed?
   Generate the dockerfiles to make life easier.
   
   ### Does this PR introduce _any_ user-facing change?
   No, dev only.
   
   ### How was this patch tested?
   ```shell
   # Prepare new env
   python3 -m venv ~/xxx
   pip install -r ./tools/requirements.txt
   source ~/xxx/bin/activate
   
   # Generate 3.3.0
   ./add-dockerfiles.sh 3.3.0
   
   # no diff
   git diff
   ```
   
   lint:
   ```
   $ flake8 ./tools/template.py
   $ black ./tools/template.py
   All done! ✨ 🍰 ✨
   1 file left unchanged.
   ```
   


-- 
This is an automated message from the 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-docker] Yikun commented on a diff in pull request #12: [SPARK-40528] Support dockerfile template

Posted by GitBox <gi...@apache.org>.
Yikun commented on code in PR #12:
URL: https://github.com/apache/spark-docker/pull/12#discussion_r996710951


##########
Dockerfile.template:
##########
@@ -0,0 +1,96 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+FROM {{ BASE_IMAGE }}
+
+ARG spark_uid=185
+
+RUN set -ex && \
+    apt-get update && \
+    ln -s /lib /lib64 && \
+    apt install -y gnupg2 wget bash tini libc6 libpam-modules krb5-user libnss3 procps net-tools && \

Review Comment:
   This patch would not change the dockerfile itself, if has problems, I prepare to do it in a separate PR. : )
   
   I take a look on exsiting docker official image `-q` is not used: 
   https://github.com/search?l=Dockerfile&p=3&q=org%3Adocker-library+apt&type=Code
   
   I guess it might help to see the version/extra dependency packages.



-- 
This is an automated message from the 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-docker] Yikun commented on pull request #12: [SPARK-40528] Support dockerfile template

Posted by GitBox <gi...@apache.org>.
Yikun commented on PR #12:
URL: https://github.com/apache/spark-docker/pull/12#issuecomment-1280475467

   @HyukjinKwon @martin-g Thanks, merged.


-- 
This is an automated message from the 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-docker] Yikun commented on pull request #12: [SPARK-40528] Support dockerfile template

Posted by GitBox <gi...@apache.org>.
Yikun commented on PR #12:
URL: https://github.com/apache/spark-docker/pull/12#issuecomment-1280471876

   ```
   (venv) ➜  spark-docker git:(SPARK-40528) flake8 ./tools/template.py
   (venv) ➜  spark-docker git:(SPARK-40528) black ./tools/template.py
   All done! ✨ 🍰 ✨
   1 file left unchanged.
   (venv) ➜  spark-docker git:(SPARK-40528) ./add-dockerfiles.sh 3.3.0
   (venv) ➜  spark-docker git:(SPARK-40528) git --no-pager diff
   ```
   
   Just a rebase, will merge soon


-- 
This is an automated message from the 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-docker] martin-g commented on a diff in pull request #12: [SPARK-40528] Support dockerfile template

Posted by GitBox <gi...@apache.org>.
martin-g commented on code in PR #12:
URL: https://github.com/apache/spark-docker/pull/12#discussion_r996649838


##########
add-dockerfiles.sh:
##########
@@ -0,0 +1,45 @@
+#!/usr/bin/env bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+INPUT_VERSION=$1
+VERSION=${INPUT_VERSION:-"3.3.0"}

Review Comment:
   Do you need `INPUT_VERSION` ?
   `VERSION=${1:-"3.3.0"}` should be the same.



##########
Dockerfile.template:
##########
@@ -0,0 +1,96 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+FROM {{ BASE_IMAGE }}
+
+ARG spark_uid=185
+
+RUN set -ex && \
+    apt-get update && \
+    ln -s /lib /lib64 && \
+    apt install -y gnupg2 wget bash tini libc6 libpam-modules krb5-user libnss3 procps net-tools && \

Review Comment:
   Consider using `-q` or even `-qq` for the `apt` commands.
   The default output verbosity is usually too big.



-- 
This is an automated message from the 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-docker] Yikun closed pull request #12: [SPARK-40528] Support dockerfile template

Posted by GitBox <gi...@apache.org>.
Yikun closed pull request #12: [SPARK-40528] Support dockerfile template
URL: https://github.com/apache/spark-docker/pull/12


-- 
This is an automated message from the 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