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
Closed

Comments

@Kevin922
Copy link

Kevin922 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
Copy link
Contributor

What is the output, and what did you expect?

@alanhamlett
Copy link
Contributor

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
Copy link

ael-code 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
Copy link

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
Copy link

JelteF 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
Copy link
Member

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
Copy link

JelteF 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
Copy link
Contributor

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
Copy link

JelteF 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
Copy link

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
Copy link
Contributor

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
Copy link

jjwon0 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
Copy link
Contributor

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

@mitsuhiko
Copy link
Contributor

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

@ThiefMaster
Copy link
Member

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
Copy link

JelteF 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
Copy link
Member

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

@JelteF
Copy link

JelteF 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
Copy link
Member

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
Copy link
Contributor

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

@mitsuhiko
Copy link
Contributor

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

@mitsuhiko
Copy link
Contributor

@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
Copy link
Member

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
Copy link

JelteF 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
Copy link
Contributor

@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
Copy link

JelteF 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
Copy link
Contributor

It's also used for successive calls to render_template.

@JelteF
Copy link

JelteF 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
Copy link
Contributor

mitsuhiko 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
Copy link

JelteF commented May 22, 2016

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

@mitsuhiko
Copy link
Contributor

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

@mitsuhiko
Copy link
Contributor

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

@JelteF
Copy link

JelteF commented May 22, 2016

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

@alanhamlett
Copy link
Contributor

alanhamlett 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
Copy link
Member

davidism 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
Copy link
Member

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
Copy link
Contributor

alanhamlett 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
Copy link
Member

davidism 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
Copy link
Contributor

Can we have this added as a Snippet?

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

@JohnRobson
Copy link

JohnRobson 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
Copy link
Member

davidism 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
Copy link

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

@davidism
Copy link
Member

davidism 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
Copy link

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
Copy link
Member

davidism 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
Copy link

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

@Querela
Copy link

Querela 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
Copy link
Member

davidism commented Aug 7, 2016

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

@alanhamlett
Copy link
Contributor

@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
Copy link

Querela 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
Copy link

NoiSek 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
Copy link
Member

davidism 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.

This is thoroughly documented: https://flask.palletsprojects.com/en/1.1.x/blueprints/#templates

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

No branches or pull requests