-
-
Notifications
You must be signed in to change notification settings - Fork 950
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
typing: add middleware protocols #2390
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2390 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 64 64
Lines 7728 7732 +4
Branches 1071 1071
=========================================
+ Hits 7728 7732 +4 ☔ View full report in Codecov by Sentry. |
One thing that we need to investigate (not necessarily related to this change) is how to control annotations in Sphinx. Because if this new union of protocols gets evaluated/expanded, it really clutters the docs... 😬 |
It might be possible to resolve the Sphinx issue via |
Summary of Changes
Typed the middleware classes with union of protocols.
Middleware class may conform to any protocol of the union to pass the type check
It's not perfect due the nature of python's static duck typing, but better than the plain
object
) Class may have correct signature forprocess_request()
, but wrong signature forprocess_response()
. It would be accepted sinceMiddlewareWithProcessRequest
says nothing about theprocess_response
.But if user explicitly states implemented protocols, then the typecheker will ensure strict conformance.
The protocols definitions are in the private
falcon._typing
module now. Should they be moved to the publicfalcon.typing
to support this usecase ?The protocols names are long and noisy. Use some abbreviations ?
Related Issues
#2372