Fix pre-commit in the image #507
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danielhollas
changed the title
/etc/pip.conf -> /opt/conda/pip.conf
danielhollas
changed the title
/etc/pip.conf -> /opt/conda/pip.conf/etc/pip.conf to /opt/conda/pip.conf
danielhollas
changed the title
/etc/pip.conf to /opt/conda/pip.conf
|
@PNOGillespie could you please take a look and see if the image Tagging multiple people for review so that we're all aware that this pip configuration exists. |
danielhollas
requested review from
unkcpz,
yakutovicha,
PNOGillespie and
superstar54
PNOGillespie
approved these changes
Nov 13, 2024
unkcpz
reviewed
Nov 13, 2024
| @@ -42,8 +42,19 @@ RUN cat /opt/requirements.txt | xargs -I{} conda config --system --add pinned_pa | |||
|
|
|||
| # Configure pip to use the same requirements file as constraints file. | |||
| ENV PIP_CONSTRAINT /opt/requirements.txt | |||
| # Ensure that pip installs packages to ~/.local by default | |||
| COPY pip.conf /etc/pip.conf | |||
| # Ensure that pip installs packages to '~/.local/lib/python3.X/site-packages/' by default | |||
There was a problem hiding this comment.
Is this means when python version changed in the image, dependencies of apps need to be reinstalled?
There was a problem hiding this comment.
Indeed, see #317
unkcpz
approved these changes
Nov 13, 2024
There was a problem hiding this comment.
Thanks @danielhollas
One question in comment. The changes looks all good.
Comment on lines
+52
to
+57
| # Other locations such as '~/.config/pip/pip.conf' or '/etc/pip.conf' would interfere with virtual environments, | ||
| # for example those used by pre-commit. | ||
| # We can't use the PIP_USER env variable for the same reason. | ||
| # To better understand this, try running `pip config debug` and see | ||
| # https://github.com/aiidalab/aiidalab-docker-stack/issues/501 | ||
| COPY pip.conf "${CONDA_DIR}/pip.conf" |
|
@PNOGillespie thank you very much for testing, and thank you @unkcpz for review! I am going to merge this in the afternoon and release a new image version (which will also include AiiDA 2.6.3) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes pre-commit by moving the pip.conf configuration (which automatically sets the
--useroption topip install) from the globaletc/pip.confto site-specific/opt/conda/pip.conf. In this way, the configuration does not mess with virtual environments. See the long comment in the Dockerfile for detailed explanation.Fixes #501