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

Method `render_template` does not use blueprint specified `template_folder` #1361

Closed
Kevin922 opened this issue Mar 3, 2015 · 57 comments

Comments

Projects
None yet
@Kevin922
Copy link

commented Mar 3, 2015

  1. File structure:
project -
      - templates \
                - a \
                      search.html
                - b \
                      search.html
      - start.py
      - myapplication \
                - test.py 
  1. Start file:
# start.py
from flask import Flask
from myapplication.test import test_blueprint

app = Flask(__name__)
app.register_blueprint(test_blueprint)
  1. my application
# test.py
from flask import Blueprint, render_template, abort
from jinja2 import TemplateNotFound

test_blueprint = Blueprint('test_blueprint', __name__,
                        template_folder='absolute_path_to_project/templates/a')  
                        # YES, I specified the absolute path to the template a

@test_blueprint.route('/test')
def show():
    try:
        return render_template(''search.html") # HERE is the problem
    except TemplateNotFound:
        abort(404)

Is this a problem?
# Actually, I want to render a/search.html
# But, render_template() does not use test_blueprint's template_folder
# render_template() search the template list, find the first search.html, then render
# So, maybe render a/search.html or b/search.html
# This depend on the sequence of the template list

@untitaker

This comment has been minimized.

Copy link
Member

commented Mar 29, 2015

What is the output, and what did you expect?

@alanhamlett

This comment has been minimized.

Copy link
Contributor

commented Jun 23, 2015

The render_template function should look like this:

def render_template(template_name_or_list, **context):
    """Renders a template from the template folder with the given
    context.
    :param template_name_or_list: the name of the template to be
                                  rendered, or an iterable with template names
                                  the first one existing will be rendered
    :param context: the variables that should be available in the
                    context of the template.
    """
    ctx = _app_ctx_stack.top
    ctx.app.update_template_context(context)

    template = None

    if (request.blueprint is not None and
        isinstance(template_name_or_list, string_types)):
        blueprint_template_folder = current_app.blueprints[request.blueprint]
        if blueprint_template_folder is not None:
            template = ctx.app.jinja_env.get_or_select_template(
                os.path.join(blueprint_template_folder, template_name_or_list))

    if template is None:
        template = ctx.app.jinja_env.get_or_select_template(template_name_or_list)

    return _render(template, context, ctx.app)

Alternatively we could modify the Jinja2 context so get_or_select_template looks in the blueprint's template folder first.

@ael-code

This comment has been minimized.

Copy link

commented Jul 8, 2015

I encountered the same bug. This is a huge bug that could brake a lot of blueprints.

I build up a simple example to reproduce the bug. You can look at the comments inside code for more details.
https://github.com/ael-code/flask_bugs

@dagostinelli

This comment has been minimized.

Copy link

commented Jul 12, 2015

I suspect that I'm running into this as well. Is this just the way it works?

Maybe it would help matters if the stock Blueprint example was updated to show the intended way to run multiple blueprints. The existing example doesn't seem to address what we are asking about in this issue. I think it could be updated to show two different Blueprints, running side by side, with their own copy of /templates. They both should have an index.html showing how to load one or the other from the Blueprint's-specified templates folder. That way, we will understand clearly what is the intended way to do this.

I'd do it myself, except I don't know if what I'm writing below is the intended way or if there actually is a bug here.

It feels like:

  • flask.render_template ought to be searching the blueprint's template_folder first and then fall back to app.template_folder (and deal with the subsequent problem of caching internally)
  • or maybe the Blueprint class should have a render_template works like that (search the blueprint's template_folder first and then fall back to app.template_folder.

For my project, I am currently doing things as follows. This is working, but it's messy because everything in mixed together in the /templates folder.

/myapp
  /blueprint1
    views.py
  /blueprint2
    views.py
  /templates
    /blueprint1
      index.html
    /blueprint2
      index.html

blueprint1/views.py

bp = Blueprint('blueprint1', __name__, static_folder='static', template_folder='templates")
@bp.route("/")
def index():
    return flask.render_template('blueprint1/index.html')

blueprint2/views.py

bp = Blueprint('blueprint2', __name__, static_folder='static', template_folder='templates")
@bp.route("/")
def index():
    return flask.render_template('blueprint2/index.html')

See how we have to specify the name of the blueprint path in render_template?

I was hoping for this to work instead:

blueprint1/views.py

bp = Blueprint('blueprint1', __name__, static_folder='static', template_folder='blueprint1/templates")
@bp.route("/")
def index():
    return flask.render_template('index.html')

blueprint2/views.py

bp = Blueprint('blueprint2', __name__, static_folder='static', template_folder='blueprint2/templates")
@bp.route("/")
def index():
    return flask.render_template('index.html')

An improvement is to add a render_template() function to the Blueprint object that is guaranteed to search the template_folder first and fall back to app.template_folder if it doesn't exist.

bp = Blueprint('blueprint2', __name__, static_folder='static', template_folder='blueprint2/templates")
@bp.route("/")
def index():
    return bp.render_template('index.html')

If I had a vote, I'd vote for this last idea because I think it's the most clear.

@JelteF

This comment has been minimized.

Copy link

commented Jul 15, 2015

I was running into the same problem, I have changed the code supplied by @alanhamlett to work with the way paths were supposed to be supplied to blueprints.

def render_template(template_name_or_list, **context):
    """
    Renders a template from the template folder with the given
    context.
    :param template_name_or_list: the name of the template to be
                                  rendered, or an iterable with template names
                                  the first one existing will be rendered
    :param context: the variables that should be available in the
                    context of the template.
    """
    ctx = _app_ctx_stack.top
    ctx.app.update_template_context(context)

    template = None

    if request.blueprint is not None and \
            isinstance(template_name_or_list, string_types):
        bp = current_app.blueprints[request.blueprint]
        try:
            template = bp.jinja_loader.load(ctx.app.jinja_env,
                                            template_name_or_list,
                                            ctx.app.jinja_env.globals)
        except TemplateNotFound:
            pass

    if template is None:
        template = ctx.app.jinja_env\
            .get_or_select_template(template_name_or_list)

    return _render(template, context, ctx.app)

I think it is really weird and unwanted behaviour that one blueprint searches in another blueprint's template_folder. This still does not fix that, but it at least searches is own folder first, then the app folder and then all the other blueprints their folders.

@davidism

This comment has been minimized.

Copy link
Member

commented Jul 15, 2015

The point of searching the app template folder first is so that an app developer can override templates supplied by a blueprint from an extension. Is this still possible with these changes?

@JelteF

This comment has been minimized.

Copy link

commented Jul 15, 2015

No, this method first searches the blueprint template folder. Overriding of the blueprint templates would require some changes that I'm not sure how to do. The reason for this is that currently all the blueprint template folders are added to the global app search path. This means that loading from the app will try to load from any blueprint as well, which means another blueprint folder can be searched before the blueprint of the route that is actually used.

The above code fixes this by first searching the blueprint template folder and only after nothing is found it will search all other options. To fix this something would need to change in the app loader, which requires more work. Since I do not need overriding of templates I did not implement this. Feel free to change the code though.

@mattupstate

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2015

Preferring the blueprint template folder over the application's template folder would break the existing contract between app's and blueprints. This contract is used to much success across the extension ecosystem. For example, the Flask-Security extension mounts a blueprint to an application and comes packaged with a set of minimal default templates. The app developer can then override the stock templates by placing their own files in the app's template folder under the same path as the extension's blueprint.

@JelteF

This comment has been minimized.

Copy link

commented Aug 10, 2015

I know my fix is not sufficient for inclussion, but for my use case this was sufficient. That any blueprint searches in any other blueprints template folder is totally crazy. It makes no sense and I can see no reason for it, especially since the search order seems to be "random".

@dagostinelli

This comment has been minimized.

Copy link

commented Sep 1, 2015

I second the sentiment. It's weird how they are searching one another's folders. The order makes no sense. I can't control it. I keep going in circles.

@alanhamlett

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2015

Are there any extensions depending on this feature? It seems this should be a config variable for the extension, instead of implicitly overriding templates.

@jjwon0

This comment has been minimized.

Copy link

commented Oct 24, 2015

Still encountering this issue but across two different blueprints, one of my templates is being rendered instead of the other one when they have the same name.

@mitsuhiko

This comment has been minimized.

Copy link
Member

commented May 22, 2016

I'm super surprised this is an issue. Is the expectation that template loading is blueprint relative?

@mitsuhiko

This comment has been minimized.

Copy link
Member

commented May 22, 2016

I actually think that a lot would be won if Flask would warn if someone renders a template without a path separator.

@ThiefMaster

This comment has been minimized.

Copy link
Member

commented May 22, 2016

At least for small applications it makes lots of sense to just render index.html

Anyway, having e.g. 'foo/*.html' for templates from the foo blueprint is pretty useful, but it adds a lot of redundancy if you end up with a folder structure like this: yourapp/modules/foo/templates/foo/bar.html (i.e. you already have a folder for the blueprint and end up repeating it inside the templates folder)

@JelteF

This comment has been minimized.

Copy link

commented May 22, 2016

@mitsuhiko
You mean when it doesn't start with a path seperator?
Because I don't think I've ever seen it called with a starting slash.
http://flask.pocoo.org/docs/dev/quickstart/#rendering-templates

@ThiefMaster

This comment has been minimized.

Copy link
Member

commented May 22, 2016

I guess he meant template paths specifying only a file, nothing else

@JelteF

This comment has been minimized.

Copy link

commented May 22, 2016

Also I think it would at least be very nice if it was configurable per blueprint if it template loading is relative. If you try to separate different parts of big applications using blueprints it would be very useful.
However, if you use them for extensions where templates could be overridden by a user you don't want it.

@ThiefMaster

This comment has been minimized.

Copy link
Member

commented May 22, 2016

The main problem is that you cannot make it fully separate without introducing some way to reference a template from the main app or another blueprint, e.g. in a {% extends %} or {% import %}

@alanhamlett

This comment has been minimized.

Copy link
Contributor

commented May 22, 2016

What's the point of having the template_folder argument for Blueprint if it doesn't do anything?

@mitsuhiko

This comment has been minimized.

Copy link
Member

commented May 22, 2016

@alanhamlett what do you mean it does not do anything? It adds the folder to the load path.

@mitsuhiko

This comment has been minimized.

Copy link
Member

commented May 22, 2016

@JelteF i mean that if we render from a blueprint we could just warn if someone does render_template('foo.html') instead of render_template('blueprint_name/foo.html'). (eg: no '/' in the path).

@ThiefMaster

This comment has been minimized.

Copy link
Member

commented May 22, 2016

How would you avoid the warning if you don't want any url rules in your main app (e.g. because you have an api and frontend blueprint)? Having templates/frontend/index.html and using render_template('frontend/index.html')? Feels like something where at config option would be appropriate to disable the warning for applications where there's only one blueprint that contains templates and adding a subdirectory would just add extra noise.

@JelteF

This comment has been minimized.

Copy link

commented May 22, 2016

@ThiefMaster
I think the search order I proposed under my code sample would work there as well.

  1. Search template directory of this blueprint
  2. Search main app template directory
  3. Search all other template directories

Furthermore, a big issue I have with how it works at the moment is that the load order is non deterministic. If the template name is not in the main app template directory it will choose the first one it finds in a blueprint directory based on the key order in a dictionary. This changes on every startup. So debugging this issue is also very hard.

@mitsuhiko

This comment has been minimized.

Copy link
Member

commented May 22, 2016

@JelteF this will never happen for a few reasons:

  • it will cause templates to go by different names which causes caching issues internally among many other problems (extends breaking hilariously)
  • we did something like this with modules before and the general feedback was that it's a terrible idea and very confusing
  • it would break many patterns that people currently have

That said we could have a conversation about render_template('./foo.html') being relative to the current blueprint's name or something. Eg: it expands to render_template('blueprint_name/foo.html').

@JelteF

This comment has been minimized.

Copy link

commented May 22, 2016

@mitsuhiko
What I think @alanhamlett means is that template_folder sounds like it should at least try to search earlier in that folder than in other template_folders

Maybe a different search order would make more sense then for backwards compatibility:

  1. Search main app template directory.
  2. Search directory of this blueprint
  3. Search directories of other blueprints (in a deterministic order).

The ./foo.html syntax would be fine for me as well.

@mitsuhiko

This comment has been minimized.

Copy link
Member

commented May 22, 2016

It's also used for successive calls to render_template.

@JelteF

This comment has been minimized.

Copy link

commented May 22, 2016

Ok, looking at the code in: #1361 (comment)
It seems I only use the cache when it's not in the blueprint directory. That explains it. In that case that "solution" is indeed even less nice than I thought.

In that case I think the ./ solution together with a warning when using no slashes would be very nice. Replacing the dot with the template name would be fine as a default and maybe a keyword argument should be added to allow it to change it to something different.

@mitsuhiko

This comment has been minimized.

Copy link
Member

commented May 22, 2016

@JelteF yeah. Your change is entirely bypassing the load cache which means that you are recompiling the template on every request which is ridiculously slow.

@JelteF

This comment has been minimized.

Copy link

commented May 22, 2016

I'll try and make a pull request for that when I have time.

@mitsuhiko

This comment has been minimized.

Copy link
Member

commented May 22, 2016

@JelteF no need. Such a change will not be merged.

@mitsuhiko

This comment has been minimized.

Copy link
Member

commented May 22, 2016

I will close this as wontfix and someone can open up a new issue on the improved diagnostics if they are not good enough.

@mitsuhiko mitsuhiko closed this May 22, 2016

@JelteF

This comment has been minimized.

Copy link

commented May 22, 2016

Wait, why would the ./ solution not be merged?

@alanhamlett

This comment has been minimized.

Copy link
Contributor

commented May 22, 2016

@JelteF thanks, that's exactly what I meant just too tired to have said it correctly. Telling a blueprint to use a template folder should load from that folder first. Everyone assumes it's like that and you don't notice for a while because it sometimes works until you get an error and find the load order is basically random.

@davidism

This comment has been minimized.

Copy link
Member

commented May 22, 2016

I think we're conflating two related behaviors.

  1. The app takes precedence over the blueprint when loading a template. This is good, it allows the app to customize the templates that extensions provide.
  2. If the app does not override the template, the templates are still merged together, so it's undefined which template will get loaded if two blueprints use the same directory structure.

The first is not going to change. The second is what this issue was actually opened about. I'm not sure what the fix is here besides ensuring that blueprints don't use the same directories in their template folders.

@davidism

This comment has been minimized.

Copy link
Member

commented May 22, 2016

Given that blueprints are iterated over in the order they were registered, I think the correct resolution here is to more thoroughly document the lookup order somewhere in the docs. We've already added the EXPLAIN_TEMPLATE_LOADING config, which can also help show what is going on.

@alanhamlett

This comment has been minimized.

Copy link
Contributor

commented May 22, 2016

@davidism we just want to prevent repeating ourselves by specifying the template_folder every time our blueprints call render_template. How about an optional argument to Blueprint.__init__ that says to load templates from the blueprint first?

@davidism

This comment has been minimized.

Copy link
Member

commented May 22, 2016

class MyBlueprint(Blueprint):
    def render_template(self, name, **kwargs):
        return render_template('/'.join((self.name, name)), **kwargs)

bp = MyBlueprint('blog', __name__)

@bp.route('/')
def index():
    return bp.render_template('index.html')

The actual template path is still blog/index.html, so it can still be overridden. But you don't have to type blog/. Of course, this just causes confusion because extends and include still require the whole path, and the templates still need to live in the blog subpath.

@alanhamlett

This comment has been minimized.

Copy link
Contributor

commented May 26, 2016

Can we have this added as a Snippet?

Overwriting the include template block for this blueprint would allow relative paths there too.

@JohnRobson

This comment has been minimized.

Copy link

commented Aug 3, 2016

Blueprint template_folder only works if I put the absolute path. I would like to use: template_folder='templates/my_sub_folder' and: render_template('file.html') not render_template('my_sub_folder/file.html'). Maybe there is a bug in template_folder.

@davidism

This comment has been minimized.

Copy link
Member

commented Aug 3, 2016

@JohnRobson read the thread before posting in it please. This is working as intended and will not be changed. There is a snippet that simplifies your request two posts up.

@JohnRobson

This comment has been minimized.

Copy link

commented Aug 3, 2016

@davidism, you created that workaround because "template_folder" doesn't work, right?

@davidism

This comment has been minimized.

Copy link
Member

commented Aug 3, 2016

It does work, exactly as expected. The template folder is relative to the blueprint's location, and blueprints have lower priority than the app and previously registered blueprints. This is not a bug.

@JohnRobson

This comment has been minimized.

Copy link

commented Aug 3, 2016

Ok, thank you for your explanation. I expect to use it to setup the templates folder for my blueprint file.

So, if 'template_folder' works properly, how can I use it? eg: bp = Blueprint('bp', name, template_folder='templates/bpsubfolder')

@davidism

This comment has been minimized.

Copy link
Member

commented Aug 3, 2016

Your expectation is not how template lookups work. You create a subdirectory in order to distinguish the blueprint templates. You pass a template folder to a blueprint in order to append it to the search path, not to just use those templates. So don't pass your subdirectory as the template folder, pass it during render template or use the subclass I wrote above.

my_project/
    my_app/
        __init__.py
        templates/
            index.html
        users/
            __init__.py
            templates/
                users/
                    index.html
bp = Blueprint('users', __name__, template_folder='templates')

@bp.route('/')
def index():
    return render_template('users/index.html')
@JohnRobson

This comment has been minimized.

Copy link

commented Aug 4, 2016

I'm already passing the sub-folder during render_template, thank you.

@Querela

This comment has been minimized.

Copy link

commented Aug 6, 2016

Attached is a small patch for a better seach path of render_template() (modifies def iter_loaders(...)). Like @dagostinelli suggested, it attempts to load the template first from the current blueprint, then the app and then the other blueprints.
It doesn't really solve the problem of refering to a specific template file with duplicate name from a blueprint to another blueprint ... Here I would suggest another method (render_template) that has an additional argument for the blueprint name / app. It can almost be like in url_for() in that it refers to an endpoint.

templating.py.patch.txt

@davidism

This comment has been minimized.

Copy link
Member

commented Aug 7, 2016

This suffers the same problems as the other "fixes": it breaks the ability to override templates provided by extensions.

@alanhamlett

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2016

@davidism with @Querela's patch we just need a new attribute on Blueprint called templates_prefer_local that will only load from the local Blueprint first when True. The default could be False so all existing implementations would not change.

@Querela

This comment has been minimized.

Copy link

commented Aug 7, 2016

@alanhamlett that would probably be better.
@davidism for routes from the blueprint I would normally expect to have a higher priority for its own templates and then the app and others. I'm not too sure on how it interfers with extensions.

Additionally I tried to extend the render_template method. It now has an additional arguement for a name/list of names of Blueprints in which the template should be searched only. This patch is on top of my previous one. Maybe in this way it would be better. Here Blueprints could be addressed something like in url_for but not as endpoints. Well, I mean:

# assuming that mod_a und mod_b are blueprints with their own template
# directories (both are named index.html)

@mod_a.route('/mod_a')
def func_a():
    # to only search for index.html in blueprint mod_b
    return render_template('index.html', 'mod_b', title='Hello World', ...)

@mod_a.route('/mod_a2')
def func_a2():
    # normal behaviour
    return render_template('index.html', title='Hello World', ...)

templating.patch.new_render_template.txt

@NoiSek

This comment has been minimized.

Copy link

commented Mar 17, 2017

For anyone who stumbles upon this as I did many months ago, and are as perfectly alright violating the blueprint contract as they are understanding of it's implications, I ended up writing a tiny Flask plugin to solve this issue.

Hopefully this will come in useful to you, future Googler.

@pallets pallets deleted a comment from findsarfaraz Nov 3, 2017

@pallets pallets locked and limited conversation to collaborators Nov 3, 2017

@davidism

This comment has been minimized.

Copy link
Member

commented Nov 3, 2017

Our stance is that there is no issue here. Blueprint template folders do not take precedence over the app template folder. This is to allow an app to override extension templates. If you are in a situation where you want to go the other direction, please reconsider exactly why you want to do that.

Locking this as there is nothing more to say on the issue, it just keeps going in circles.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.