ncar-xdev / jupyter-forward

Launch Jupyter Lab over SSH with a single command!
https://jupyter-forward.readthedocs.io/
Apache License 2.0
31 stars 12 forks source link

Support for more shells #135

Closed mnlevy1981 closed 2 years ago

mnlevy1981 commented 2 years ago
  1. besides tcsh and bash, #133 makes it clear we need to support sh as well
  2. Can we figure out remote shell from $SHELL (and raise exception mandating use of --shell if that variable is undefined)
  3. one possible path forward would be to create a parent class that contains wrappers for running commands and then shell-specific child classes with correct functionality (e.g. sh -l -c "[run this command]" for sh and bash but sh -c "[run this command]" for tcsh; also '{command} > {self.log_file} 2>&1' for sh and bash but '{command} >& {self.log_file}' for tcsh)
  4. May also need template for expanding variables (e.g. hostname in 'jupyter lab --no-browser --ip=\$(hostname -f)')

Ideally this would let us set up simple tests launching commands other than jupyter so that we can use Github Actions to run these tests and make sure we are supporting as many shells as possible (and that future PRs don't break existing support)

kmpaul commented 2 years ago

What is the real root of this problem? Is it that fabric/paramiko doesn't let you forcefully execute remote commands in a different shell other than the user's default shell? For example, why couldn't we execute all remote commands with bash -c "{command}"?

mnlevy1981 commented 2 years ago

What is the real root of this problem? Is it that fabric/paramiko doesn't let you forcefully execute remote commands in a different shell other than the user's default shell? For example, why couldn't we execute all remote commands with bash -c "{command}"?

bash -c "conda ..." requires a user to have run conda init bash, so it will fail for most tcsh users.

edit: looking at the actual code, I suspect the issue will be that tcsh users will not be able to run bash -c "jupyter lab" since jupyter won't be found in the path (since bash doesn't have conda configured correctly for those users)

kmpaul commented 2 years ago

Ah! I see the problem. So you would need the user to specify something like a --shell option to use conda activate and run all commands in the user's native shell.... Hmm.

Doesn't conda-incubator/setup-miniconda activate conda without conda init? Can't we do something like that?

andersy005 commented 2 years ago

Doesn't conda-incubator/setup-miniconda activate conda without conda init? Can't we do something like that?

During the conda setup, conda-incubator/setup-miniconda invokes conda init --all to initialize all available shells

kmpaul commented 2 years ago

Hmm. I thought there was a way to manually source the conda init script.