[python] Set PIP_USER=no #461
Conversation
|
|
|
@csweichel let's merge #458 before this one, cancel the circle CI build, and merge this PR to release the new images? cc @jankeromnes |
|
@csweichel @aledbf Many thanks! 💯 @AlexTugarev Since you previously worked on the Python setup, please have a look as well if you have time. 🙂 Also, on a more general note, I believe we should soon do two things with our Python setup:
|
| @@ -154,6 +154,7 @@ RUN curl -fsSL https://github.com/pyenv/pyenv-installer/raw/master/bin/pyenv-ins | |||
| mypy autopep8 pep8 pylama pydocstyle bandit notebook \ | |||
| twine \ | |||
| && sudo rm -rf /tmp/* | |||
| ENV PIP_USER=no | |||
There was a problem hiding this comment.
why unsetting it here whereas its set in gitpod-layer on top? my first impression is that this won't be effective.
also, just a reminder, without forcibly enabling PYTHONUSERBASE under /workspace the backup strategy for python is gone, and prebuilds as well. using a virutal env setup seems to be the best way. it would be great to provide a replacement that works.
There was a problem hiding this comment.
@AlexTugarev the gitpod-layer was removed gitpod-io/gitpod#4923. I will add the .bashrc-append changes here.
@jankeromnes, we do? |
We don't? 😳 @csweichel @aledbf do you know? |
It is not enabled yet (there is an issue with the user permissions when we create the workspace backup) |
There was a problem hiding this comment.
Thanks again for improving Gitpod's Python setup! 💯
Blocking this PR until #461 (comment) is resolved, so that we don't accidentally merge it as is.
|
|
|
@csweichel @jankeromnes @AlexTugarev can we merge this PR? Without it, most of the examples are broken when the new image builder mk3 is used (due to the removal of the gitpod layer) |
There was a problem hiding this comment.
Approving to unblock this PR, but I would very very much prefer it if we remove all of these silly variables.
-
For Python, we're unwittingly breaking many people's setups by forcing some PIP_USER or VENV things
-
For Ruby, our setup is so broken that every major Ruby project just replaces it entirely in a Dockerfile (also, we still use RVM instead of today's standard Rbenv)
-
For Rust, the /workspace hack still looks okay, but here too I'd very much prefer removing the hack altogether, and instead enable Full Workspace Backup everywhere
| ENV GEM_HOME=/workspace/.rvm | ||
| ENV GEM_PATH=$GEM_HOME:$GEM_PATH | ||
| ENV PATH=/workspace/.rvm/bin:$PATH |
There was a problem hiding this comment.
I think this does more harm than good. Maybe let's remove it?
| ENV GEM_HOME=/workspace/.rvm | |
| ENV GEM_PATH=$GEM_HOME:$GEM_PATH | |
| ENV PATH=/workspace/.rvm/bin:$PATH |
| ENV PIP_USER=no | ||
| ENV PIPENV_VENV_IN_PROJECT=true | ||
| ENV PYTHONUSERBASE=/workspace/.pip-modules | ||
| ENV PATH=$PYTHONUSERBASE/bin:$PATH |
There was a problem hiding this comment.
I have no Python experience to tell whether setting any of these variables is okay.
However, I don't think you'll find any reviewers here with Python experience, so continuing to wait for a review here is unhelpful.
Also, if possible, I would very much prefer not setting any of these variables. I'm aware that this may break package persistence (they'll land outside of /workspace) but for that I'd rather focus on enabling Full Workspace Backup instead.
There was a problem hiding this comment.
but for that I'd rather focus on enabling Full Workspace Backup instead.
this is one of those issues where we have a circular dependency; we need this PR to switch to the new image builder mk3. With that, we can focus on the dazzle refactor that will reduce the image size and simplify the dependencies tangle, and then we can start using stargz. After that, we should focus on Full Workspace Backup. Without all those previous steps we are not improving the size, usability, or modularity of the workspace container images.
Signed-off-by: Manuel Alejandro de Brito Fontes <aledbf@gmail.com>
fixes gitpod-io/gitpod#4886