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

Enable template auto-reloading in app.run() #1910

Closed

Conversation

davebarkerxyz
Copy link
Contributor

When Flask app debugging is enabled (app.debug==True), and Jinja2
template auto-reloading is not explicitly disbaled, template
auto-reloading should be enabled.

If the app is instantiated, the jinja_env object is accessed (thereby
initialising the Jinja2 environment) and the server is then started with
app.run(debug=True), template auto-reloading is not enabled.

This is because reading the jinja_env object causes the environment
initialisation function to set auto_reload to app.debug (which isn't yet
True). Calling app.run(debug=True) should correct this in order to
remain consistent with Flask code reloading (which is enabled within
app.run() if debug == True).

When Flask app debugging is enabled (app.debug==True), and Jinja2
template auto-reloading is not explicitly disbaled, template
auto-reloading should be enabled.

If the app is instantiated, the jinja_env object is accessed (thereby
initialising the Jinja2 environment) and the server is then started with
app.run(debug=True), template auto-reloading is *not* enabled.

This is because reading the jinja_env object causes the environment
initialisation function to set auto_reload to app.debug (which isn't yet
True). Calling app.run(debug=True) should correct this in order to
remain consistent with Flask code reloading (which is enabled within
app.run() if debug == True).
@jeffwidman
Copy link
Contributor

jeffwidman commented Jun 14, 2016

This looks fine to me, although I'd prefer another maintainer sign off as well before it's merged. Can you add a test though?

@davebarkerxyz
Copy link
Contributor Author

No problem, I'll add a test to this later this evening. Thanks.

@@ -839,6 +839,8 @@ def run(self, host=None, port=None, debug=None, **options):
options.setdefault('use_reloader', self.debug)
options.setdefault('use_debugger', self.debug)
options.setdefault('passthrough_errors', True)
if debug:

This comment was marked as off-topic.

@davebarkerxyz
Copy link
Contributor Author

I don't think so - that's the root of the problem.

The template reloader gets disabled at Flask app instantiation if it's not explicitly enabled and debug mode isn't turned on at that point.

However, in the case if the code reloader, reloading is enabled in app.run() it is not explicitly disabled and debug is enabled.

I think the patch harmonises the behaviour between template and code reloading (if no explicit reloading configuration and debug is enabled via app.run(debug=True), reloading is set to match debug).

@untitaker
Copy link
Contributor

I think you're confusing the template reloader of Jinja (enabled by default) with the server reloader from Werkzeug (disabled by default).

@untitaker
Copy link
Contributor

untitaker commented Jun 14, 2016

And TEMPLATES_AUTO_RELOAD controls how Jinja's auto_reload is set. The default None value means that it already depends on debug mode (see create_jinja_environment,

if self.config['TEMPLATES_AUTO_RELOAD'] is not None:
)

@untitaker
Copy link
Contributor

Sorry, I see that you've figured all of this already out in #1907 (comment). In that case I would say that we need to deduplicate that logic anyway, i.e. remove the logic for TEMPLATE_AUTO_RELOAD out of create_jinja_environment into run.

@davebarkerxyz
Copy link
Contributor Author

@untitaker That's the problem. If you don't explicitly enable debug via configuration at Flask app instantiation then access the jinja_env, template auto reloading is disabled. If you then subsequently enable debug by starting the server with app.run(debug=True), Werkzeug debugging and reloading is enabled (regardless of the fact it wasn't otherwise explicitly enabled via config) but template auto reloading isn't enabled.

I think the intention is that calling app.run(debug=True) should enable debugging and auto reloading. It's confusing that this enables code reloading (when no explicit option is set) but doesn't enable template reloading (when no explicit option is set).

Sorry if I'm not explaining this very well..

@untitaker
Copy link
Contributor

Yes I oversaw your post in #1907, it all makes sense.

But with this fix there's another inconsistency: If I explicitly set app.jinja_env.auto_reload = False and then run app.run(debug=True), the value gets ignored?

@justanr
Copy link

justanr commented Jun 14, 2016

@davb5 What i meant specifically was using options['use_reloader'] instead of just the debug flag itself. There's instanceswhen I've enabled debug but disabled reloading intentionally.

@untitaker
Copy link
Contributor

You are confusing Werkzeug server reloading with Jinja env reloading.

On 14 June 2016 23:36:46 CEST, Alec Nikolas Reiter notifications@github.com wrote:

@davb5 What i meant specifically was using options['use_reloader']
instead of just the debug flag itself. There's instanceswhen I've
enabled debug but disabled reloading intentionally.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#1910 (comment)

Sent from my Android device with K-9 Mail. Please excuse my brevity.

@davebarkerxyz
Copy link
Contributor Author

Ah, I understand now @untitaker.

Sorry, I missed that comment. You're right in that we're clobbering any configuration set value from create_jinja_environment, and this probably isn't the best course of action.

The difficulty here is that between instantiating our Flask app and calling app.run() there are a few places that template auto reloading could be enabled or disabled.

My proposed patch works for most scenarios but fails for the one you've highlighted - if we explicitly disable template auto reloading (using any of the above mechanisms) then call app.run(debug=True), we clobber our explicit disablement of template reloading. And that's probably not ideal.

@justanr's comment that we might want to use options['use_reloader'] rather than debug makes some sense, but could this cause further confusion by tying Jinja template autoreloading to Werkzeug reloading in one place (app.run) but no-where else?

I think you're right in that moving the logic for TEMPLATE_AUTO_RELOAD into app.run might be the solution. We'd have to make sure that nothing else fails and no behaviours change as a result of template auto-reloading not being configured one way or another before app.run (i.e. make sure nothing expects some value to be there before app.run).

What does Jinja do if auto_reload is not set? Does it have a sensible default behaviour?

If so, we could probably just move that block from create_jinja_environment into run pretty much wholesale. Would this be a better solution?

@davebarkerxyz
Copy link
Contributor Author

Actually, that might not work without some more changes.

I don't believe run() is called at all in WSGI apps, so that couldn't be the only place that we set jinja_env.auto_reload.

@davebarkerxyz
Copy link
Contributor Author

davebarkerxyz commented Jun 14, 2016

Actually, #1910 (comment) might not matter as I don't think the Werkzeug auto reloader works in a WSGI environment (could someone please confirm?).

If we don't auto reload code in a WSGI environment, keeping the template auto reloader configuration in app.run() would make sense and keep things consistent.

However, it would appear that Jinja defaults to auto_reload=True. test_templates_auto_reload expects unconfigured auto reload to default to False.

Moving the config code from create_jinja_environment would mean that between instantiating the Flask app and calling app.run(), the config in app.config would be ignored. This wouldn't work. And running it was a WSGI app would leave the reloader on by default (not the currently established behaviour).

I think we'll need a different approach (clobbering the value in app.run() as per the original patch is starting to look more pragmatic now).

@davebarkerxyz
Copy link
Contributor Author

I've added a test for the original patch (wether or not we use that solution, it's a starting point for the unit test).

@illume
Copy link

illume commented Jun 28, 2016

A quick workaround is to enable it manually:

class YourFlask(Flask):
    def create_jinja_environment(self):
        self.config['TEMPLATES_AUTO_RELOAD'] = True
        return Flask.create_jinja_environment(self)
app = YourFlask()

I guess this issue is blocking a lot of people who use flask to develop templates.

@neatorobito
Copy link

neatorobito commented Jul 7, 2016

Thank goodness I found this, I've been tearing my hair out for a few hours trying to figure out why my templates weren't auto updating. I guess while running apt-get upgrade I didn't notice that I had updated to 0.11.1 which has this issue. What release are you guys targeting for this fix?

@deelaka
Copy link

deelaka commented Mar 27, 2017

Adding app.jinja_env.auto_reload = True along with app.config['TEMPLATES_AUTO_RELOAD'] = True seems to fix.

@kennethreitz
Copy link
Contributor

needs a rebase!

@kennethreitz
Copy link
Contributor

closing due to inactivity. please consider re-opening this pull request :)

@davidism davidism reopened this Jun 14, 2017
@davidism davidism closed this Jun 14, 2017
@davidism
Copy link
Member

Sorry for close/reopen spam, was trying to see if "allow edits from maintainers" was enabled for the PR. I'm going to continue this in another PR.

@GhastlyParadox
Copy link

GhastlyParadox commented Feb 2, 2018

So, I'm at a loss. I've tried every proposed solution I've been able to find, without luck. My locally run Flask app completely ignores any changes made to my html template files, no matter what I do. I've even redefined the location of the template folder via template_folder=template_dir - doesn't change a thing. Cleared browser cache, changed browsers. Nothing.

I've tried every permutation of setting debug and auto_reload I can think of. I currently have it running with a solution proposed in this thread (code below) - no errors, but still it completely ignores any changes made to template html files.

Am I going mad? What could I be missing?

EDIT: nevermind, realized I wasn't even calling the file I'd been editing. New to flask :)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants