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

Add a topic on jupyter-lmod in documentation #640

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

cmd-ntrf
Copy link
Contributor

This is a first draft of new topic for Lmod's documentation and address issue #639.

Comments and thoughts are welcomed.

@cmd-ntrf cmd-ntrf requested review from lexming and wpoely86 and removed request for lexming March 27, 2023 14:33
@cmd-ntrf cmd-ntrf marked this pull request as ready for review March 29, 2023 18:31
@cmd-ntrf cmd-ntrf requested review from wpoely86 and lexming and removed request for wpoely86 and lexming March 29, 2023 18:33
Copy link

@lexming lexming left a comment

Choose a reason for hiding this comment

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

Looks good, I only have a few minor comments. Please see below

Jupyter and Lmod
================

It is possible to use Lmod inside of Jupyter with the terminal or jupyter-lmod.
Copy link

Choose a reason for hiding this comment

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

Maybe add a link to your repo from the start 🙂

Suggested change
It is possible to use Lmod inside of Jupyter with the terminal or jupyter-lmod.
It is possible to use Lmod inside of Jupyter with its terminal or the Jupyter extension `jupyter-lmod <https://github.com/cmd-ntrf/jupyter-lmod>`_.

Comment on lines +29 to +32
jupyter-lmod uses lmod python interface to generate Python code
that modifies the environment variables of the Python process running Jupyter. All child
processes of Jupyter (including kernels) created after Lmod calls inherit
the environment variables as defined by the set of loaded modules.
Copy link

Choose a reason for hiding this comment

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

This sentence is a bit confusing, I suggest to simplify it:

Suggested change
jupyter-lmod uses lmod python interface to generate Python code
that modifies the environment variables of the Python process running Jupyter. All child
processes of Jupyter (including kernels) created after Lmod calls inherit
the environment variables as defined by the set of loaded modules.
jupyter-lmod uses lmod python interface to generate Python code
that modifies the environment variables of the Python process running Jupyter. All child
processes of Jupyter (*e.g.* newly spawned kernels) will inherit the environment
as currently defined by the set of loaded modules.

Comment on lines +51 to +53
If the environment variables are defined before JupyterHub launch, you can add their
names only to `c.Spawner.env_keep <https://jupyterhub.readthedocs.io/en/stable/api/spawner.html#jupyterhub.spawner.Spawner.env_keep>`_
instead.
Copy link

Choose a reason for hiding this comment

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

Technically these variable are only needed in the single-user server. JupyterHub itself does not need them.

Suggested change
If the environment variables are defined before JupyterHub launch, you can add their
names only to `c.Spawner.env_keep <https://jupyterhub.readthedocs.io/en/stable/api/spawner.html#jupyterhub.spawner.Spawner.env_keep>`_
instead.
If the environment variables are already defined before JupyterHub is launched
or before the single-user server starts (Jupyter Notebook or JupyterLab),
you can propagate these variable by adding their names to
`c.Spawner.env_keep <https://jupyterhub.readthedocs.io/en/stable/api/spawner.html#jupyterhub.spawner.Spawner.env_keep>`_ instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants