Add separate init_app function for providing the flask app

[multimeric]

Closes #738: Support the Flask Extension API

Contributor Checklist

• I have broken down my PR scope into the following TODO tasks
  • Add defer_app flag
  • Add init_app() method
• I have run the tests locally and they passed. (refer to testing section in contributing)
• I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

optionals

• I have added entry in the CHANGELOG.md
• If this PR needs a follow-up in dash docs, community thread, I have mentioned the relevant URLS as follow
  • this github #PR number updates the dash docs
  • here is the show and tell thread in plotly dash community

[multimeric]

Not entirely sure where to write tests for this. If there's already simple test that tests Dash + external Flask app, we could copy that and edit it to test my init_app() feature. Could someone point me to the right way to do this?

[byronz]

Not entirely sure where to write tests for this. If there's already simple test that tests Dash + external Flask app, we could copy that and edit it to test my init_app() feature. Could someone point me to the right way to do this?

that's a good point, we should be more explicit in the guide.
tests should be added in tests folder, they are categorized by the testing level, your case will go to the test_integration.py file and any unit test can be added into unit/dash folder.

*take note that the whole testing part will be refactored soon, but you are safe to follow the existing pattern. *

[alexcjohnson]

I feel like our kwargs are getting a bit out of hand - can we accomplish this without adding a new one? Like, perhaps switch the server kwarg default to 'new', and that creates a new Flask app rather than looking for falsy like we do now, but you can pass in None (or really anything that's neither 'new' nor an instance of Flask) and we'll treat that like your defer_app=True.

@chriddyp does this make sense to you or is my aversion to new kwargs misplaced?

Either way it's well past time that we make a docstring for this class describing all the kwargs! Not your problem @TMiguelT, I'll take care of that.

[multimeric]

The reason I avoided changing the meaning of any kwargs is because that would break backwards compatibility. However, we could still have a server='defer' mode that would reduce the number of kwargs and not break anything.

[alexcjohnson]

Right, I appreciate your effort to maintain compatibility. This would be a pretty minor breakage though - only if someone explicitly provides a falsy server and expects it to generate the automatic one. I won't claim nobody has a use case for that, but I certainly can't think of a common one. And if we're going to be making breaking changes, now is the time - the next release will be dash 1.0.

[chriddyp]

I conceptually agree that server seems like a decent place for this, given that they have very similar intentions.

[multimeric]

Okay, so how about:

• server=True: Create new server
• server=False: Defer creating server
• server=Flask(): Use existing flask server

[multimeric]

Another issue I've encountered is that, if the you initialise the app later, you probably don't have a database connection yet, so you can't populate the layout using database queries.

For this reason, I would like to have a new kind of callback decorator that will fire after the init_app() method is run.

e.g.

@app.onload(
    dash.dependencies.Output('field_select', 'options'),
)
def get_field_options():
    return some_db_query()

Should I attempt to implement this? What do you guys think?

[multimeric]

Latest build failures are to do with this breaking backwards compatibility 🤷‍♂️

[alexcjohnson]

I like the choice of True (auto), False (defer) and Flask(...) (use existing) but then the default value should be updated to True. I'd also say, given that we're now accepting multiple value types for server, that we should explicitly throw an error if we get neither a Flask nor a bool - I know @chriddyp likes making specific types in exceptions.py, but I'd be fine with just something like ValueError

So something like:

if isinstance(server, bool):
    self.server = Flask(...) if server else None
elif isinstance(server, Flask):
    self.server = server
else:
    raise ValueError('server must be a Flask app or a boolean')

[alexcjohnson]

re: @app.onload - the idea I guess is you'd like the layout to be defined and more or less complete from the start, so we can do callback validation, but then when the external resources become available you want to update that layout based on those resources - without it becoming a network callback that has to fire on every page load.

This could be a nice thing to add, though let's look at the consequences and alternatives a bit first. I can imagine a few potentially confusing aspects here:

If app.layout is a function, we'd need to have the onload callbacks fire not just after init_app but also after every subsequent layout() call. But in that case, why not just have the function itself handle this logic? You'd have things like:
dcc.Dropdown(options=(some_db_query() if connected else []), ...)

If app.layout is a constant, these onload callbacks would presumably fire only once, right after init_app. What if the query results were to change? You'd have to manually restart your app before users see those changes. Maybe that's what you want, you're really not expecting this to change, but we could make this behavior more explicit by encouraging users to directly modify layout and providing a hook to just run a function after init_app? We could make helpers to find and modify a prop (`app.find_in_layout, or you can already preset a mutable object and then later mutate it:

dd_opts = []
app.layout = html.Div(... ... dcc.Dropdown(id='field_select', options=dd_opts), ...)

@app.after_init
def with_db():
    # slice mutation rather than extend, just in case this somehow gets run multiple times
    dd_opts[:] = some_db_query()

    # or a helper to find something in the layout
    app.find_in_layout('field_select').options = some_db_query()

Anyway given that this is pretty easy using a layout function, I'm not entirely convinced we need to add anything, but curious to hear whether you think this is sufficient.

[multimeric]

Yes, in my use case I ended up using a layout function almost exactly as you've shown (including the some_db_query() if connected else []), so I no longer think this is necessary for this PR

[alexcjohnson]

Thanks @TMiguelT - looks great!