-
Notifications
You must be signed in to change notification settings - Fork 886
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
request object in pviews misses custom request methods #1104
Comments
I lied: I noticed that pviews did not fail with AttributeError anymore, but I was not paying attention to the output: the matched resource was the root instead of the expected class three traversal levels down, so the fix is definitely more complicated than what I said. |
Alternate fix idea: when the new request object is created in pviews, see if IRequestExtensions is registered and if yes, apply it to the new request. I’ll try that. |
Yeah it's too bad that pviews doesn't use the request from bootstrap, I think it would be a lot cleaner. |
How did this know I meant to close it? |
Opening PR message did say “this should close #1104”... |
I did not know about this gh feature. |
Well I was mostly joking :‑) |
Here’s the setup:
mylib.includeme
callsadd_request_method('user_service_client', get_user_service_client, reify=True)
__getitem__
method that uses request.user_service_client to see if the user ID is found in our user service.After some interesting diving into pyramid.scripting, pyramid.bootstrap and pyramid.scripts.pviews, I found that bootstrap creates a request object and sets the custom attibutes on it (using the IRequestExtensions utility and _set_extensions method), but PViewsCommand._find_view creates a fresh request object. Thus, my custom request property is not defined when my resources get this request object.
I changed _find_view to take the request object from bootstrap (it already takes the registry) and my pviews command worked. 21 tests failed because of the changed signature; I don’t know if not creating a fresh request will cause issues for other pviews usecases, but unless someone objects, I’ll make a pull request with this change and fixed tests.
The text was updated successfully, but these errors were encountered: