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

Support argument-less factory functions in CLI #1536

Closed
wants to merge 1 commit into from
Closed

Conversation

mbr
Copy link

@mbr mbr commented Jul 17, 2015

Since the cli module does does fairly aggressive autodiscovery of a
flask application object anyway, supporting argument-less factory
functions is not far from the current behavior.

Argument-less factory function do happen if you configuration is based on your environment. There's a whole pattern around it (see http://12factor.net) that Heroku uses. I've written a Flask-Extension to support it (http://github.com/mbr/flask-appconfig) and I want to make sure it integrates well with the 1.0 CLI features.

While it is possible to write a script to get the same functionality, this is a bit of a convention-over-configuration issue and seeing that the CLI already searches pretty thoroughly for an app instance, looking for a callable create_app() function seems reasonable (and very useful!) to me.

This may get a bit confusing if create_app() does exepct arguments, but the resulting

TypeError: create_app() takes exactly 1 argument (0 given)

seems very clear.


This change is Reviewable

Since the cli module does does fairly aggressive autodiscovery of a
flask application object anyway, supporting argument-less factory
functions is not far from the current behavior.

# Finally, look for a factory function
factory = getattr(module, 'create_app', None)
if callable(factory):

This comment was marked as off-topic.

This comment was marked as off-topic.

@mbr
Copy link
Author

mbr commented Aug 19, 2015

It would be great to get some official feedback whether or not this feature has a chance of making it into 1.0. Is there an argument against it?

@RonnyPfannschmidt
Copy link
Contributor

a note from personal oppinion: since one can just pass a (lambda scriptobj: make_app()) to explicitly invoke own app factories, i don't quite see a good reason to include magical calling of code into something that's supposed to find a Flask instance, after all finding something existing is conceptually different from creating something new

@mbr
Copy link
Author

mbr commented Aug 19, 2015

a note from personal oppinion: since one can just pass a (lambda scriptobj: make_app()) to explicitly invoke own app factories, i don't quite see a good reason to include magical calling of code into something that's supposed to find a Flask instance, after all finding something existing is conceptually different from creating something new

I think the difference between creation and discovery in this case is a bit philosophical though; if I am not mistaken many apps that use the factory pattern are structured in a way that there is a single wsgi.py or similar that looks somewhat like this:

import myapp

app = myapp.create_app()

In that case, "discovering" the app by importing the module is equivalent to creating it. I get that there is a difference in semantics, because discovering an app twice would result in the same object, while calling create_app twice obviously gives two separate instances.

What I am really asking for is a way to dig into the discovery process somehow. I'd like to be able to ship Flask-Appconfig with a straightforward way of using the bundled flask tool with it without much trouble, while avoiding creating small boilerplate files like the one described above.

@mbr
Copy link
Author

mbr commented Aug 19, 2015

I should add that there are probably other ways of achieving this, but the ones I can think of right now seem really fragile (such as creating an appplication object automagically upon import and other things).

@RonnyPfannschmidt
Copy link
Contributor

but i really don't see what's wrong with just using the create_app argument to the script context and a lambda - there you can pass in exactly what you want as a function in a explicit manner,

and i think that tops over calling actual functions that wouldn't get auto-invoked at import time

also if you completely configure from the environment, whats wrong with just having a module inside your package that does exactly one thing - call the app-factory?

@mbr
Copy link
Author

mbr commented Oct 29, 2015

but i really don't see what's wrong with just using the create_app argument to the script context and a lambda - there you can pass in exactly what you want as a function in a explicit manner,

That's true, but i am trying to cover an 80% use-case here (at least for a specific style of deployment). Passing in specific arguments means leaving the domain of Flask-AppConfig anyway.

also if you completely configure from the environment, whats wrong with just having a module inside your package that does exactly one thing - call the app-factory?

Because that's exactly what I'm trying to avoid =). With that, I will have a new boilerplate file for each project I am creating, which I am not a fan of.

Ultimately I am just looking for a way to hook into the discovery process or anything before it. At worst, I'll monkeypatch things if I have to, but I'd really like way to get in before auto-discovery happens. This just seemed the cleanest way to do it. As said, I'd be really happy if I found a more acceptable alternative to achieve what I'm trying to do.

@untitaker
Copy link
Contributor

@mbr just pointed this out to me in person and I think it's an important issue. This is not a question of whether it's possible to make the Flask CLI as-is work with app factories, but rather whether we see app factories as second-class convention compared to having an app object called app or application. Currently Flask's CLI obviously does not encourage them at all while supporting the other conventions mentioned "out of the box".

I'm assigning this to @mitsuhiko, because he's the one who came up with Flask's CLI in the first place.

@untitaker
Copy link
Contributor

Discussed this in private with both.

I'm closing this, with the argumentation that app factories, in theory, are only useful when passing arguments to them, and we can't reasonably support that in the CLI (and launcher scripts as described above are as boilerplate-less as you can have it).

What definetly needs update is the documentation. We already merged a lot of PRs, mainly by @wgwz, to fix up the documentation to take the new flask CLI into account. This is just another instance of this issue. See #2027

@untitaker untitaker closed this Sep 18, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
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.

5 participants