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 pre_pty_read_hook in pty_read #136

Merged
merged 3 commits into from
Jan 27, 2022
Merged

Add pre_pty_read_hook in pty_read #136

merged 3 commits into from
Jan 27, 2022

Conversation

Wh1isper
Copy link
Contributor

In jupyter server, last_activity is patched into ptywclients in handler

class TermSocket(WebSocketMixin, JupyterHandler, terminado.TermSocket):
    ...
    def _update_activity(self):
        self.application.settings["terminal_last_activity"] = utcnow()
        # terminal may not be around on deletion/cull
        if self.term_name in self.terminal_manager.terminals:
            self.terminal_manager.terminals[self.term_name].last_activity = utcnow()

If we close the browser or refresh it, these is no socket to update last_activity

Also if we execute some time consuming terminal(like below), last_activity will not be updated

$python
Python 3.8.12 (default, Dec 21 2021, 07:30:29) 
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import time
>>> while True:
...   time.sleep(1)
...   print(1)
... 

So this PR introduces pre_pty_read_hook, which allows the jupyter server's TerminalManager to update the pty when it is reading

in jupyter server (I will create a pr in jupyter server as well)

class TerminalManager(LoggingConfigurable, terminado.NamedTermManager):
    def pre_pty_read_hook(self, ptywclients):
        ptywclients.last_activity = utcnow()

Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Sweet!

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

Successfully merging this pull request may close these issues.

2 participants