Type Annotations for Before Hook #2234
Conversation
nZac
force-pushed
the
generic/request-response
branch
2 times, most recently
from
d0b2358 to
c574069
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2234 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 63 63
Lines 6931 6896 -35
Branches 1258 1258
=========================================
- Hits 6931 6896 -35 ☔ View full report in Codecov by Sentry. |
This adds typing information for the before hook as a proof of concept. I used ParamSpec as a means to introspect the hooks arguments to type check the *args and **kwargs arguments. A problem with using ParamSpec is that additional arguments to the before decorator become impossible (so far as I can see) to specify a type. is_async, in needs to come after the positional args for backwards compat, but that is not allowed with ParamSpec. For now, I've just silenced the type error and we can see if there is a better way
nZac
force-pushed
the
generic/request-response
branch
from
c574069 to
46e542c
Compare
setup.cfg
Outdated
| @@ -54,6 +54,7 @@ include_package_data = True | |||
| packages = find: | |||
| python_requires = >=3.7 | |||
| install_requires = | |||
| typing_extensions | |||
There was a problem hiding this comment.
it would be great if we could require this only for a pthon version lower of a certain version.
Since at the moment we need ParamSpec I guess we could make it install only for < 3.10
There was a problem hiding this comment.
Yes, we want to keep Falcon free from third party dependencies.
It is OK to add dependencies for soon EOL 3.8 or 3.9.
| @@ -79,6 +91,8 @@ def do_something(req, resp, resource, params): | |||
| *action*. | |||
| """ | |||
|
|
|||
| is_async = kwargs.get('is_async', False) | |||
There was a problem hiding this comment.
if we keep is_async: bool = False, listed explicitly this line is not needed
There was a problem hiding this comment.
I would like to deprecate is_async now that Cython 3.0 properly marks coroutine functions if run on Python 3.10+. I will create a separate issue for this.
There was a problem hiding this comment.
If we can drop is_async this PR becomes possible. After I pushed this branch I continued my testing and realized this wont work the way I thought it would. I'm not seeing a way to actually type before and after methods in their current state.
I could be missing something though 🤷🏼♂️
|
this was implemented in #2183 |
|
will leave a todo in the code to improve that part |
Summary of Changes
Despite the poorly named branch, this PR brings a brings type annotations to the
beforehook. The approach is a proof of concept and I'll welcome suggestions. I've not typed all the functions beneath it, just the top level API though that should be accomplished at some point. See the commit messages for more details.Related Issues
The existing type issues are closed already, so N/A for now.
Pull Request Checklist
docs/.docs/.versionadded,versionchanged, ordeprecateddirectives.docs/_newsfragments/, with the file name format{issue_number}.{fragment_type}.rst. (Runtowncrier --draftto ensure it renders correctly.)