Skip to content

Add base.handlers.get_current_request() #6041

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

Closed
wants to merge 1 commit into from

Conversation

flupke
Copy link

@flupke flupke commented Apr 22, 2021

This function allows to retrieve the current web request in async
contexts.

Retrieving the current context can be useful for example when
customizing Enterprise Gateway RemoteProcessProxy, to retrieve
authentication headers.

The implementation makes uses of Python 3.7's new contextvars module.
There are backports efforts for 3.6:

Since notebook has no control on the event loop it is running into, I
preferred to drop Python 3.6 support.

@flupke flupke force-pushed the add-get-current-request branch from d722d4b to 70e9560 Compare April 22, 2021 12:32
This function allows to retrieve the current web request in async
contexts.

Retrieving the current context can be useful for example when
customizing Enterprise Gateway RemoteProcessProxy, to retrieve
authentication headers.

The implementation makes uses of Python 3.7's new contextvars module.
There are backports efforts for 3.6:

- https://pypi.org/project/aiocontextvars/ - does not have automatic
  asyncio support

- https://pypi.org/project/contextvars/ - which doesn't have it either.
  The tracking issue on this matter has been stale for 3 years now, and
  the conclusion of the discussion is "upgrade to a newer version of
  Python": MagicStack/contextvars#2

Since notebook has no control on the event loop it is running into, I
preferred to drop Python 3.6 support.
@flupke flupke force-pushed the add-get-current-request branch from 70e9560 to c57a166 Compare April 22, 2021 12:57
@flupke flupke marked this pull request as ready for review April 22, 2021 12:58
Comment on lines +943 to +947
def get_current_request():
"""
Get :class:`tornado.httputil.HTTPServerRequest` that is currently being processed.
"""
return _current_request_var.get(None)
Copy link
Author

Choose a reason for hiding this comment

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

Could you please point me to the right place in tests to cover this?

@kevin-bates
Copy link
Member

Hi @flupke. I think this pull request is probably better suited for jupyter_server at this point for the following reasons...

  1. It constitutes new functionality which we're trying not to include in Notebook at this time
  2. Enterprise Gateway is now (master branch) based on jupyter_server for all future releases
  3. I think we should try to continue supporting 3.6 in this project for as long as possible and there may be a better argument for dropping 3.6 in jupyter server

You should find the source bases extremely similar (although the test frameworks vary quite a bit).

As for where to apply test code, and assuming I'm understanding the change, you could probably take a look at the API kernels tests and not await the kernel start request and immediately follow it with one of these requests.

@flupke
Copy link
Author

flupke commented Apr 23, 2021

Ok thanks, I was not aware of this change (we're still using an older version), I will come back once we update :)

@flupke flupke closed this Apr 23, 2021
@flupke flupke deleted the add-get-current-request branch April 23, 2021 10:00
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants