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

Environment handling refactoring. #119

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rkdarst
Copy link
Contributor

@rkdarst rkdarst commented Sep 7, 2018

req_keepvars implies that it is for setting environment variables - but to use it, you have to re-add all of the JH defaults. This is not very intuitive, and makes it not very useful.

Rename req_keepvars to req_keepvars_default - this is pre-filled by default with the output from jupyterhub.spawner.Spawner.get_env() which is the environment that JH thinks that singleuser servers need.

Add a new req_keepvars which gets added to req_keepvars_default to become the actual req_keepvars in the batch script. This and previous would probably have helped with #118,

Add a new admin_environment which is added to the environment for running the commands, but not the --export={keepvars} in the scripts. This can be used to authenticate to batch systems in something as an admin. Note that it is not yet guaranteed that this works in all spawners.

Any thoughts on these?

All together these help with #82, considering security.

- Rework req_keepvars.  Old meaning of req_keepvars didn't make sense,
  because it overrode mandatory variables that JupyterHub sets.
  Change to these two variables:
  - req_keepvars_default: Automatically set by JupyterHub defaults,
    should not be overridden.  Old req_keepvars becomes this.  Should
    only be changed if you know what you are doing.
  - req_keepvars: Adds to the defaults in req_keepvars_default.  This
    is what cluster admins usually want to set.
- This is environment which is intended to be passed only to the batch
  commands, not to the user servers.  However, it is unknown if any
  spawners support this.  Don't rely on it unless the spawner
  explicitely says it supports it.
- This fixes up all the previous commits on this branch
@zonca
Copy link
Contributor

zonca commented Oct 1, 2019

thanks, in the next 2-3 weeks I am deploying a batchspawner-based JupyterHub, I'll test this branch then if not merged yet to master.

@rkdarst rkdarst marked this pull request as draft July 23, 2020 12:19
This was referenced Jul 23, 2020
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.

2 participants