jupyter / terminado

Terminals served by tornado websockets
http://terminado.readthedocs.org/en/latest/
BSD 2-Clause "Simplified" License
368 stars 95 forks source link

Increase scrollback to 1000 #143

Closed yusufbashi closed 2 years ago

yusufbashi commented 2 years ago

Alleviates https://github.com/jupyterlab/jupyterlab/issues/10633. A full solution should take the users choice of scrollback history length into consideration rather than setting a constant value of "maxlen" on line 44.

Wh1isper commented 2 years ago

As the author of https://github.com/jupyter/terminado/pull/127, I had considered this when submitting this PR

  1. Do we really need to save all the output of the terminal, do people want the terminal to be like a file with all the historical information available as long as the session exists. Or do people use the terminal as a tool and don't care much about saving its output (because it can be used inside ipynb ! magic)
  2. If it is necessary to save the terminal output, what kind of approach should be taken and how much output should be saved. Because the form of buffer is based on the number of messages, the user cat a very long file, or echo a very short message actually occupy only one cache, but the network transmission is different (if too much output is saved, the next time it is opened, the user's network overhead may be very high, which will be very painful for users with little bandwidth)

Therefore, for this problem, I think you can appropriately adjust the maxlen to 200 or 500 or more, but over 10,000 may not be a good choice.

yusufbashi commented 2 years ago

I dropped the value to 800 and it still seems to greatly alleviate the issue I linked. 800 is most likely low enough to accommodate users with low bandwidth, but if any bandwidth issues come up we should be able to decrease it further without any ill effects.

fcollonval commented 2 years ago

I think for those kind of need the best answer is no absolute answer - aka making it configurable.

See the best would be to add a new setting in term_settings

https://github.com/jupyter/terminado/blob/8a8337414b7fc84590985ed1732110fa5fd80da8/terminado/management.py#L157

That can be passed to PtyWithClients constructor

https://github.com/jupyter/terminado/blob/8a8337414b7fc84590985ed1732110fa5fd80da8/terminado/management.py#L41

It will then be possible to add a new configurable trait in Jupyter server to tune the value (transferring it to term_settings).

https://github.com/jupyter-server/jupyter_server/blob/745f5ba3f00280c1e1900326a7e08463d48a3912/jupyter_server/terminal/terminalmanager.py#L21

Wh1isper commented 2 years ago

I think for those kind of need the best answer is no absolute answer - aka making it configurable.

See the best would be to add a new setting in term_settings

https://github.com/jupyter/terminado/blob/8a8337414b7fc84590985ed1732110fa5fd80da8/terminado/management.py#L157

That can be passed to PtyWithClients constructor

https://github.com/jupyter/terminado/blob/8a8337414b7fc84590985ed1732110fa5fd80da8/terminado/management.py#L41

It will then be possible to add a new configurable trait in Jupyter server to tune the value (transferring it to term_settings).

https://github.com/jupyter-server/jupyter_server/blob/745f5ba3f00280c1e1900326a7e08463d48a3912/jupyter_server/terminal/terminalmanager.py#L21

I agree. The user can decide how much content to cache, especially since Jupyterhub and other similar sites have

We can implement this in jupyter_server_terminal, configure terminado when jupyter_server_terminal is enabled

fcollonval commented 2 years ago

We can implement this in jupyter_server_terminal, configure terminado when jupyter_server_terminal is enabled

Thanks for pointing that project (I was not aware of it). Is the plan to make jupyter_server depends on it to avoid code duplication?

Wh1isper commented 2 years ago

We can implement this in jupyter_server_terminal, configure terminado when jupyter_server_terminal is enabled

Thanks for pointing that project (I was not aware of it). Is the plan to make jupyter_server depends on it to avoid code duplication?

Yes, you can refer this https://github.com/jupyter-server/jupyter_server/pull/651

jupyter_server_terminal is designed to replace the terminals in jupyter_server

blink1073 commented 2 years ago

The default in iterm2 is 1000, how about we start with that?