-
Notifications
You must be signed in to change notification settings - Fork 38
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
Propagate request context to handlers #411
Conversation
c1409ed
to
24c564e
Compare
Thanks, this looks good even outside the context of #409. Will probably merge this PR later tonight. |
As mentioned in #409 (comment), we're using the same database function approach as before with the changeset rate limit. For this reason, this pull request would no longer be a prerequisite for making progress on the topic. I still find the general idea really useful, but would likely merge it only after we have released the next 0.9.x version to keep the amount of code to be backported as small as possible. I hope that's fine for now. |
24c564e
to
3ed2f37
Compare
OAuth 1.0a / Basic auth retirement is now scheduled for July 1st link. That means backporting will no longer be a concern in the near future. |
Thanks, looks good to me now! |
I did some further refactoring in #415. Most notably, I switched back from RequestContext to request for non-HTTP POST/PUT methods, since we're not using any of the non-request fields. Previously, this looked a bit too redundant to me. OTOH, I have replaced the input parameters in validate_user_db_update_permission method by RequestContext, which also happens to be inside the POST/PUT code path. Other changes are listed in the PR. I hope this makes sense. |
Make context information about the request (user id, roles, ...) available in change handlers.
Unblocks future features like #409 (comment)