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

Jira ticket ECS-5104: Automatically close hutch-python sessions that have been idle for 48 hours or more. #383

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions hutch_python/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ def configure_ipython_session(args: HutchPythonArgs):
# Important Utilities
ipy_config.InteractiveShellApp.extensions = [
"hutch_python.ipython_log",
"hutch_python.ipython_session_timer",
"hutch_python.bug",
"hutch_python.pt_app_config"
]
Expand Down
93 changes: 93 additions & 0 deletions hutch_python/ipython_session_timer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
"""
This module modifies an ``ipython`` shell to automatically close if it has been
idle for a certain number of hours. If no command is entered in the ipython instance
for more than the maximum idle time, the session is automatically closed. The
maximum idle time can be updated. Currently, it is set to 48 hours.
"""

import time
from IPython import get_ipython
from threading import Thread


class IPythonSessionTimer:
'''
Class tracks the amount of time the current `InteractiveShell` instance (henceforth
called 'user session') has been idle and closes the session if more than 48
hours have passed.

Time is in seconds (floating point) since the epoch began. (In UNIX the
epoch started on January 1, 1970, 00:00:00 UTC)

Parameters
----------
ipython : ``IPython.terminal.interactiveshell.TerminalInteractiveShell``
The active ``ipython`` ``Shell``, perhaps the one returned by
``IPython.get_ipython()``.

Attributes
----------
curr_time: float
The current time in seconds.

max_idle_time: int
The maximum number of seconds a user session can be idle (currently set
to 172800 seconds or 48 hours).

last_active_time: float
The time of the last user activity in this session.

idle_time: float
The amount of time the user session has been idle.
'''

def __init__(self, ipython):
self.curr_time = 0
self.max_idle_time = 172800
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it'd be good to expose a way for the user to set this. That'd also help us test this feature.

self.last_active_time = 0
self.idle_time = 0

# _set_last_active_time() function will trigger every time user runs a cell
ipython.events.register('post_run_cell', self._set_last_active_time)

def _set_last_active_time(self, result):
self.last_active_time = time.time()

def _get_time_passed(self):
self.curr_time = time.time()
self.idle_time = self.curr_time - self.last_active_time

def _timer(self, sleep_time):
time.sleep(sleep_time)

def _start_session(self):

# Check if idle_time has exceeded max_idle_time
while (self.idle_time < self.max_idle_time):
self._timer(self.max_idle_time - self.idle_time)
self._get_time_passed()

# Close the user session
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if there's some way we can warn the user at regular intervals, e.g. "this session will time out in 2 hours if there is no further input"

print("This hutch-python session has timed out. Please start a new session.")
Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to also use the logger here. Currently this doesn't show up in our log files at all, so the session will terminate and uninformed people won't know what happened.


# Close this ipython session
get_ipython().ask_exit()
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm also not sure if there's a better way. Currently if the session times out, we can actually still request a close with Ctrl+D, though it doesn't matter what we choose.
image

Copy link
Member

Choose a reason for hiding this comment

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

Taking inspiration from the linked github thread, we apparently need to follow this up with another exit call to kill the prompt that's waiting for input, otherwise as you say in the desc this waits for user input before exiting.

ip = get_ipython()
ip.ask_exit()
ip.pt_app.app.exit()



def load_ipython_extension(ipython):
"""
Initialize the `IPythonSessionTimer`.

This starts a timer that checks if the user session has been
idle for 48 hours or longer. If so, close the user session.

Parameters
----------
ip: ``ipython`` ``Shell``
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is copied from ipython_log.py, but the parameter name is wrong and the type hint is... odd? It should probably be the

ipython : ``IPython.terminal.interactiveshell.TerminalInteractiveShell``

from above

Copy link
Member

Choose a reason for hiding this comment

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

The weird thing about IPython is that there are dozens of shell types

The active ``ipython`` ``Shell``, perhaps the one returned by
``IPython.get_ipython()``.
"""
UserSessionTimer = IPythonSessionTimer(ipython)
t1 = Thread(target=UserSessionTimer._start_session, daemon=True)
t1.start()

Loading