Skip to content

Commit

Permalink
chore: remove is_async support (#2344)
Browse files Browse the repository at this point in the history 
* refactor: remove is_async from hook and jsonschema

* docs: add news fragment
  • Loading branch information
@CaselIT
CaselIT authored Sep 27, 2024
1 parent 0c24a18 commit ec13858
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 155 deletions.
5 changes: 5 additions & 0 deletions docs/_newsfragments/2343.breakingchange.rst
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Removed ``is_async`` argument from :meth:`~falcon.media.validators.jsonschema.validate`
and the hooks :meth:`~falcon.before` and :meth:`~falcon.after` since it's
no longer needed.
Cython from 3.0 will correctly mark ``asnyc def`` as coroutine, making
this argument no longer useful.
87 changes: 12 additions & 75 deletions falcon/hooks.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
from falcon._typing import ResponderMethod


# TODO: if is_async is removed these protocol would no longer be needed, since
# TODO: when targeting only 3.10+ these protocol would no longer be needed, since
# ParamSpec could be used together with Concatenate to use a simple Callable
# to type the before and after functions. This approach was prototyped in
# https://github.com/falconry/falcon/pull/2234
Expand Down Expand Up @@ -109,9 +109,7 @@ def __call__(
)


def before(
action: BeforeFn, *args: Any, is_async: bool = False, **kwargs: Any
) -> Callable[[_R], _R]:
def before(action: BeforeFn, *args: Any, **kwargs: Any) -> Callable[[_R], _R]:
"""Execute the given action function *before* the responder.
The `params` argument that is passed to the hook
Expand Down Expand Up @@ -142,21 +140,6 @@ def do_something(req, resp, resource, params):
and *params* arguments.
Keyword Args:
is_async (bool): Set to ``True`` for ASGI apps to provide a hint that
the decorated responder is a coroutine function (i.e., that it
is defined with ``async def``) or that it returns an awaitable
coroutine object.
Normally, when the function source is declared using ``async def``,
the resulting function object is flagged to indicate it returns a
coroutine when invoked, and this can be automatically detected.
However, it is possible to use a regular function to return an
awaitable coroutine object, in which case a hint is required to let
the framework know what to expect. Also, a hint is always required
when using a cythonized coroutine function, since Cython does not
flag them in a way that can be detected in advance, even when the
function is declared using ``async def``.
**kwargs: Any additional keyword arguments will be passed through to
*action*.
"""
Expand All @@ -168,26 +151,22 @@ def _before(responder_or_resource: _R) -> _R:
):
if _DECORABLE_METHOD_NAME.match(responder_name):
responder = cast('Responder', responder)
do_before_all = _wrap_with_before(
responder, action, args, kwargs, is_async
)
do_before_all = _wrap_with_before(responder, action, args, kwargs)

setattr(responder_or_resource, responder_name, do_before_all)

return cast(_R, responder_or_resource)

else:
responder = cast('Responder', responder_or_resource)
do_before_one = _wrap_with_before(responder, action, args, kwargs, is_async)
do_before_one = _wrap_with_before(responder, action, args, kwargs)

return cast(_R, do_before_one)

return _before


def after(
action: AfterFn, *args: Any, is_async: bool = False, **kwargs: Any
) -> Callable[[_R], _R]:
def after(action: AfterFn, *args: Any, **kwargs: Any) -> Callable[[_R], _R]:
"""Execute the given action function *after* the responder.
Args:
Expand All @@ -201,21 +180,6 @@ def after(
arguments.
Keyword Args:
is_async (bool): Set to ``True`` for ASGI apps to provide a hint that
the decorated responder is a coroutine function (i.e., that it
is defined with ``async def``) or that it returns an awaitable
coroutine object.
Normally, when the function source is declared using ``async def``,
the resulting function object is flagged to indicate it returns a
coroutine when invoked, and this can be automatically detected.
However, it is possible to use a regular function to return an
awaitable coroutine object, in which case a hint is required to let
the framework know what to expect. Also, a hint is always required
when using a cythonized coroutine function, since Cython does not
flag them in a way that can be detected in advance, even when the
function is declared using ``async def``.
**kwargs: Any additional keyword arguments will be passed through to
*action*.
"""
Expand All @@ -227,17 +191,15 @@ def _after(responder_or_resource: _R) -> _R:
):
if _DECORABLE_METHOD_NAME.match(responder_name):
responder = cast('Responder', responder)
do_after_all = _wrap_with_after(
responder, action, args, kwargs, is_async
)
do_after_all = _wrap_with_after(responder, action, args, kwargs)

setattr(responder_or_resource, responder_name, do_after_all)

return cast(_R, responder_or_resource)

else:
responder = cast('Responder', responder_or_resource)
do_after_one = _wrap_with_after(responder, action, args, kwargs, is_async)
do_after_one = _wrap_with_after(responder, action, args, kwargs)

return cast(_R, do_after_one)

Expand All @@ -250,11 +212,7 @@ def _after(responder_or_resource: _R) -> _R:


def _wrap_with_after(
responder: Responder,
action: AfterFn,
action_args: Any,
action_kwargs: Any,
is_async: bool,
responder: Responder, action: AfterFn, action_args: Any, action_kwargs: Any
) -> Responder:
"""Execute the given action function after a responder method.
Expand All @@ -264,24 +222,14 @@ def _wrap_with_after(
method, taking the form ``func(req, resp, resource)``.
action_args: Additional positional arguments to pass to *action*.
action_kwargs: Additional keyword arguments to pass to *action*.
is_async: Set to ``True`` for cythonized responders that are
actually coroutine functions, since such responders can not
be auto-detected. A hint is also required for regular functions
that happen to return an awaitable coroutine object.
"""

responder_argnames = get_argnames(responder)
extra_argnames = responder_argnames[2:] # Skip req, resp
do_after_responder: Responder

if is_async or iscoroutinefunction(responder):
# NOTE(kgriffs): I manually verified that the implicit "else" branch
# is actually covered, but coverage isn't tracking it for
# some reason.
if not is_async: # pragma: nocover
async_action = cast('AsyncAfterFn', _wrap_non_coroutine_unsafe(action))
else:
async_action = cast('AsyncAfterFn', action)
if iscoroutinefunction(responder):
async_action = cast('AsyncAfterFn', _wrap_non_coroutine_unsafe(action))
async_responder = cast('AsgiResponderMethod', responder)

@wraps(async_responder)
Expand Down Expand Up @@ -326,7 +274,6 @@ def _wrap_with_before(
action: BeforeFn,
action_args: Tuple[Any, ...],
action_kwargs: Dict[str, Any],
is_async: bool,
) -> Responder:
"""Execute the given action function before a responder method.
Expand All @@ -336,24 +283,14 @@ def _wrap_with_before(
method, taking the form ``func(req, resp, resource, params)``.
action_args: Additional positional arguments to pass to *action*.
action_kwargs: Additional keyword arguments to pass to *action*.
is_async: Set to ``True`` for cythonized responders that are
actually coroutine functions, since such responders can not
be auto-detected. A hint is also required for regular functions
that happen to return an awaitable coroutine object.
"""

responder_argnames = get_argnames(responder)
extra_argnames = responder_argnames[2:] # Skip req, resp
do_before_responder: Responder

if is_async or iscoroutinefunction(responder):
# NOTE(kgriffs): I manually verified that the implicit "else" branch
# is actually covered, but coverage isn't tracking it for
# some reason.
if not is_async: # pragma: nocover
async_action = cast('AsyncBeforeFn', _wrap_non_coroutine_unsafe(action))
else:
async_action = cast('AsyncBeforeFn', action)
if iscoroutinefunction(responder):
async_action = cast('AsyncBeforeFn', _wrap_non_coroutine_unsafe(action))
async_responder = cast('AsgiResponderMethod', responder)

@wraps(async_responder)
Expand Down
31 changes: 2 additions & 29 deletions falcon/media/validators/jsonschema.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@


def validate(
req_schema: Schema = None, resp_schema: Schema = None, is_async: bool = False
req_schema: Schema = None, resp_schema: Schema = None
) -> Callable[[ResponderMethod], ResponderMethod]:
"""Validate ``req.media`` using JSON Schema.
Expand Down Expand Up @@ -51,20 +51,6 @@ def validate(
resp_schema (dict): A dictionary that follows the JSON
Schema specification. The response will be validated against this
schema.
is_async (bool): Set to ``True`` for ASGI apps to provide a hint that
the decorated responder is a coroutine function (i.e., that it
is defined with ``async def``) or that it returns an awaitable
coroutine object.
Normally, when the function source is declared using ``async def``,
the resulting function object is flagged to indicate it returns a
coroutine when invoked, and this can be automatically detected.
However, it is possible to use a regular function to return an
awaitable coroutine object, in which case a hint is required to let
the framework know what to expect. Also, a hint is always required
when using a cythonized coroutine function, since Cython does not
flag them in a way that can be detected in advance, even when the
function is declared using ``async def``.
Example:
Expand Down Expand Up @@ -96,23 +82,10 @@ async def on_post(self, req, resp):
# -- snip --
.. tab-item:: ASGI (Cythonized App)
.. code:: python
from falcon.media.validators import jsonschema
# -- snip --
@jsonschema.validate(my_post_schema, is_async=True)
async def on_post(self, req, resp):
# -- snip --
"""

def decorator(func: ResponderMethod) -> ResponderMethod:
if iscoroutinefunction(func) or is_async:
if iscoroutinefunction(func):
return _validate_async(func, req_schema, resp_schema)

return _validate(func, req_schema, resp_schema)
Expand Down
53 changes: 18 additions & 35 deletions tests/asgi/_cythonized.pyx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,22 @@ from falcon.media.validators.jsonschema import validate


_MESSAGE_SCHEMA = {
'definitions': {},
'$schema': 'http://json-schema.org/draft-07/schema#',
'$id': 'http://example.com/root.json',
'type': 'object',
'title': 'The Root Schema',
'required': ['message'],
'properties': {
'message': {
'$id': '#/properties/message',
'type': 'string',
'title': 'The Message Schema',
'default': '',
'examples': ['hello world'],
'pattern': '^(.*)$'
}
}
'definitions': {},
'$schema': 'http://json-schema.org/draft-07/schema#',
'$id': 'http://example.com/root.json',
'type': 'object',
'title': 'The Root Schema',
'required': ['message'],
'properties': {
'message': {
'$id': '#/properties/message',
'type': 'string',
'title': 'The Message Schema',
'default': '',
'examples': ['hello world'],
'pattern': '^(.*)$'
}
}
}


Expand All @@ -44,19 +44,9 @@ class NOPClass:


class TestResourceWithValidation:
@validate(resp_schema=_MESSAGE_SCHEMA, is_async=True)
async def on_get(self, req, resp):
resp.media = {
'message': 'hello world'
}


class TestResourceWithValidationNoHint:
@validate(resp_schema=_MESSAGE_SCHEMA)
async def on_get(self, req, resp):
resp.media = {
'message': 'hello world'
}
resp.media = {'message': 'hello world'}


class TestResourceWithScheduledJobs:
Expand Down Expand Up @@ -85,7 +75,7 @@ class TestResourceWithScheduledJobsAsyncRequired:
pass

# NOTE(kgriffs): This will fail later since we can't detect
# up front that it isn't a coroutine function.
# up front that it isn't a coroutine function.
resp.schedule(background_job_sync)


Expand All @@ -99,13 +89,6 @@ async def my_after_hook(req, resp, resource):


class TestResourceWithHooks:
@falcon.before(my_before_hook, is_async=True)
@falcon.after(my_after_hook, is_async=True)
async def on_get(self, req, resp):
pass


class TestResourceWithHooksNoHint:
@falcon.before(my_before_hook)
@falcon.after(my_after_hook)
async def on_get(self, req, resp):
Expand Down
Loading

0 comments on commit ec13858

Please sign in to comment.