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

using aiohttp-jinja2 with nested apps #170

Closed
espositofulvio opened this issue Nov 25, 2017 · 8 comments
Closed

using aiohttp-jinja2 with nested apps #170

espositofulvio opened this issue Nov 25, 2017 · 8 comments

Comments

@espositofulvio
Copy link

Hi,
my understanding is that if I have a sub app I need to register aiohttp-jinja2 for it too in addition to the main app. That is corroborated by the following code:

from aiohttp import web
import jinja2
import aiohttp_jinja2

@aiohttp_jinja2.template( 'index.htm.j2')
async def index( request: web.Request ) -> web.Response:
	return web.Response(  text = 'Test' )

@aiohttp_jinja2.template( 'test.htm.j2' )
async def blog_index( request: web.Request ) -> web.Response:
	return {}

app = web.Application()
aiohttp_jinja2.setup( app, loader = jinja2.PackageLoader( 'catalysis', 'templates' ) )

blog_app = web.Application();
blog_app.router.add_route( 'GET', '/', blog_index )

app.add_subapp( '/blog', blog_app )
app.router.add_route( 'GET', '/', index )

web.run_app( app )

My hunch is that because template() looks for request.app and that is the equal to the nested application it fails to find the jinja2 env object. I thought of setting up aiohttp-jinja2 for each subapp that needs it but what do you think of a function that falls-back to the previous apps in request.match_info.apps? I probably am going for this idea with a custom loader that can also start from the module of the current app and falls-back if the template is not found which gives a nice opportunity for app specific templates that can reference a base template but wanted to check if someone has a better idea.

@popravich
Copy link
Member

This is aiohttp bug, see aio-libs/aiohttp#2550

@espositofulvio
Copy link
Author

espositofulvio commented Nov 27, 2017

While you are right about that bug, this is a bit deeper I guess. The problem here is that the template decorator (actually the render_string() call inside it) uses the request.app variable which is correctly pointing to the blog sub_app in my case. Here I want to inherit the plugin from a parent app without additional setup (which seems fair, if anyone has a different point of view, I'm all ears). I see two options: 1) having render_string() look just in the app that was setup for aiohttp_jinja2 and maybe just check that request.app is a subapp of the setup one (for consistency) or 2) falling back to one of the parent apps in match_info.apps until we found jinja's env. The first choice is difficult as aiohttp_jinja2 is a module and not an object, so we can have a global in the module pointing at the top level app but then we can't override the Env in subapps. Choice number two allows for sub_app to override and setup a different Env and have that picked up if we walk match_info.apps in order from last to first.
What do you think?

@asvetlov
Copy link
Member

In my mind supapp is completely separated application, it should work without any dependencies from main application.
However for convenience aiohttp_jinja2 could search in parent apps for jinja environment. The behavior should be opt-in controlled by new parameter in setup() function, the param can setup a new subapp state like supapp['aiohttp_jinja2_lookup_in_parents'] = True.

Pull Request is welcome!

@popravich
Copy link
Member

However for convenience aiohttp_jinja2 could search in parent apps for jinja environment. The behavior should be opt-in controlled by new parameter in setup() function, the param can setup a new subapp state like supapp['aiohttp_jinja2_lookup_in_parents'] = True.

So the code should look like this?

root_app = web.Application()
sub_app = web.Application()
root_app.add_subapp('/sub', sub_app)

# should be like this 1)
aiohttp_jinja2.setup(root_app)
aiohttp_jinja2.setup(sub_app, lookup_in_parent=True)

# or like this 2)
aiohttp_jinja2.setup(root_app, propagate_to_subapps=True)

# or even worse (don't mind its just a sarcasm):
aiohttp_jinja2.setup(root_app)
root_app.add_subapp(sub_app, aiohttp_jinja2_lookup_in_parent=True)

With the option 1) one must explicitly call aiohttp_jinja2.setup() so I don't see any reason
why it should become more complicated (to lookup in parent apps) instead of setup isolated environment
for sub-application.
With option 2) it is a bit complicated to set aiohttp_jinja2_lookup_in_parents in sub applications, either
aiohttp should patched to allow callbacks on add_subapp call or some hacks should be made in aiohttp_jinja2.

If we end up with option 1) it could be a better choice to share jinja environment instead of
making @template code more complicated.
Also there is another issue with app instance in environment globals:
https://github.com/aio-libs/aiohttp-jinja2/blob/master/aiohttp_jinja2/__init__.py#L32
It doesn't matter if we share environment or look it up in parent apps, app instance in globals must point to proper application.

@espositofulvio
Copy link
Author

I'll see first if I can solve my problem with a mix of jinja loaders, what I wanted to achive is something like this (I'm experimenting, so before going to work on a feature I want to know if there's a requirement for it): I want to write a blog application, then I want to reuse it as a section of different websites. Now, the blog app can define the post and post list templates, but the general layout and style is defined in the parent website. My problem is that the base template will be in a different package and even if I setup jinja for the subapp I won't be able to access the parent as I don't know the package name or the file system layout before-hand as it changes from site to site. If you have a better idea besides sharing the rendering engine between apps I would be very glad to hear.

@asvetlov
Copy link
Member

Sharing an engine sounds like robust option.
We can add setup_with_env(app, env) helper for this where env=aiohttp_jinja2.get_env(main_app)

@code-cro
Copy link
Contributor

code-cro commented Mar 8, 2018

@espositofulvio
Do you maybe need something like this:

aiohttp_jinja2.setup(
    app,
    loader=jinja2.FileSystemLoader([
        './templates',
        './subapps/subapp1/templates',
        './subapps/subapp2/templates',
    ])
)

@asvetlov
Copy link
Member

asvetlov commented Oct 2, 2018

Fixed by #229

@asvetlov asvetlov closed this as completed Oct 2, 2018
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

No branches or pull requests

4 participants