Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

fix: update dockerfile to use virtualenv and python 3.8 #468

Closed
wants to merge 1 commit into from
Closed

fix: update dockerfile to use virtualenv and python 3.8 #468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ RUN pip3 install --upgrade pip setuptools
RUN rm -rf /var/lib/apt/lists/*

# Python is Python3.
RUN ln -s /usr/bin/python3 /usr/bin/python
ENV VIRTUAL_ENV=/edx/app/registrar/venvs/registrar
RUN python3.8 -m venv $VIRTUAL_ENV
ENV PATH="$VIRTUAL_ENV/bin:$PATH"

# Use UTF-8.
RUN locale-gen en_US.UTF-8
Expand All @@ -39,8 +41,8 @@ WORKDIR /edx/app/registrar

# Copy just Python requirements & install them.
COPY requirements/ /edx/app/registrar/requirements/
COPY Makefile /edx/app/registrar/
RUN make production-requirements
RUN pip install -r requirements/pip.txt
RUN pip install -r requirements/production.txt
Comment on lines +44 to +45
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you revert these lines? It looks like they do (very very nearly) the same thing, from reading the Makefile. (Also helps if the stuff called "production" stays the thing we do in production, and reverting these lines is the easiest way I can see of making sure that's still true in the future.)


USER app

Expand Down
1 change: 1 addition & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ upgrade: piptools $(COMMON_CONSTRAINTS_TXT) ## re-compile requirements .txt fi
mv requirements/test.tmp requirements/test.txt

piptools:
pip install -r requirements/pip.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

@ohnickmoy Can you describe why this was needed and/or how pip and setuptools was installed before this addition in places like devstack or production?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my rationale is based on what was described here: https://github.com/openedx/edx-notes-api/pull/206/files#r499761008 and here: https://github.com/edx/demographics/pull/69/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R36

I believe it's to pin pip and setup tools to a particular version

pip install -r requirements/pip-tools.txt

requirements: devstack-requirements ## alias to make devstack-requirements
Expand Down
2 changes: 2 additions & 0 deletions requirements/pip.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pip==20.2.3
setuptools==50.3.0