#273: Added jupyterenv/bin to environment variable PATH inside the Entrypoint #314
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
ckunki
had a problem deploying
to
approve-test-execution
GitHub Actions
Failure
ckunki
had a problem deploying
to
approve-test-execution
GitHub Actions
Failure
rather than relying on shell variable expansion
ckunki
had a problem deploying
to
approve-test-execution
GitHub Actions
Failure
ckunki
changed the title
ckunki
had a problem deploying
to
approve-test-execution
GitHub Actions
Failure
ckunki
had a problem deploying
to
approve-test-execution
GitHub Actions
Failure
ckunki
had a problem deploying
to
approve-test-execution
GitHub Actions
Failure
tomuben
previously approved these changes
Aug 9, 2024
tkilias
requested changes
Aug 10, 2024
| @@ -175,7 +183,8 @@ def _commit_container( | |||
| "Env": [ | |||
| f"VIRTUAL_ENV={virtualenv}", | |||
| f"NOTEBOOK_FOLDER_FINAL={notebook_folder_final}", | |||
| f"NOTEBOOK_FOLDER_INITIAL={notebook_folder_initial}" | |||
| f"NOTEBOOK_FOLDER_INITIAL={notebook_folder_initial}", | |||
| f"PATH={path}", | |||
There was a problem hiding this comment.
This can not go here, because the we can not activate the the environment for the whole system. It would be also for the wrong user. This needs to go into the entry point because there we are calling the correct python version and only there this addition to PATH is valid.
There was a problem hiding this comment.
We actually need to do the same for the AMI and VMs, this means we should use the entry point in the systemd unit. We already have a ticket for that. Please, work on that afterwards.
ckunki
had a problem deploying
to
approve-test-execution
GitHub Actions
Failure
tkilias
reviewed
Aug 12, 2024
| @@ -75,6 +79,11 @@ def arg_parser(): | |||
| return parser | |||
|
|
|||
|
|
|||
| def command_with_venv(venv_activate: Path, command: List[str]) -> List[str]: | |||
There was a problem hiding this comment.
Ah - left over from an experiment.
Thanks!
Removed in next push.
tkilias
reviewed
Aug 12, 2024
doc/changes/changes_2.1.0.md
Outdated
| @@ -18,6 +18,7 @@ Version: 2.1.0 | |||
| * #279: Made the notebooks tests running in SaaS as well as in the Docker-DB. | |||
| * #19: Added SLC notebook | |||
| * #301: Added CloudFront distribution for example data S3 bucket | |||
| * #273: Added `jupyterenv/bin` to environment variable `PATH` inside the DockerContainer | |||
There was a problem hiding this comment.
Suggested change
| * #273: Added `jupyterenv/bin` to environment variable `PATH` inside the DockerContainer | |
| * #273: Added `jupyterenv/bin` to environment variable `PATH` inside the Entrypoint |
There was a problem hiding this comment.
Yes, thanks!
See next push.
tkilias
changed the title
ckunki
had a problem deploying
to
approve-test-execution
GitHub Actions
Failure
ckunki
had a problem deploying
to
approve-test-execution
GitHub Actions
Failure
ckunki
had a problem deploying
to
approve-test-execution
GitHub Actions
Failure
tkilias
approved these changes
Aug 13, 2024
ckunki
temporarily deployed
to
approve-test-execution
GitHub Actions
Inactive
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.
Closes #273
pipin/usr/bin/pip/home/jupyter/jupyterenv/binneeds to be prepended to environment variablePATHin order to override the system-wide binarypython -m pip would also be finesincepythonandpipboth are aware of the directory of the initially called binaryActually, inside a Jupyter notebook the Jupyter virtual env should be activated anyway, but we have observations not confirming this assumption.