Skip to content
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

Caching of RendererHelper in view_config() #1268

Closed
digitalresistor opened this issue Mar 10, 2014 · 3 comments · Fixed by #1575
Closed

Caching of RendererHelper in view_config() #1268

digitalresistor opened this issue Mar 10, 2014 · 3 comments · Fixed by #1575
Milestone

Comments

@digitalresistor
Copy link
Member

incomplete: Wanted to get the thoughts and findings that @mmerickel were going through down in the bug tracker.

As of right now pyramid_mako has broken the pyramid.reload_templates setting, digging into it a little deeper the issue can be traced back to the renderer factories return being cached in RendererHelper: https://github.com/Pylons/pyramid/blob/master/pyramid/renderers.py#L400

When used with render(), this isn't really an issue since a new RenderHelper is created for each call to it.

However using view_config() the RendererHelper gets cached, so after the first request the return from the renderer factory is reified and the factory is never called again, and the RendererHelper persists.


To support reload_templates the returned function that deals with rendering is actually doing all of the work of verifying the template on disk has changed or not, this basically moves any logic for asset resolving out of a one time thing in the factory back into something that has to be done every time the renderer is called and there is no good way to cache the information so that asset resolving doesn't have to be done each and every call to render.


Is there a better way to solve this, to support reload_templates without having to go through the trouble the renderers currently go through? Is there a way to save the result of an asset lookup, or an effective way to cache it?

@digitalresistor
Copy link
Member Author

Specifically the issue in pyramid_mako as seen from a PR: Pylons/pyramid_mako#20

@mmerickel
Copy link
Member

I think it's surprising that the invocations of p.renderers.render() and the renderer from a add_view(renderer=...) have different behavior. I think it's even more surprising that the IRenderer returned from the factory is cached across requests without really telling anyone. My thought would be to instead pass some renderer_factory to the ViewDeriver, such that it can keep the required package/info/registry. This renderer_factory then would return a new RendererHelper object per-request.

The current behavior of caching the IRenderer at the time of "first render" is also a sneaky little race condition in Pyramid right now, despite probably not technically affecting anyone. I would not expect that my IRenderer code needed to be thread-safe, and if my suggestions are rejected then the documentation should be updated.

@mmerickel
Copy link
Member

Is there a better way to solve this, to support reload_templates without having to go through the trouble the renderers currently go through?

Renderers are intentionally agnostic to whether they are serving asset specs, or files or whatever else, so I doubt much can be done by Pyramid to alleviate this other than providing some helpers that renderers could use to handle this logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants