Skip to content

Conversation

@davidism
Copy link
Member

The current AppContext object is passed through the various request dispatch methods, rather than each method accessing the proxies. closes #5815

@pgjones first proposed this in #5229 as a way to speed up dispatch especially for Quart and async views. This PR applies to more methods, and also implements compatibility during a deprecation period.

Dispatch methods now take ctx: AppContext as the first parameter. The following Flask methods were changed:

  • update_template_context
  • handle_http_exception
  • handle_user_exception
  • handle_exception
  • log_exception
  • dispatch_request
  • full_dispatch_request
  • finalize_request
  • make_default_options_response
  • preprocess_request
  • process_response
  • do_teardown_request
  • do_teardown_appcontext

url_for and make_response were not changed, as it's much more likely that these are called from user code that only has access to the proxy.

An __init_subclass__ class method is added to detect old signatures on subclasses of Flask. The second parameter is inspected (first is self). If it is not annotated, it must be named ctx. If it is annotated, it must either be the string or class AppContext. If an old signature is detected, the method is wrapped to remove the argument when other Flask methods call it during dispatch. The base method is also wrapped to inject the argument so that super().base_method from the overridden method will continue to work.

I did not apply the compat wrapper to every base Flask method, only the ones that a subclass overrides. Therefore, if user code is directly calling these internal dispatch methods, they will get a TypeError. This is only likely (and unlikely at that) to happen during testing. I did this over concern that the wrapper would be unnecessary and a performance hit for most applications. If we get bug reports we can consider adding the wrapper.

Copy link
Member

@pgjones pgjones left a comment

Choose a reason for hiding this comment

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

Looks good to me

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pass context internally instead of using contextvars

2 participants