Skip to content
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

docker: add example Dockerfile #2844

Closed
wants to merge 5 commits into from
Closed

docker: add example Dockerfile #2844

wants to merge 5 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Nov 26, 2019

Fixes #2774, iterative/dvc.org#811

Build: docker build -t $TAG --build-arg USER_ID=${UID} .
Usage: docker run -ti --rm -v ${PWD}:/usr/src $TAG

@ghost ghost requested review from casperdcl and shcheklein November 26, 2019 00:04
Comment on lines 22 to 25

# Set Zsh as the default shell
SHELL ["/usr/bin/zsh", "-c"]

Copy link
Contributor

Choose a reason for hiding this comment

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

I think most people are used to Bash. It's not like we recommend Zsh over Bash in our docs. So this one may be unnecessary? Up to you guys.

Copy link
Author

Choose a reason for hiding this comment

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

@jorgeorpinel , using Zsh is not that different from Bash. I could argue that a lot of Bash users are using it because they are not aware of Zsh 😅

I'd prefer using zsh over bash, since the autocomplete features are better, thus, offering a better experience.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agee about the autocomplete functions. I use Zsh myself 😬 I was just wondering whether a Bash user would find the running container unfamiliar or difficult to use because of differences between Bash and Zsh. But I guess that's not likely. So unless anyone else thinks otherwise, I'm OK with this.

docker/Dockerfile Outdated Show resolved Hide resolved
Comment on lines 39 to 53
# Load and rice the completion menu with `zstyle`:
# http://zsh.sourceforge.net/Doc/Release/Zsh-Modules.html#The-zsh_002fzutil-Module
RUN echo "autoload -U compinit && compinit" >> $HOME/.zshrc \
&& echo "zstyle ':completion:*:*:*:*:*' menu select" >> $HOME/.zshrc \
&& echo "zstyle ':completion:*:matches' group 'yes'" >> $HOME/.zshrc \
&& echo "zstyle ':completion:*:options' description 'yes'" >> $HOME/.zshrc \
&& echo "zstyle ':completion:*:options' auto-description '%d'" >> $HOME/.zshrc \
&& echo "zstyle ':completion:*:corrections' format ' %F{green}-- %d (errors: %e) --%f'" >> $HOME/.zshrc \
&& echo "zstyle ':completion:*:descriptions' format ' %F{yellow}-- %d --%f'" >> $HOME/.zshrc \
&& echo "zstyle ':completion:*:messages' format ' %F{purple} -- %d --%f'" >> $HOME/.zshrc \
&& echo "zstyle ':completion:*:warnings' format ' %F{red}-- no matches found --%f'" >> $HOME/.zshrc \
&& echo "zstyle ':completion:*:default' list-prompt '%S%M matches%s'" >> $HOME/.zshrc \
&& echo "zstyle ':completion:*' format ' %F{yellow}-- %d --%f'" >> $HOME/.zshrc \
&& echo "zstyle ':completion:*' group-name ''" >> $HOME/.zshrc \
&& echo "zstyle ':completion:*' verbose yes" >> $HOME/.zshrc
Copy link
Contributor

Choose a reason for hiding this comment

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

really if all of this is genuinely useful for zsh users then surely it should be under the scripts/ directory just like completion/?

Copy link
Author

Choose a reason for hiding this comment

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

@casperdcl , it is usually provided by Zsh prompts like "pure" or frameworks like "oh-my-zsh".

so you don't need to deal with zstyle and stuff like that :)

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case not really sure it belongs here. At the most maybe use pure or omz?

Basically I feel either put it in the scripts folder or remove it completely from the dockerfile.

Copy link
Author

Choose a reason for hiding this comment

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

Right, let's remove this, then; just for context, this is how it looks like

with zstyle:
2019-11-25-20:08:35

without zstyle (no way to iterate between the options):
2019-11-25-20:10:26

Copy link
Author

Choose a reason for hiding this comment

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

2019-11-25-20:18:36
boring bash 😛

@ghost
Copy link
Author

ghost commented Nov 26, 2019

Ohh, by the way, this solves the uid problem)

Comment on lines +3 to +9
# Better movement with [Ctrl + →] and [Ctrl + ←]
ENV INPUTRC=/etc/inputrc

RUN set -ex \
&& echo '"\e[1;5C": forward-word' >> /etc/inputrc \
&& echo '"\e[1;5D": backward-word' >> /etc/inputrc

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't quite understand what this is for and why this is needed in the docker file.

Copy link
Author

Choose a reason for hiding this comment

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

Usability. If you get into the shell, you won't be able to use Ctrl + arrows to move between words, for me it is annoying, that's why I added this.

Copy link
Contributor

Choose a reason for hiding this comment

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

again I think this is on the same level as zstyle - something that is generic shell/system-related and not about dvc

Comment on lines +10 to +13
# Set working directory.
# Mount your project under the same path.
WORKDIR /usr/src

Copy link
Contributor

Choose a reason for hiding this comment

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

Using /usr/src seems pretty strange. I think it is used in some systems (e.g. slackware?) to store actual sources.

Copy link
Author

Choose a reason for hiding this comment

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

It could be whatever you want, I've seen some Dockerfiles using /usr/src since it is expected that you'll only put one source into it.

Comment on lines +15 to +17
RUN apt-get update -y && apt-get install -y \
less \
git
Copy link
Contributor

Choose a reason for hiding this comment

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

What's up with this odd formatting?

Copy link
Author

Choose a reason for hiding this comment

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

Readability, you can see at first glance what packages are being installed.

Comment on lines +23 to +25
RUN wget \
-O /etc/bash_completion.d/dvc \
https://raw.githubusercontent.com/iterative/dvc/master/scripts/completion/dvc.bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I understand why docker image needs bash completion. And if it does, why not zsh as well?

Copy link
Author

Choose a reason for hiding this comment

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

Usability. We opted to use Bash instead of Zsh. I don't see the point of using both.

https://raw.githubusercontent.com/iterative/dvc/master/scripts/completion/dvc.bash

# Add a user to run DVC
ARG USER_ID=1000
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose you are doing this so that files created by dvc are not owned by root, right? If so, now they are going to be owned by UID 1000, which is absolutely not guaranteed to match your current user on the host.

Copy link
Author

Choose a reason for hiding this comment

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

The user must build the image with their UID, see the description of the issue:
#2844 (comment)

Copy link
Contributor

@efiop efiop left a comment

Choose a reason for hiding this comment

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

I think I don't quite understand the purpose of this docker file. Is it to create a fully functional workspace for the user to loginto and use from inside?

@ghost
Copy link
Author

ghost commented Nov 26, 2019

I think I don't quite understand the purpose of this docker file. Is it to create a fully functional workspace for the user to loginto and use from inside?

@efiop, it is not clear. I'll close it for not having a clear reason to add it.

@ghost ghost closed this Nov 26, 2019
@casperdcl
Copy link
Contributor

@efiop for me it's documentation for how to install dvc outside docker. As a secondary point it could be used to install dvc within docker for use in an automated workflow (no end user types commands - dvc and git are used to push/pull code and data instead of wget/curl)

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

provide docker image(s)
3 participants