-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Add a docker file for building frontend assets #4843
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
FROM ubuntu:bionic | ||
|
||
ENV LANG C.UTF-8 | ||
|
||
# install latest python and nodejs | ||
RUN apt-get update && \ | ||
apt-get install -y \ | ||
nodejs \ | ||
npm \ | ||
gettext \ | ||
git \ | ||
sudo \ | ||
build-essential \ | ||
python3 \ | ||
python3-dev \ | ||
python3-pip \ | ||
libxml2-dev \ | ||
libxslt1-dev \ | ||
zlib1g-dev | ||
|
||
RUN bash -c "pip3 install pip setuptools --upgrade --force-reinstall" | ||
RUN bash -c "pip3 install virtualenv" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason to use virtualenv? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Python requirements are installed into a virtualenv, they are less predictable if installed into a system env - things are more likely to break with time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wouldn't use a virtual environment either. |
||
RUN bash -c "npm install -g bower" | ||
|
||
# RUN adduser --system --shell /bin/bash --home "/repo" rtd | ||
|
||
# A volume used to share `pex`/`whl` files and fixtures with docker host | ||
VOLUME /repo | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has come up any time docker has been discussed, but mounting like this is bound to result in permission errors, especially for any folks on Linux, which is majority of core team. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the essence of the whole Docker file though: It builds the output assets directly in your local files. I'm on Linux and it works fine. Without the use of a volume, I can't complete this PR. So might as well close it then. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @benjaoming when running this docker image, don't the generated files by the docker image end up belonging to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @humitos not existing files, but I can see that newly created files are owned by Reference: moby/moby#3124 I can see these options:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are some methods of inheriting the I haven't come across anything better. I can implement this. |
||
|
||
# do the time-consuming base install commands | ||
CMD /bin/bash -c "/usr/local/bin/virtualenv -p python3 rtd \ | ||
&& source rtd/bin/activate \ | ||
&& cd /repo \ | ||
&& pip install -r requirements.txt \ | ||
&& npm install \ | ||
&& bower --allow-root install \ | ||
&& npm run vendor \ | ||
&& npm run build" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The command that we are running now is:
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,9 +53,36 @@ may change, so that assets are compiled before deployment, however as our front | |
end assets are in a state of flux, it's easier to keep absolute sources checked | ||
in. | ||
|
||
|
||
Getting Started | ||
--------------- | ||
|
||
We describe two different ways of building front end assets: Either in a | ||
containerized environment (Docker) or step-by-step in your local development | ||
environment. | ||
|
||
Containerized build (Docker) | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
Front end assets can be built in a Docker container. This reduces the steps | ||
required and can be considered more secure, since you won't expose your local | ||
development environment to packages from NPM. | ||
|
||
You will need to install Docker on your system, then run the following commands | ||
from the root of your repository: | ||
|
||
.. code-block:: console | ||
|
||
# Builds the Docker image | ||
docker image build . -t "rtd-frontend" -f docker/frontend.dockerfile | ||
|
||
# Runs the image from the repository root | ||
docker run -v $PWD/:/repo "rtd-frontend" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this should be priority over local build still. We don't have a requirement for docker for contributors yet, so I'd like to avoid making this assumption. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, the heading are off here, I'd rather we keep a level 2 heading for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NPM and security is my concern. We are forcing people to install an unsafe environment directly in the dev runtime. Secondly it's the annoyance of having to set it up, the NPM cache etc. Docker is more disposable. I'd rather explain Docker to a Python developer than Node-js and NPM. I can fix the heading stuff. |
||
|
||
|
||
Local builds | ||
~~~~~~~~~~~~ | ||
|
||
You will need a working version of Node and NPM to get started. We won't cover | ||
that here, as it varies from platform to platform. | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We put files like this in
contrib/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should use
ubuntu:18.04
here which is easier to read.