-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Preserve handler function attributes across middlewares #4195
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4195 +/- ##
==========================================
- Coverage 97.57% 97.56% -0.02%
==========================================
Files 43 43
Lines 8825 8825
Branches 1381 1381
==========================================
- Hits 8611 8610 -1
- Misses 92 93 +1
Partials 122 122
Continue to review full report at Codecov.
|
I modified a unit test to check for this. I don't know whether this behaviour should be documented explicitly. Or else developers will find that it just works, and needs no documentation as such. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The updated test is for application+middleware.
Please provide a test that checks sub-app handler attributes in parent-app middleware.
Pushed a new test_middleware_subapp test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I'll merge it soon; when will have time to generate backport PR as well
Thanks! Well, for me, personally, there is no need to backport it, necessarily. I have an ugly workaround in the code, it can wait a few months. |
Thanks. |
…4195). (cherry picked from commit 2c70eb8) Co-authored-by: Gustavo J. A. M. Carneiro <[email protected]>
…4195). (#4276) (cherry picked from commit 2c70eb8) Co-authored-by: Gustavo J. A. M. Carneiro <[email protected]>
What do these changes do?
When using middlewares (such as the hidden middleware implicitly added by subapps), it fixes the functools.partial() usage on the handler view, to copy any attributes from the view function.
Are there changes in behavior for the user?
Now any attributes added to a view handler function, e.g. by a decorator, are seen by all the middlewares in the chain.
Before this change, attributes we only seen by a middleware in the case of the last middleware (first to run), and only for the main Application, for subapps that wasn't true.
Related issue number
#4174
Checklist
CONTRIBUTORS.txt
CHANGES
folder<issue_id>.<type>
for example (588.bugfix)issue_id
change it to the pr id after creating the pr.feature
: Signifying a new feature..bugfix
: Signifying a bug fix..doc
: Signifying a documentation improvement..removal
: Signifying a deprecation or removal of public API..misc
: A ticket has been closed, but it is not of interest to users.