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

Pip no cache dir #216

Merged
merged 4 commits into from
Oct 7, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
6 changes: 3 additions & 3 deletions f34-py39/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Thoth's extension to OpenShift's S2I build
FROM registry.fedoraproject.org/f34/python3:latest
FROM registry.fedoraproject.org/f34/python3:0-31.container

ENV SUMMARY="Thoth's Source-to-Image for Python ${PYTHON_VERSION} applications" \
DESCRIPTION="Thoth's Source-to-Image for Python ${PYTHON_VERSION} applications. This toolchain is based on Fedora 34. It includes Pipenv." \
Expand Down Expand Up @@ -30,8 +30,8 @@ COPY ./requirements.txt $HOME/requirements.txt
RUN TMPFILE=$(mktemp) && \
TMPFILE_ASSEMBLE=$(mktemp) && \
pushd "${STI_SCRIPTS_PATH}" && patch -p 1 </tmp/s2i_assemble.patch && popd && \
pip3 install -U "pip==20.3.3" && \
/usr/bin/pip3 install -U "pip==20.3.3" && \
pip3 --no-cache-dir install -U "pip==20.3.3" && \
/usr/bin/pip3 --no-cache-dir install -U "pip==20.3.3" && \
curl https://raw.githubusercontent.com/thoth-station/micropipenv/master/micropipenv.py | MICROPIPENV_NO_LOCKFILE_WRITE=1 MICROPIPENV_PIP_BIN=/usr/bin/pip3 /usr/bin/python3 - install -- && \
curl "https://raw.githubusercontent.com/thoth-station/s2i-thoth/master/assemble" -o "${TMPFILE_ASSEMBLE}" && \
cp "${STI_SCRIPTS_PATH}/assemble" "${TMPFILE}" && \
Expand Down
32 changes: 20 additions & 12 deletions f34-py39/s2i_assemble.patch
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
--- a/assemble 2020-10-31 11:55:38.000000000 +0100
+++ b/assemble 2020-11-25 22:32:16.739312082 +0100
@@ -18,38 +18,8 @@
fi
--- a/assemble 2021-09-12 14:46:51.130401513 -0400
+++ b/assemble 2021-09-12 14:49:27.881361622 -0400
@@ -14,38 +14,8 @@
python3.9 -m venv $1
}

-# Install pipenv or micropipenv to the separate virtualenv to isolate it
-# from system Python packages and packages in the main
-# virtualenv. Executable is simlinked into ~/.local/bin
Expand All @@ -27,7 +27,7 @@
-}
-
set -e

-# First of all, check that we don't have disallowed combination of ENVs
-if [[ ! -z "$ENABLE_PIPENV" && ! -z "$ENABLE_MICROPIPENV" ]]; then
- echo "ERROR: Pipenv and micropipenv cannot be enabled at the same time!"
Expand All @@ -39,10 +39,10 @@
shopt -s dotglob
echo "---> Installing application source ..."
mv /tmp/src/* "$HOME"
@@ -57,16 +27,6 @@
@@ -53,50 +23,19 @@
# set permissions for any installed artifacts
fix-permissions /opt/app-root -P

-# We have to first upgrade pip to at least 19.3 because:
-# * pip < 9 does not support different packages' versions for Python 2/3
-# * pip < 19.3 does not support manylinux2014 wheels. Only manylinux2014 wheels
Expand All @@ -55,11 +55,14 @@
-
if [[ ! -z "$UPGRADE_PIP_TO_LATEST" ]]; then
echo "---> Upgrading pip to latest version ..."
if ! pip install -U pip setuptools wheel; then
@@ -75,28 +35,7 @@
- if ! pip install -U pip setuptools wheel; then
+ if ! pip install --no-cache-dir -U pip setuptools wheel; then
echo "WARNING: Installation of the latest pip,setuptools and wheel failed, trying again from official PyPI with pip --isolated install"
- pip install --isolated -U pip setuptools wheel
+ pip install --no-cache-dir --isolated -U pip setuptools wheel
fi
fi

-if [[ ! -z "$ENABLE_PIPENV" ]]; then
- if [[ ! -z "$PIN_PIPENV_VERSION" ]]; then
- # Add == as a prefix to pipenv version, if defined
Expand All @@ -83,6 +86,11 @@
- pip install -r requirements.txt
-fi
+thamos install
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
+thamos install
+thamos install -- --disable-pip-version-check --no-cache-dir

thamos uses pip under the hood, it can pass any options to pip as necessary:

https://github.com/thoth-station/thamos/#installing-requirements

When using environment variables (see bellow), the options would be automatically propagated.


if [[ -f setup.py && -z "$DISABLE_SETUP_PY_PROCESSING" ]]; then
echo "---> Installing application ..."
- pip install .
+ pip install --no-cache-dir .
Copy link
Contributor

Choose a reason for hiding this comment

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

We could eventually also include --disable-pip-version-check. Here and in other install parts above:

Suggested change
+ pip install --no-cache-dir .
+ pip install --no-cache-dir --disable-pip-version-check .

By the way, pip also supports passing options as environment variables:

https://pip.pypa.io/en/stable/topics/configuration/#environment-variables

We could set all the options globally and all the pip calls would inherit this configuration in the container image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hum, good question...
Personal opinion: it's faster and more elegant to use the env vars. However, people looking at the assemble file won't necessarily check the Dockerfile (or whatever other means to pass those variables), so it may become difficult to understand what's going on with half the config at one place and the other somewhere else.
@fridex , any counter-arguments? 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

@fridex , any counter-arguments?

Out of bullets 🔫 👍🏻

fi

if should_collectstatic; then