Dev Mode Config Settings

I wanted to kick off a quick discussion about how we deal with dev mode settings. We've got a few PRs open that introduce new dev mode features and I want to make sure that we have the right abstraction in place.

For context, please read the thread from #369 (comment) and the "debug, dev, production" section in this issue: #312 and an issue by one of the flask developers about running debug mode: #16

Requirements:

We minimize the risk that users run these features that have security implications accidentally in production mode
- See Do not encourage Flask.run in production #16 for a discussion on this. This is why we made debug=False the default behaviour while including
```
if __name__ == '__main__':
     app.run_server(debug=True)
```
in all of our examples.
We allow users to turn on wsgi-compatible features if they are using gunicorn in their dev environment
- This means that we can't just stick these features in run_server as gunicorn won't use run_server
- In Add unminified components bundle support. #369, there was a suggestion to make a separate method that run_server calls (enable_dev_mode). That way, users using gunicorn in their dev env can just call this function
There is a simple flag to turn on and off all of the dev mode features. Additionally a way to turn individual features on and off (e.g. turn off hotloading but turn on sourcemaps)
For users using run_server as a dev environment, they shouldn't have to change their code before deploying it to the dev environment.
- This means that these settings can not live in the constructor like the rest of the dash arguments.
- This limits us to either having them as part of run_server or as an env variable. env variable is too hard for our users.

With these constraints, the only solution that I see is:

The debug=True command in app.run_server(debug=True) ends up doing two things:
- Passes debug=True into flask.run(debug=True) for auto-reloading
- Calls app.set_debug_mode(debug=True)
Each dev mode feature that we write gets its own additional setting that's prefixed with debug_:
- debug_flask=True - This is the setting that allows users to turn on/off flask's debug environment (i.e. app.server.run(debug=debug_flask)). This would include auto-reloading and the werkzeug debugger.
- debug_javascript_bundles=True - This turns on/off the source maps/unminified bundles Add unminified components bundle support. #369
- debug_hot_reload=True - This turns on/off hot reloading. If debug_hot_reload=True but debug_flask=False, then an error is raised (because we need flask's autodebugger - right?)
- debug_display_javascript_errors=True - This turns on/off displaying the JS error messages in the front-end app (bubbling the error messages up from the browser console to the app) [WIP] Dash dev tools dash-renderer#64
- debug_display_python_debugger=True - This turns on/off displaying the python debugger in the browser context and highlighting the error messages ([WIP] Dash dev tools dash-renderer#64). If debug_display_python_debugger=True but debug_flask=False, then an error is raised (because we need flask's autodebugger - right?)
- debug_display_callback_dag=True - This turns on/off the graph of the callback visual
- debug_hide_url_route_logs=True - This turns on the HTTP request logging (and we turn it off by default) - remove werkzeug url print statements by default? #382
dash-renderer may need some of these settings. So, they'll get serialized like the rest of the config settings: as an element in Dash's JSON config element (<script id="_dash-config">{...}</script> or w/e)

Drawbacks:

Because of Do not encourage Flask.run in production #16, technically, these aren't on by default. However, all of our examples use app.run_server(debug=True) and I instinctively write this whenever I create an app, so it's practically the default option.
Previously, debug=True was 1-1 with flask's app.server.run(debug=True). Now, debug=True will set many other settings. This is OK, if a user needs to turn this off but they want something else on, they can do something like: app.run_server(debug=False, debug_javascript_bundles=True) or app.run_server(debug=True, debug_flask=False)

Naming:

I think these names should be read as "Debug mode: " rather than "Debug ". That is, instead of debug_javascript (which reads as "Debug javascript"), we'd have more specific debug_server_dev_javascript (which reads as "Debug mode: Serve dev javascript").
That is, these settings should be tightly integrated with the underlying the feature, allowing users to turn on and off any particular feature.

Does this sound right @plotly/dash ? Sorry for holding things up here, but I want to make sure that we're laying down the right foundation 🚧

Just to be precise, this is what I imagine run_server and set_debug_mode looking like:

def run_server(self, debug=False, **kwargs):

    debug_flask = kwargs.pop('debug_flask', debug)

    self.set_debug_mode(debug=debug, **kwargs)

    flask_run_args = {k: v for k in kwargs if k not in
        ['debug_flask', 'debug_serve_dev_javascript', 'debug_hot_reloading',
         'debug_display_javascript_errors']
    }
    app.server.run(debug=debug_flask, **flask_run_args)

def set_debug_mode(self, debug=False, **kwargs):
    # store the debug args in the instance so that we can use them when we
    # serve the front-end JS config settings
    self._debug_args = copy.copy(kwargs)

    if 'debug_flask' in kwargs:
        raise Exception('''
            `debug_flask` can only be set when calling `app.run_server` as it
            configures flask's dev server directly.
        ''')

    debug_serve_dev_javascript = kwargs.pop('debug_serve_dev_javascript', debug)
    debug_hot_reloading = kwargs.pop('debug_hot_reloading', debug)
    debug_display_javascript_errors = kwargs.pop('debug_display_javascript_errors', debug)
    # and all the other ones we end up having

    # now actually do the things that these settings enable
    # ...

@chriddyp

I don't like relying on kwargs that much, it tends to hide options to the users. Like the only time you gonna get auto-completion for the kwargs is in __init__ subclass. Making them explicit doesn't take much more work here.

@chriddyp
In #369, I prefixed the variables with dev_tools_, here I see debug_, can we get fixed on the naming ? I liked dev_tools since it was different than the debug keyword of flask but I don't mind changing it.

Here's what it would look like with dev_tools and the kwargs expanded:

def run_server(self,
               debug=False,
               dev_tools_debugger=None,
               dev_tools_serve_dev_javascript=None,
               dev_tools_display_javascript_errors=None,
               dev_tools_flask=None, **flask_run_args):

    debug_flask = debug if dev_tools_debugger is None else dev_tools_debugger

    self.set_dev_mode(
        debug=debug,
        dev_tools_debugger=dev_tools_debugger,
        dev_tools_serve_dev_javascript=dev_tools_serve_dev_javascript,
        dev_tools_display_javascript_errors=dev_tools_display_javascript_errors,
        dev_tools_flask=dev_tools_flask
    )

    app.server.run(debug=debug_flask, **flask_run_args)


def set_dev_mode(self,
                   debug=False,
                   dev_tools_debugger=None,
                   dev_tools_serve_dev_javascript=None,
                   dev_tools_display_javascript_errors=None):
    # store the devtools args in the instance so that we can use them when we
    # serve the front-end JS config settings
    self._dev_tools = {
      'dev_tools_debugger': dev_tools_debugger,
      'dev_tools_serve_dev_javascript': dev_tools_serve_dev_javascript,
      ...
    }

    def with_default(some_value):
        if some_value is None:
            return debug
        return some_value

    dev_tools_debugger = with_default(dev_tools_debugger)
    dev_tools_serve_dev_javascript = with_default(dev_tools_serve_dev_javascript)
    dev_tools_display_javascript_errors = with_default(dev_tools_display_javascript_errors)
    # and all the other ones we end up having

    # now actually do the things that these settings enable
    # ...

I like dev_tools too. One thing that I like about the debug_ prefix is that I think it's clearer that debug is the default value for the rest of the dev tool settings. That is, if dev_tools_serve_dev_javascript is None, then it gets set to whatever debug is.

That being said, I do like the sound of dev_tools better than debug.

@ned2

I'd like to make sure that we have consensus among @ned2 and @rmarren1 as well

Some comments on the dev tools flags:

I don't think displaying the javascript errors without dev bundles would be so useful. The only good information you could get from this is which component failed (the one that turns red) but it would be hard to see what actually happened. Can we have dev_tools_display_javascript_errors turn on dev_tools_serve_dev_javascript by default?
I think dev_tools_debugger should read dev_tools_display_python_errors. That way, it matches dev_tools_display_javascript_errors and it is clear they are related. Or perhaps both should be changed to dev_tools_display_backend_errors and dev_tools_display_frontend_errors.

Also, I think debug should over-ride dev_tools_debugger and not the other way around. Just because people should be using gunicorn in production doesn't mean they will, so this could be a safety net for people who just flip debug=False and run python app.py on an EC2 instance thinking they are safe.

Ah, I see this is a mechanism to turn on everything or only turn certain debug options. Guess we just need people to be careful.

Also, what is the best way to go about rolling this out? Should we add the config options in each PR as we merge them, or add them all at once and have the un-implemented ones do nothing?

I don't think displaying the javascript errors without dev bundles would be so useful. The only good information you could get from this is which component failed (the one that turns red) but it would be hard to see what actually happened. Can we have dev_tools_display_javascript_errors turn on dev_tools_serve_dev_javascript by default?

Yes it should be activated.

Also, what is the best way to go about rolling this out? Should we add the config options in each PR as we merge them, or add them all at once and have the un-implemented ones do nothing?

#369 is almost done and I added a enable_dev_tools method, just waiting for the consensus on the variable naming. Let's get that in first then we can rebase and add support in the ongoing PRs.

Also, what is the best way to go about rolling this out? Should we add the config options in each PR as we merge them, or add them all at once and have the un-implemented ones do nothing?

. Let's get that in first then we can rebase and add support in the ongoing PRs.

Yeah, let's implement them one-by-one. I just want to make sure that we agree on:

debug ➡️ overriding other, unspecified dev_tools_* flags
The dev_tools_ prefix for all of these settings
The enable_dev_tools method name and behaviour
The placement of these variables in app.run_server and not in dash.Dash.

Once we have consensus (add 👍 if you are OK with 1-4), we can introduce this pattern in our new PRs, one-by-one.

Nice. I reckon this sounds like a solid foundation for building out dev-mode features on top of. Was definitely worth pausing to think about this.

Excellent, thanks for chiming in everyone. I'll close this down now that we have consensus 👍

treat `dev_tools_ui` like the other `dev_tools_*` parmeters

Fix merge conflicts in PR-201 (plotly#383)

Navigation Menu

Repository navigation

Dev Mode Config Settings #383

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Dev Mode Config Settings #383

Description

Activity

chriddyp commented on Sep 12, 2018

T4rk1n commented on Sep 13, 2018

chriddyp commented on Sep 13, 2018

chriddyp commented on Sep 13, 2018

chriddyp commented on Sep 13, 2018

rmarren1 commented on Sep 14, 2018

rmarren1 commented on Sep 14, 2018

rmarren1 commented on Sep 14, 2018

T4rk1n commented on Sep 14, 2018

chriddyp commented on Sep 14, 2018

ned2 commented on Sep 17, 2018

chriddyp commented on Sep 17, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions