[GitHub] [drill] vvysotskyi commented on a change in pull request #2317: DRILL-7999 Base Docker image on maven:3.8.2-jdk-11 and openjdk-11

[GitBox <gi...@apache.org>]

vvysotskyi commented on a change in pull request #2317:
URL: https://github.com/apache/drill/pull/2317#discussion_r710082331



##########
File path: Dockerfile
##########
@@ -16,31 +16,53 @@
 # limitations under the License.
 #
 
-# This Dockerfile is used for automated builds in DockerHub. It adds project sources into the build image, builds
-# Drill and copies built binaries into the target image based on openjdk:8u232-jdk image.
+# This Dockerfile is used for automated builds in DockerHub. It adds
+# project sources into the build image, builds Drill and copies built
+# binaries into the target image based on the image name in BASE_NAME
+# env var which you should set when invoking docker build. 
+# Example BASE_NAME values: openjdk:8-jre, openjdk:11-jre, openjdk:latest
 
 # Uses intermediate image for building Drill to reduce target image size
-FROM maven:3.6.3-jdk-8 as build
+FROM maven:3.8.2-openjdk-11 as build
+
+WORKDIR /src
 
 # Copy project sources into the container
-COPY . /src
+COPY . .
 
-WORKDIR /src
+# Optimisation: build a selection of Drill modules in advance.
+# It isn't necessary to build any modules independently but doing so divides
+# the downloading and compiling up over multiple cacheable layers, better
+# enabling reuse in the event of an isolated change and resume in the event
+# of a build error.  This paragraph can safely be commented out.
+RUN mvn -am -pl tools        -Dmaven.artifact.threads=5 -T1C install -DskipTests
+RUN mvn -am -pl protocol     -Dmaven.artifact.threads=5 -T1C install -DskipTests
+RUN mvn -am -pl common       -Dmaven.artifact.threads=5 -T1C install -DskipTests
+RUN mvn -am -pl logical      -Dmaven.artifact.threads=5 -T1C install -DskipTests
+RUN mvn -am -pl exec         -Dmaven.artifact.threads=5 -T1C install -DskipTests
+RUN mvn -am -pl drill-yarn   -Dmaven.artifact.threads=5 -T1C install -DskipTests
+RUN mvn -am -pl distribution -Dmaven.artifact.threads=5 -T1C install -DskipTests
 
 # Builds Drill
-RUN  mvn clean install -DskipTests -q
+RUN mvn -Dmaven.artifact.threads=5 -T1C install -DskipTests
 
 # Get project version and copy built binaries into /opt/drill directory
 RUN VERSION=$(mvn -q -Dexec.executable=echo -Dexec.args='${project.version}' --non-recursive exec:exec) \
  && mkdir /opt/drill \
  && mv distribution/target/apache-drill-${VERSION}/apache-drill-${VERSION}/* /opt/drill
 
 # Target image
-FROM openjdk:8u232-jdk
 
-RUN mkdir /opt/drill
+# Set the BASE_IMAGE env var when you invoke docker build.  
+FROM $BASE_IMAGE

Review comment:
       Please assign a default value for `BASE_IMAGE` to preserve backward compatibility when building images without specifying env variables.

##########
File path: Dockerfile
##########
@@ -16,31 +16,53 @@
 # limitations under the License.
 #
 
-# This Dockerfile is used for automated builds in DockerHub. It adds project sources into the build image, builds
-# Drill and copies built binaries into the target image based on openjdk:8u232-jdk image.
+# This Dockerfile is used for automated builds in DockerHub. It adds
+# project sources into the build image, builds Drill and copies built
+# binaries into the target image based on the image name in BASE_NAME
+# env var which you should set when invoking docker build. 
+# Example BASE_NAME values: openjdk:8-jre, openjdk:11-jre, openjdk:latest
 
 # Uses intermediate image for building Drill to reduce target image size
-FROM maven:3.6.3-jdk-8 as build
+FROM maven:3.8.2-openjdk-11 as build

Review comment:
       Are you sure that jars built using JDK 11 will be running fine on JDB 8?




-- 
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: dev-unsubscribe@drill.apache.org

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