Skip to content

Add exclude_last_activity_patterns to prevent some requests from keeping server alive - #609

Open
minrk wants to merge 6 commits into
jupyterhub:mainfrom
minrk:allow-exclude
Open

Add exclude_last_activity_patterns to prevent some requests from keeping server alive#609
minrk wants to merge 6 commits into
jupyterhub:mainfrom
minrk:allow-exclude

Conversation

@minrk

@minrk minrk commented Sep 1, 2026

Copy link
Copy Markdown
Member

allows for excluding certain endpoints from activity that might keep the server alive even when it's really idle.

and allow update_last_activity to be a callable

Checklist

  • I am the author of this work

What does this PR do?

Type of change:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation
  • Maintenance (testing, packaging, infrastructure, metadata)
  • Other

Is this PR related to an issue, or is it part of a larger body of work?

Does this PR introduce a breaking change?

How can this PR be tested?

set exclude_last_activity_patterns and submit requests. last_activity should not be updated.

What should a reviewer concentrate their feedback on?

if this is adequate configuration, or if it should be expressed differently

Other information

cc @shaneknapp who's dealing with lots of idle rstudio sessions not being culled due to constant POST /rstudio/events/get_events

I'll do another PR against rsession-proxy to ignore get_events by default

allows for excluding certain endpoints from activity that might keep the server alive
even when it's really idle.

and allow update_last_activity to be a callable
@shaneknapp

Copy link
Copy Markdown

this is great @minrk! thanks so much!

@ryanlovett ryanlovett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This will be a useful feature, thanks! A couple comments:

  1. In order for downstream proxies to be able to set exclude_last_activity_patterns in their configs (like in the draft PR to jupyter-rsession-proxy), it needs to be a ServerProcess traitlet. I think it should be added to config.py as a traitlet and then passed through in get_proxy_attributes.
  2. Should it be documented that when one sets a callable for update_last_activity, it will cause exclude_last_activity_patterns to be ignored?
  3. Maybe tests can be added in a later PR.

and properly expose exclude_last_activity_patterns via config
@minrk

minrk commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

Added tests and fixed propagation from config, including docstrings explaining that callable update_last_activity is the only thing considered, if defined.

iso date strings are already sortable without parsing (assuming consistent timezone)
@ryanlovett

Copy link
Copy Markdown
Collaborator

Thanks, looks good. The re.compile in handlers.py can probably be removed too, although I think its a no-op if they're already compiled?

already guaranteed in config
@minrk

minrk commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

yeah, should be redundant if we can assume the list always comes via validated config. It is definitely a no-op, re.compile(re.compile(re.compile('pattern'))) is fine.

@minrk minrk closed this Sep 2, 2026
@minrk minrk reopened this Sep 2, 2026
@minrk

minrk commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

tests seem to have become flaky, some due to waiting for proxy processes to start on Windows, and some of the unix socket tests occasionally failing with ECONNREFUSED. Not sure what's going on there

@minrk

minrk commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

ok, tests passing after a re-run

@minrk minrk changed the title Add exclude_last_activity_patterns to exclude Add exclude_last_activity_patterns to prevent some requests from keeping server alive Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants