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

Dash's config system #312

Closed
chriddyp opened this issue Jul 31, 2018 · 4 comments
Closed

Dash's config system #312

chriddyp opened this issue Jul 31, 2018 · 4 comments

Comments

@chriddyp
Copy link
Member

chriddyp commented Jul 31, 2018

No description provided.

@chriddyp
Copy link
Member Author

chriddyp commented Aug 2, 2018

This is a discussion about Dash's config system. As we add features that can be turned on and off or configured, like those in our dash dev tools system, we should make sure that we're building them on a sound configuration / settings system.

In principle, I'm interested in Dash having a small, declarative API. That is, when you learn Dash, you only need to know:

  • Config settings
  • app.callback
  • app.layout
  • app.run_server
  • The component's declarative input arguments

The config settings are currently exposed in an ad-hoc way in app.config and through the dash.Dash constructor. I propose a few changes/guidelines:


config in constructor
Expose all config arguments through dash.Dash constructor. One day, years from now, this may mean that we'll have hundreds of items in here


config in os.environ
Provide an alternative way of setting the variables through os.environ. We'll prefix os.environ settings with dash_ so that there aren't conflicts with other services.
For example, the following two settings are equivalent:

dash.Dash(port=8050)
os.environ['dash_port'] = '8050'
dash.Dash()

The arguments in the constructor will take precedence over os.environ

Some other notes:

  • lowercase or UPPERCASE would be accepted
  • environ variables are strings, so we'd convert them back into numbers

immutable config
Right now, config is accessible in app.config which sort of implies that its mutable. but many of the settings aren't actually mutable. We want to do things in our __init__ with the arguments that are passed in or available in os.environ (like set up our routes with requests_pathname_prefix). We don't want to have to maintain multiple pathways for setting and resetting aspects of the object that depend on app.config.

So, we'd deprecate app.config

We'd also deprecate the ad-hoc app.<whatever> settings like app.title


defaults, overrides, specificity
Some settings will override other settings. For example, originally I had
url_base_pathname but found that this wasn't specific enough so I introduced requests_pathname_prefix and routes_pathname_prefix

So, setting url_base_pathname would set the same variable for requests_pathname_prefix and routes_pathname_prefix


debug, dev, production
as we introduce different debugging features as part of https://github.com/orgs/plotly/projects/3, we will want users to turn them on and off.

Perhaps we have a dev variable: True or False

Each individual feature should have its own variable as well. dev=True turns them all on, dev=False turns them all off, and individual features could configured separately:

  • dev = True - if False, then it's "prod" mode
  • dev_hotloader = True
  • dev_webtools = True

One issue with this system is "what's the default value?". It can't be True or our users will inevitably deploy their apps without setting it to False. If it's False, then we'd need to explicitly set it to True everywhere and note that they should set it to False when deploying. And if it takes precedence over os.environ, then it's not something they could overwrite in their deploy environment.

We've gotten around this issue with debug by placing it in app.run_server: a statement that only runs in the if __name__ == '__main__', a condition that isn't called when the app is run by gunicorn:

if __name__ == '__main__':
    app.run_server(debug=True)

So, instead of dev=True, we could use this debug=True in the run_server command. I wanted to avoid this: if set config variables in run_server, then the user has two different places to set config variables: in dash.Dash and in app.run_server. Further, variables in app.run_server would only be applied right before the app is run. This would prevent us from doing dev-mode things before runtime (like validating app.layout). Maybe that's OK?

So, perhaps we have one set of config variables that are specified in dash.Dash and reserve all of the dev-mode stuff in app.run_server?

Another issue with using debug=True as our generic dev setting is that Flask's reloader doesn't work for some of our user's dev environments (mainly Jupyter notebooks). So, if they wanted all of the dev features except the reloader, they could keep debug=True but set debug_reloader=False (or dev_reloader=False).


error messsages

One argument against removing app.config is that app.config makes our error messages really actionable. For example, the suppress exceptions error message:

Attempting to assign a callback to the application but
the `layout` property has not been assigned.
Assign the `layout` property before assigning callbacks.
Alternatively, suppress this warning by setting
`app.config['suppress_callback_exceptions']=True`

If we deprecate app.config, then our error message would have to be like

Alternatively, suppress this warning by setting
`app = dash.Dash(suppress_callback_exceptions=True)`

which I suppose isn't that much worse.


dash_renderer

certain config variables would get serialized as JSON and embedded in <div id="_dash-config"/> for dash's front-end to consume. This would be a whitelist, not a blacklist, to ensure that we're careful about not serving secret config variables.

this is the current behavior.

@T4rk1n
Copy link
Contributor

T4rk1n commented Aug 2, 2018

What about config in a file ?

@ned2
Copy link
Contributor

ned2 commented Aug 5, 2018

config in constructor
Expose all config arguments through dash.Dash constructor. One day, years from now, this may mean that we'll have hundreds of items in here

I think this makes the most sense from the perspective of accessibility and backwards compatibility. I guess the other option could be to have a dedicated config argument, which is just a dictionary. Not really sure that this is much of an improvement though, and comes at the cost of backwards incompatibility.

config in os.environ

This is a great idea, let's definitely do this!

immutable config
...
So, we'd deprecate app.config
We'd also deprecate the ad-hoc app. settings like app.title

Moving away from imperative ad-hoc setting of config feels much more elegant to me. I'm on board with this.

debug, dev, production

I have a strong preference for not introducing features that depend on Dash being run with app.run_server(). I think it makes sense that Dash has a convenience method for running the Flask dev server, but not everyone uses it, even in a development context. In projects I work on, I typically use a WSGI server for starting Dash apps even in development. Although, I guess I don't know how common this is.

There is also the argument that the Flask docs do not recommend using the Flask.run() method (which run_server uses) for starting Flask apps using the development server, as this can lead to a poorer auto-reload functionality on source-file changes. We would then be forcing people to invoke Dash using a mode of invocation for development that Flask does not recommend for development.

What about setting debug=False on Dash.__init__ and then have the run_server method's arguments take precedence over any shared arguments that are also present in Dash.__init__?

Or example code snippets could use Dash(debug=True) rather than in run_server, in order to have debug mode applied before run time. Either way, it seems hard to avoid the need to tell people to disable debug mode from the provided code snippets, if having debug=True is considered important for code examples.

error messages
One argument against removing app.config is that app.config makes our error messages really actionable. For example, the suppress exceptions error message:

I don't feel like this is that much of an issue. Another advantage of putting all config params in the Dash constructor, is that people sharing snippets on gthe forums etc are less likely to omit ad-hoc changes to config which can currently be defined anywhere in a Dash app's source code that is run at initialisation time.

@chriddyp
Copy link
Member Author

dev/debug mode has moved to a separate proposal: #383. We've also starting moving forward with "config in constructor". I'll close this down since we've settled on this pattern.

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

No branches or pull requests

3 participants