Skip to content

Interactive docker image #709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Prev Previous commit
Next Next commit
Dockerfile update to fix Dialog issue
  • Loading branch information
indy committed Oct 6, 2020
commit 3e2de92485ca1495b62e0d4a003bb68d1362634c
3 changes: 2 additions & 1 deletion docker/images/interactive/dotnet-interactive/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ FROM jupyter/base-notebook:ubuntu-18.04
ARG NB_USER=jovyan
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the usage in which this ARG would be specified when building? I am not seeing how this would work if a different user were specified. By this I mean I don't see the user being defined/added. The base image defines the "jovyan" user, why would you want do define something different? The same applies to NB_UID.

ARG NB_UID=1000
ARG DOTNET_CORE_VERSION=3.1
ARG DEBIAN_FRONTEND=noninteractive
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? The base image is already defining ENV DEBIAN_FRONTEND=noninteractive


ENV DOTNET_CORE_VERSION=$DOTNET_CORE_VERSION
ENV USER ${NB_USER}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to a previous comment, are these USER related ENVs necessary since they are defined in the base image.

Expand All @@ -19,7 +20,7 @@ ENV \
USER root

RUN apt-get update \
&& DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \
&& apt-get install -y --no-install-recommends \
dialog apt-utils wget ca-certificates openjdk-8-jdk bash software-properties-common supervisor unzip socat net-tools vim \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is requiring all of these dependencies. Ones that are already provided by the base image don't seem necessary to include.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the Dockerfile best practices, it helps the readability to break this apart one package per line and alphabetize them.

libc6 libgcc1 libgssapi-krb5-2 libicu60 libssl1.1 libstdc++6 zlib1g \
&& wget -q --show-progress --progress=bar:force:noscroll https://packages.microsoft.com/config/ubuntu/18.04/packages-microsoft-prod.deb -O packages-microsoft-prod.deb \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider: I've typically seen Dockerfile avoid using --show-progress as it does have a perf impact.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That raises an interesting point about the purpose of the Dockerfile(s). As far as I am aware, the focus at the moment is to enable an user to build the image(s) her/himself, instead of automating the image build process. That's why I thought it would be useful to show the download progress. Now, for small downloads that doesn't really matter and I therefore have removed it. However, I have added the following line to the dotnet-spark/Dockerfile

&& echo "\nDownloading spark-${SPARK_VERSION}-bin-hadoop${HADOOP_VERSION}.tgz ..." \

as the spark download can take a while. Does that make sense?

Expand Down