Manager Run use_debugger overrides flask app debug setting #91

Closed
osiloke opened this Issue Feb 17, 2014 · 13 comments

Comments

Projects
None yet
6 participants

osiloke commented Feb 17, 2014

I noticed that the debug setting is overriden by manager run command at some point (i think at the initial request).

It would be nice to warn about this and maybe ensure that you set use_debugger based on your flask config file.

Owner

smurfix commented Apr 28, 2014

It's probably overridden by the way you create your app; "debug" or "DEBUG" does not occur in flask-script's source code.
Can you create a gist with a test case?

Contributor

miguelgrinberg commented May 6, 2014

The problem is in the logic that builds the options for the Server() command. For example:

    if self.use_debugger:
        options += (Option('-d', '--debug',
                           action='store_true',
                           dest='use_debugger',
                           help="(no-op for compatibility)"),)
        options += (Option('-D', '--no-debug',
                           action='store_false',
                           dest='use_debugger',
                           default=self.use_debugger),)

Note how in this case you set a default only for the -D case. For some reason argparse appears to ignore this default, probably due to having another option using the same storage that does not have a default. The same thing happens on the reloader.

I can make this work if I include the default in both variants.

This is a pretty serious problem, since there is no way to enable debugging/reloader. I hope you can address it in a timely manner.

Three months and not fixed (its happening with the latest pip version). Theres some nice response times!

By the way 0.6.7 works fine.

Collaborator

techniq commented May 6, 2014

Thread/old PR about setting debug / use_debugger. It seems like it would be best to store the status of debug in it's own variable but fall back to app.debug if not set?

Contributor

miguelgrinberg commented May 6, 2014

@techniq My old fix #75 is still there, this broke with the recent change that defines both the enable and disable options for debugger and reloader. With the code I pasted above argparse returns False for the use_debugger setting. That then is set in app.debug and Werkzeug's use_debugger. It appears having two options going to the same variable and not having defaults in both is what causes the problem.

Owner

smurfix commented May 6, 2014

Hi,

Sean Lynch:

Thread/old PR about setting debug / use_debugger. It seems like it would be best to store the status of debug in it's own variable but fall back to app.debug if not set?

Makes sense.

Pull request appreciated, I need to do actual paying work this week. :-/

-- Matthias Urlichs

Owner

smurfix commented May 6, 2014

OK, changed and tested.
This means that I can no longer show what the default is, because the Flask app is not available to flask_script unter after options are set.

@smurfix smurfix closed this May 6, 2014

Contributor

miguelgrinberg commented May 6, 2014

All previous releases (at least in the 0.x line) had debugging enabled by default. I'm sorry but I think it is a terrible idea to change that.

Flask-Script's runserver command is not useful in production, you never use Werkzeug's server there. So you either not use Flask-Script at all, or else implement a different command that runs gunicorn or something else. This new default with debug disabled does not seem very useful to me.

You also broke the examples I have in my book with this change, which I wrote using the 0.6 line, and now it is too late for me to make changes to it.

Would you consider reverting to debug enabled?

@miguelgrinberg miguelgrinberg referenced this issue in miguelgrinberg/flasky May 6, 2014

Closed

Possible enhancement? #9

Contributor

miguelgrinberg commented May 6, 2014

And just to clarify, when I say "debug enabled" I mean both use_debugger and use_reloader having a default value of True.

Owner

smurfix commented May 7, 2014

Hi,

Miguel Grinberg:

Would you consider reverting to debug enabled?

Clarification: I do not default debugging to False. I default to whatever the
Flask configuration is set to, which is False by default.

On the other hand, I do see your problem.

I plan to do this:

I'll update that so that flask-script's default will be True, which will be
used if Flask's debug value is None. Thus, DEBUG=None instead of =False in
the settings file will do the right thing.

I will also submit a bug report / pull request to Flask, asking them to default
app.config['DEBUG'] to None instead. That will let me default to True if
there's no value set in the configuration, which I currently cannot do.

I really do not want to silently override explicit configuration with
something that's unsafe, esp. since one can run a low-volume web server
perfectly well behind "runserver".

Would that work for everybody?

-- Matthias Urlichs

Contributor

miguelgrinberg commented May 7, 2014

I think this is a good solution going forward. It does not change the fact that with the current version of Flask there is a change of behavior between the 0.x and later releases.

Owner

smurfix commented May 7, 2014

Hi,

Miguel Grinberg:

I think this is a good solution going forward. It does not change the fact that with the current version of Flask there is a change of behavior between the 0.x and later releases.

Between 1.x and 2.x, to be exact.

Right, but given the fact that it's incompatible anyway (-h vs. -?,
positional options vs. not) IMHO we can live with that.

-- Matthias Urlichs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment