Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

Initialize Flask debug variable to the correct setting #75

Merged
merged 1 commit into from
Sep 24, 2013

Conversation

miguelgrinberg
Copy link
Contributor

The debug state in Flask is never changed, only Werkzeug gets set with the correct debug state. Because if that the debugger is not activated.

@techniq
Copy link
Collaborator

techniq commented Sep 20, 2013

Hmm, we've gone back and forth on this in the past. It had been set to that but that meant it ignored your app.config value. We then set it to read from your app.config first, then fallback to the use_debugger value...
637aa21#L0L387

but then Flask started defaulting this value to False, which meant we'd never use the use_debugger value.
#54

Not sure how best to proceed after the change in Flask to always have app.config.debug set to False

@miguelgrinberg
Copy link
Contributor Author

The problem is that there is a discrepancy in the defaults for debugging. Flask defaults to False, while Flask-Script defaults to True. So who wins?

My opinion is that if you are using Flask-Script then it should have precedence over Flask, so I think it would be all right to always override Flask's setting like I did in my patch.

The other solution would be for Flask-Script to set its default for debugging to False, like Flask. But this would mean a change in behavior, now you would need to add the "-d" option to get the debugger, which at least for me would be inconvenient, as most of the time I want to run with the debugger.

@techniq
Copy link
Collaborator

techniq commented Sep 23, 2013

Flask-Script doesn't default debugging to false, it just enables the Werkzeug debugger by default. I agree if you're using Flask-Script's Server you are developing and therefore debugging most of the time, but you still need the ability to on occasion run the server without the debug flag (testing using production minified/compiled assets, enabling using production database vs local, etc).

The think the proper solution is for the end user to set app.debug = True, which is easy. I also recommend with Flask-Script to setup multiple configurations using a -c option, as mentioned here.

@miguelgrinberg
Copy link
Contributor Author

I'm not sure I agree. I think what you are basically saying is that Flask-Script should not mess with the debug configuration, the setting should come from configuration. I guess that is a valid solution, but that doesn't seem to be what Flask-Script documents.

It seems before Flask 0.10 there were three cases for Flask Script to handle:

  1. User set app.debug = True
    In this case Flask-Script defaults to debug = True, and -d turns debugging off, both for Flask and Werkzeug
  2. User set app.debug = False
    Flask-Script defaults to debug = False, and -d turns debugging on for Flask and Werkzeug
  3. User did not set app.debug
    Flask-Script defaults to debug = True for Flask and Werkzeug, -d switch debug off.

Is this a correct interpretation of how things worked before Flask defaulted its debug setting to False?

The problem that we have now is that item 3 in my list cannot be determined anymore, that case looks exactly like 2 now.

My understanding of the current issue is that item 1 in my list works the same as before, but for items 2 and 3 Flask-Script will end up leaving Flask's debug setting to False and Werkzeug's debug setting to True. In this situation the reloader works, but the debugger seems to be suppressed by Flask's app.debug == False.

If your choice is to let the user deal with this, then shouldn't you also leave use_debugger alone? And the -d option also makes no sense to have. My problem with this solution is that in this mode it is confusing because it is unusual to have the reloader enabled and the debugger disabled. It may be more clear to also drop the reloader setting, so that everything is controlled from Flask's debug setting.

@techniq
Copy link
Collaborator

techniq commented Sep 23, 2013

Those 3 cases were true until pull request #54 6 months ago. Note: "-d" never affect's Flask or Werkzeug's "debug" setting now, after that pull request change, it only affect's the Werkzeug interactive debugger, so the last part of item 1 is not correct (about Flask's debug being turned off).

What -d as well as -r is used for is to invert Werkzeug server's interactive debugger and reloader. For example, if you use a standard Flask-Script manager...

manager = Manager(create_app)

python runserver would have app.debug = False, interactive debugger = True (enabled) and reloader = True (enabled)
python runserver -d would have app.debug = False, interactive debugger = False (disabled) and reloader = True (enabled)
python runserver -r would have app.debug = False, interactive debugger = True (enabled) and reloader = False (disabled)

but if the user overrides the default Server command with their own instance which disables the interactive debugger by default

manager = Manager(create_app)
manager.add_command("runserver", Server(use_debugger=False)

python runserver would have app.debug = False, interactive debugger = False (disabled) and reloader = True (enabled)
python runserver -d would have app.debug = False, interactive debugger = True (enabled) and reloader = True (enabled)
python runserver -r would have app.debug = False, interactive debugger = False (disabled) and reloader = False (disabled)

I think some confusion of -d stems from it being labeled --debug instead of say --interactive-debugger. One thing a user could do would be to add an option to set the app.debug within the app factory (just like the config example in the docs. I know -d would make the most sense for the flag, but since it would overlap with the Server -d, something else would need to be used. If the user requires even more control, they could subclass the Server class and control the options exposed or how handle/run is executed.

@miguelgrinberg
Copy link
Contributor Author

But is there any purpose to enabling Werkzeug's interactive debugger while Flask's debug setting is False? Flask will not let the debugger run in that case unless you set app.debug = True, but if you do that then Flask enables the interactive debugger anyway.

It seems to me the only valid case where the developer needs to mess with settings is when you want to run Flask in debug mode without the interactive debugger. In that case you set app.debug = True and then call app.run(use_debugger = False).

@techniq
Copy link
Collaborator

techniq commented Sep 24, 2013

Ok, I think you've finally won me over.

I'll be honest, my free time has been pretty limited the past 6+ months and doesn't look to be getting any better. I'm considering looking for a new maintainer for this extension so it can get the attention it deserves. I was hoping a Flask Github organization would be created (pallets/flask#729) and move it under there, but it doesn't appear to be getting any traction. Would you consider helping maintain this extension?

Since we've moved to using Argparse's subparsers there are some third party script compatibility issues that really need resolved (I've hit a few roadblocks while investigating myself) and could use a fresh view on it.

techniq added a commit that referenced this pull request Sep 24, 2013
Initialize Flask debug variable to the correct setting
@techniq techniq merged commit a98b7c1 into smurfix:master Sep 24, 2013
@miguelgrinberg
Copy link
Contributor Author

Thanks!

I'll look at the Flask-Assets issue and let you know if I can come up with something. I'm happy to help out, I don't have a lot of time myself, but I do have some.

@techniq
Copy link
Collaborator

techniq commented Sep 25, 2013

Thanks

@techniq
Copy link
Collaborator

techniq commented Sep 25, 2013

I thought on this some more and think maybe we should change -d and thus --debug and --no-debug to control a new self.debug on Server instead of self.use_debugger. self.debug would be a nullable boolean to know when it's been set. We would use this if set, but fall back to app.debug.

app.run(host=host,
                 debug=self.debug or app.debug,
                 ...

The user can still disable use_debugger, but must to globally when registing a Server instance with Flask-Script. Does this seem to allow for all use cases you can think of?

@miguelgrinberg
Copy link
Contributor Author

I like it. That brings back the ability for the user to set app.debug, while leaving the defaults in the expected state.

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.

2 participants