Define a CURRENT_JUPYTER_HANDLER context var

[Zsailer]

Fixes #1250

This PR defines a CURRENT_JUPYTER_HANDLER context variable that provides access to the current JupyterHandler context from within any manager, extension, etc.

This is a super simple way to enable e.g. manager methods to access information/context about the server request that called the method. The most compelling use-case might be if you want to get the current_user from within a manager's method.

To do this, simply import the context variable and call .get(). For example, if you want to know the current_user within the start_kernel method of the MappingKernelManager, it might look like:

from jupyter_server.base.handlers import CURRENT_JUPYTER_HANDLER


class CustomAsyncMappingKernelManager:

    def start_kernel(...):
        # Basic example for fetching current user.
        handler = CURRENT_JUPYTER_HANDLER.get()
        user = hander.current_user

I've included a fairly round about unit test to demonstrate that the variable remains isolated per request.

[codecov]

Codecov Report

Patch coverage: 93.10% and project coverage change: +0.01 🎉

Comparison is base (87b2158) 80.49% compared to head (ba30c15) 80.51%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1251      +/-   ##
==========================================
+ Coverage   80.49%   80.51%   +0.01%     
==========================================
  Files          68       69       +1     
  Lines        8270     8299      +29     
  Branches     1601     1607       +6     
==========================================
+ Hits         6657     6682      +25     
- Misses       1191     1194       +3     
- Partials      422      423       +1     

Impacted Files	Coverage Δ
jupyter_server/base/call_context.py	92.30% <92.30%> (ø)
jupyter_server/__init__.py	91.66% <100.00%> (+0.75%)	⬆️
jupyter_server/base/handlers.py	78.94% <100.00%> (+0.08%)	⬆️

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[kevin-bates]

This is great @Zsailer - thank you!

I was thinking this would be a bit more generic where the current handler would be one item available and with which managers (and anything else) could add their own items into.

Perhaps we could have CURRENT_CONTEXT where CURRENT_CONTEXT.get() returns a dictionary where the items are pieces of the context. Some items, like handler, would be agreed upon, but others could be relative to the service. Each of these keys should probably be namespaced/prefixed by the service and perhaps handler should be request_handler or jupyter_handler. This might require subclassing contextvars (which we should probably do anyway).

For activity that is generated from within the server (like periodic callbacks), they may want to set some state as well since the request_handler "state" won't exist in those cases.

[kevin-bates]

Hi @Zsailer - I've gone ahead and pushed the CallContext class implementation. This essentially maintains a dictionary-based ContextVar that is the set of variables the application wishes to make available in a context-sensitive manner.

I had originally implemented it as a dictionary of name/contextVar pairs but because the dictionary instance itself should be context-sensitive, it seemed a little redundant to have each variable (those desired by the application and the mapping itself) manifested as context variables. We can revisit that if necessary - the tests passed either way.

I did leave a TODO about what to return from CallContext.get("var_name") when var_name hasn't been set. Currently, the method returns None, but I'm wondering if we should make this more like ContextVar behaves and raise LookupError (or a custom subclass thereof).

I'm happy to circle back to the main description and title to reflect these changes but thought I should first wait to make sure we're on the same page here.

[echarles]

This is a super simple way to enable e.g. manager methods to access information/context about the server request that called the method. The most compelling use-case might be if you want to get the current_user from within a manager's method.

• Will the information be persisted across user HTTP requests?
• Is the information per "authenticated" user?

e.g. Can this be used to implement a HTTP request counter per user (USER_A has hit the endpoints 5 times since the start of his/her session which is 2 days ago, while USER_B has hit the endpoints 45643 times since the start of his/her session which is 1 hour ago).

[kevin-bates]

Hi @echarles.

• Will the information be persisted across user HTTP requests?
• Is the information per "authenticated" user?

No. This is purely relative to the current request and is essentially user-agnostic (although user-specific by definition). It's tantamount to thread-local storage.

If current session behavior doesn't account for this, I think it would make sense to extend SessionManager to accommodate this kind of functionality since it already fronts a persistent store. In fact, one of the things we could set into the CallContext is a session indicator (ID) that can be used to access this more persistent (request-spanning) state. If that information is part of each request, then it can already be pulled from the JUPYTER_HANDLER entry - which is the request handler instance.

[Zsailer]

I know we briefly discussed this, but thinking about this more—

I find it a bit confusing that the contextvar is in the "sessions" module. Annoyingly, jupyter_server uses "sessions" to mean a few different things. The sessions module+service, however, is specifically about notebook-kernel mapping, right? It's not related to anything about the user, request, or context.

That said, I don't know a better place for this. Maybe we need a "session" outside of "services" module? Or we can name it something else? Either way, I think the mixing of what we mean by "session" in this PR is adding the confusion.

[kevin-bates]

Hmm - ok. I was thinking this would be associated with the longer-term stuff we wanted to do with session - essentially what @echarles was asking about here.

I'm okay moving this but don't have anything I believe to be a better location/name. I would, though, like to see the server become more session-centric and really believe that will be necessary when moving forward in multi-user/tenancy.

[blink1073]

Maybe call it http_session to differentiate and make it framework-agnostic?

[Zsailer]

Maybe call it http_session to differentiate and make it framework-agnostic?

I like this. And we should put it at the top level, next to base, services, etc.?

[kevin-bates]

But this is available to any application and services (sans REST API). As a result, I don't think http_ is very appropriate and find the use of session here to be similarly (per the previous comment) confusing - sorry.

make it framework-agnostic

What does this mean in this context?

[blink1073]

I meant framework in terms of tornado, fastapi, etc.

[kevin-bates]

make it framework-agnostic

What does this mean in this context?

I meant framework in terms of tornado, fastapi, etc.

Thanks. The file is framework-agnostic. Does its location in the package change that designation?

And we should put it at the top level, next to base, services, etc.?

If we were to place this at the top level (and not in services/sessions) what directory would it reside in? It feels pretty "basey" so I guess I'd prefer base over creating another name that holds one file.

[Zsailer]

I think jupyter_server/base/call_context.py seems like a good compromise. Thoughts?

[Zsailer]

BTW, I didn't say this earlier but @kevin-bates this CallContext object is sweet! Well done! 🚀

[kevin-bates]

I think jupyter_server/base/call_context.py seems like a good compromise. Thoughts?

Yeah, that sounds good. As this evolves we might want to move it. Hmm, in that case, would it make sense to add an entry into https://github.com/jupyter-server/jupyter_server/blob/main/jupyter_server/__init__.py so folks wouldn't need to adjust import statements?

from .base.call_context import CallContext

and consumer imports would be:

from jupyter_server import CallContext

Or is this bad practice?

[Zsailer]

Yeah, I think that seems reasonable enough to me.

[kevin-bates]

OK - I'll go ahead and push these relocation changes...

[Zsailer]

This LGTM! Thanks, @kevin-bates

[echarles]

I think jupyter_server/base/call_context.py seems like a good compromise. Thoughts?

I like it. It is less confusing as not related to any session (nor http, nor kernel)

[Zsailer]

@kevin-bates I'll let you do the honors of merging, since I opened the initial PR 😎

[dualc]

I found some bugs about CallContext when I changed this jupyter to suppor multi-user/tenancy ，
`
@classmethod
def _get_map(cls) -> Dict[str, Any]:
"""Get the map of names to their values from the _NAME_VALUE_MAP context var.

    If the map does not exist in the current context, an empty map is created and returned.
    """
    ctx: Context = copy_context()
    if CallContext._name_value_map not in ctx:
        CallContext._name_value_map.set({})
    return CallContext._name_value_map.get()

`

copy_context is a shallow copy，_name_value_map is a dict，so when multi user visite jupyter，if we record some user infomation in , it will casue context values bleed

[Zsailer]

@dualc would you like to open an issue for this so we can track and test? I'm happy to do it to if you prefer.

[dualc]

@dualc would you like to open an issue for this so we can track and test? I'm happy to do it to if you prefer.

ok，#1462