Unable to Disable Terminal through 'jupyter_server_config.py`

[rprater]

Description

The jupyter server terminal is not disabled on jupyterhub after setting the jupyter_server_config.py like so:

c.ServerApp.terminals_enabled = False

I can confirm that the configuration is acknowledged by the jupyter server by executing the following command jupyter server --show-config which outputs:

...
Loaded config files:
  /Users/rprater/.jupyter/jupyter_server_config.py

ServerApp
  .jpserver_extensions = <LazyConfigValue value={'jupyter_lsp': True, 'jupyter_server_terminals': True, 'jupyterlab': True, 'nbclassic': True, 'notebook_shim': True}>
  .terminals_enabled = False # <--- acknowledged 

Reproduce

python -m venv venv
source venv/bin/activate
pip install jupyterhub jupyterlab notebook
jupyter server --generate-config
# Edit jupyter_server_config file as described above
jupyterhub

Context

I can confirm this issue does not occur if I pin the jupyter_server package to less than version 2.

Looking at the source code this may make sense, the value gets hard coded here.

[welcome]

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.

You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

[Wh1isper]

Nice catch!

In version 1.x, terminal-related functionality has not yet been fully separated from the Jupyter server: https://github.com/jupyter-server/jupyter_server/blob/1.x/jupyter_server/serverapp.py#L2048

[lp9052]

by looking at the source code, it does respect the terminal config.

However, I believe it should update termina_avaliable to False instead of just return.

nbclassic actually read this field and will fail to launch if the value is missing.

I know they should have a default value for missing fields(issue raised). But I also believe it should be populated, instead of just missing.

Link to nbclassic issue: jupyter/nbclassic#255

[Wh1isper]

I know they should have a default value for missing fields(issue raised). But I also believe it should be populated, instead of just missing.

Agree, this allows us to be consistent with Jupyter Server 1.x behaviour.

https://github.com/jupyter-server/jupyter_server/blob/1.x/jupyter_server/serverapp.py#L363

EDIT: PR opened #105