Pip no cache dir #216
Pip no cache dir #216
Conversation
sesheta
added
the
needs-ok-to-test
|
Hi @guimou. Thanks for your PR. I'm waiting for a thoth-station member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
sesheta
added
the
size/M
|
/lgtm |
|
/ok-to-test |
sesheta
added
ok-to-test
and removed
needs-ok-to-test
|
/assign @guimou please have a look at the pre-commit issues, thx!! |
f34-py39/s2i_assemble.patch
Outdated
| @@ -83,6 +86,11 @@ | |||
| - pip install -r requirements.txt | |||
| -fi | |||
| +thamos install | |||
There was a problem hiding this comment.
| +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.
f34-py39/s2i_assemble.patch
Outdated
| if [[ -f setup.py && -z "$DISABLE_SETUP_PY_PROCESSING" ]]; then | ||
| echo "---> Installing application ..." | ||
| - pip install . | ||
| + pip install --no-cache-dir . |
There was a problem hiding this comment.
We could eventually also include --disable-pip-version-check. Here and in other install parts above:
| + 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.
There was a problem hiding this comment.
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? 😉
There was a problem hiding this comment.
@fridex , any counter-arguments?
Out of bullets 🔫 👍🏻
I'd like to, but when I click on the Details link it opens the Prow page but it is empty. I only get the tile "pre-commit #1441629034663383040" then nothing... |
You can run pre-commit locally until these issues get resolved. |
|
New changes are detected. LGTM label has been removed. |
sesheta
added
size/S
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fridex The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
sesheta
added
the
approved
This introduces a breaking change
This Pull Request implements
registry.fedoraproject.org/f34/python3. Withlatestthe assemble patch was not valid anymore. Other solution would be to constantly maintain the assemble patch to reflect changes in base image.no-cache-diroption to the all the pip install to minimize the image size.