v20 issue: Failed to find application object 'create_app()' in 'app'

[tjwaterman99]

I had neglected to pin my version of gunicorn, and the run command started breaking this morning when I re-deployed my app and it automatically upgraded to 20.0.

Downgrading my version of gunicorn back to 19.9 fixed the issue.

This is the command I'm using to run my app:

gunicorn 'app:create_app()' --workers 4 --threads 4 --bind 0.0.0.0:$PORT

The error is:

Failed to find application object 'create_app()' in 'app'

[joshuamhtsang]

I have experienced this issue also, i.e.
Failed to find application object 'create_app()' in 'app'
and pinning to version 19.9.0 solves the issue.

I initially though the fix was to change the gunicorn command from:
gunicorn --bind 0.0.0.0:$PORT app:create_app()
to:
gunicorn --bind 0.0.0.0:$PORT app:create_app
(notice the brackets after create_app are now gone). Initially, all seems well:

website_1 | [2019-11-10 19:18:54 +0000] [1] [INFO] Starting gunicorn 20.0.0
website_1 | [2019-11-10 19:18:54 +0000] [1] [INFO] Listening at: http://0.0.0.0:8000 (1)
website_1 | [2019-11-10 19:18:54 +0000] [1] [INFO] Using worker: sync
website_1 | [2019-11-10 19:18:54 +0000] [11] [INFO] Booting worker with pid: 11

But alas it is only a mirage because when you try load your flask website/endpoint it will say:

[2019-11-10 19:20:28 +0000] [11] [ERROR] Error handling request /
website_1 | Traceback (most recent call last):
website_1 | File "/usr/local/lib/python3.7/site-packages/gunicorn/workers/sync.py", line 134, in handle
website_1 | self.handle_request(listener, req, client, addr)
website_1 | File "/usr/local/lib/python3.7/site-packages/gunicorn/workers/sync.py", line 175, in handle_request
website_1 | respiter = self.wsgi(environ, resp.start_response)
website_1 | TypeError: create_app() takes 0 positional arguments but 2 were given

This is clearly a problem with gunicorn version 20.0.0.

[benoitc]

It must be related to this change: 3701ad9#diff-0b90f794c3e9742c45bf484505e3db8dR377 via #2043 .

One way to fix it on your side is to have exported myapp = create_app() in your main module and ten start with app:myapp. This should work, let me know if it doesn't.

I will look if something need to be done there. @berkerpeksag why the removal of the eval was needed there?

[jrusso1020]

It must be related to this change: 3701ad9#diff-0b90f794c3e9742c45bf484505e3db8dR377 via #2043 .

One way to fix it on your side is to have exported myapp = create_app() in your main module and ten start with app:myapp. This should work, let me know if it doesn't.

I will look if something need to be done there. @berkerpeksag why the removal of the eval was needed there?

I have made this change in my application and fixed the crashing, Gunicorn is now able to run my application by saving the result of create_app() in a variable and exporting it so that it can be used in my Gunicorn run command

# app.py
def create_app():
    ...

my_app = create_app()

gunicorn "app:my_app" --workers 8

[joshuamhtsang]

I can confirm that doing what @benoitc and @jrusso1020 suggested above fixes the problem. Thanks all!

[cometaj2]

I don't see the fix working if parameter passing is required at launchtime like:

gunicorn --chdir hcli_core path "hcli_core:HCLI("hcli_core sample hfm").connector".

Parameter passing works in 19.9.0 but fails in 20.0.0.

[tjwaterman99]

@benoitc in case it's helpful to know, the flask docs recommend the app:create_app() pattern when using gunicorn. I suspect some of your new users first try gunicorn as a result of building flask apps, and they'll attempt to use the now-broken recommendation from those docs (that was my experience at least).

I can reach out to that team to ask them to update, however I'll wait for @berkerpeksag to weigh in on the dropping of exec in case it makes sense to bring it back in.

[benoitc]

@tjwaterman99 well I am not sure I like passing arguments this way to an app. I don't think it's a good idea. Arguments should be passed via the env.

Our own example of Flask usage is doing what I describe . I'm thinking the current way is simpler to handle. Thoughts?

[benoitc]

cc @tilgovi @berkerpeksag ^^

[aberres]

FWIW we are running into this as well.

I expect out there are quite a lot of people following the "application factory" Flask pattern.
There is a work around for sure, but at least the changelog should mention this as breaking change.

[berkerpeksag]

I don't think we ever intentionally supported usages such as app:callable() and app:callable(some, args). I'd say it was an unfortunate side effect of using eval() in the previous implementation.

The current implementation is pretty close to what Django's import_string() does:

https://github.com/django/django/blob/master/django/utils/module_loading.py#L7-L24

I'd be happy to improve documentation, add a release note, and raise a more descriptive error message.

[benoitc]

I don't think we ever intentionally supported usages such as app:callable() and app:callable(some, args). I'd say it was an unfortunate side effect of using eval() in the previous implementation.

Yes I agree. We never supported such way to start an application as far as I am looking.

I'm +1 for a more explicit error. Maybe we should raise an error if the application name is not a simple name?

[fiskl]

Please keep in mind that this was explicit behaviour mentioned in public documentation for one of the major wsgi frameworks (flask), and has previously been supported by your project. Removing the eval prevents lazy initiation of an application, which is a problem if an application is 1) provided by a library, and 2) incurs non-trivial setup costs. If there is no security or other reason why an eval is inappropriate, would it not be simpler just to continue to support your existing behaviour?

[fiskl]

In case anybody encounters a similar case, the appropriate workaround from Python 3.7 onwards would be faking a module-level variable by creating a module-level __getattr__, as per this PEP. That would allow lazy initiation (a la application factories) without hitting the breaking change in gunicorn 20.0.0.

[benoitc]

Well, we never supported such behaviour really, none of our docs or examples use it. That doesn't fit the command line.

But right this is really a breaking change and unexpected . I would be then in favor to put back the eval and warn the user about a deprecated behaviour. Maybe also to replace it and let people use a "factory" design pattern we could add a setting --init-args:

gunicorn -b :8000 --init-args="arg1,arg2"  app:factory_method

Or something like it. Thoughts?

[connorbrinton]

@benoitc Supporting factory methods with an explicit command-line flag would be excellent 😄 Maybe something like:

$ gunicorn -b :8000 \
  --callable \
  --callable-arg "abc" \
  --callable-arg "xyz" \
  --callable-kwarg "key" "value" \
  app:factory_method

(Or maybe another base name, like --factory)

I've been running into issues with this change because there's no longer a way for me to easily run tests. Because (i) my app depends on environment variables, (ii) test collection loads all modules (for doctests) and (iii) I can no longer defer app construction until after import, I can't test my project without adding a long string of environment variables before every test command, and testing takes longer than it used to.

Since I'm on Python 3.7, I think I can hack around this with a module-level __getattr__, but for pre-3.7 I don't think there's any solution to this issue besides downgrading.

I think supporting factory methods with a command-line flag would solve this problem. If I'm missing an obvious solution though, other suggestions would also be appreciated 🙃

[tjwaterman99]

@tjwaterman99 well I am not sure I like passing arguments this way to an app. I don't think it's a good idea. Arguments should be passed via the env.

Our own example of Flask usage is doing what I describe . I'm thinking the current way is simpler to handle. Thoughts?

I agree, I think passing arguments via the environment is more intuitive and encourages users to have their configuration live in one place. However supporting callable objects / factories is important for at least Flask, and perhaps other frameworks too.

+1 for raising a warning, and providing instructions on how to use Gunicorn with factories before deprecating exec next release.

[tilgovi]

It's unfortunate that this happened. We have have two choices of how to respond. We could change the behavior back or we can help everyone migrate.

If we were to change the behavior back, it might make sense to pull the release from PyPI, but I think this is too drastic. Gunicorn never documented or suggested this usage.

Therefore, I suggest we just help everyone adapt and apologize for the inconvenience.

We should reach out to Flask with a PR to update their documentation. I'm happy to do that. I think others are already documenting the migration path here.

I'll add to the suggestions that it can be useful to have a separate module or script that imports the application factory, calls it, and exports it. That can serve as the Gunicorn entry point and it can be omitted from doctest and other tooling so that it doesn't trigger unwanted imports when running these tools in development. Something like a __main__.py or web.py script can work for this.

In the future, we should make release candidates available even when we think releases should be safe. We could have caught this with a release candidate and then had an opportunity to document the breaking change in our release notes, or deprecate it for a cycle.

I don't think it makes sense to add support for initialization arguments on the command line. It's too late for this release; we already support custom applications for advanced use cases; and many frameworks have their own recommended ways to pass settings to applications. Gunicorn should not need to provide its own. Trying to add arguments to fix this issue expands the surface area for this kind of breaking change in the future. We should aim to minimize the CLI surface of Gunicorn as much as is practical.

[tilgovi]

We should reach out to Flask with a PR to update their documentation. I'm happy to do that. I think others are already documenting the migration path here.

I see that @bilalshaikh42 already has done this at pallets/flask#3421

[ThiefMaster]

(One of the Flask maintainers here)

While I fully agree with getting rid of eval there, I think there should be explicit support for app factories! The whole point of an app factory is to avoid having an importable app object (since using that often results in circular dependency hell).

In the flask run cli (only for development) we actually added explicit support for app factories, because they are so common.

Sure, creating a wsgi.py containing from myapp. import make_app; app = make_app() is easy. But I either need to maintain this file separately (which is inconvenient because now pip install myapp won't install everything needed to just run it), or put it in my package (which means that now you could import it from within the app itself which would be wrong)

In Flask we went for an explicit way of checking for a callable app factory and calling it without resorting to eval - maybe you could consider something like this as well? If you want less magic you could even use different CLI args for pointing to an application and for pointing to an app factory.

[davidism]

That causes application to always be evaluated when importing the code, defeating the purpose of the factory.

[jamadden]

A WSGI callable can also be a class instance, so perhaps this is what was intended:

class Application:
    _app = None
    def __call__(self, environ, start_response):
        if self._app is None:
            from wherever import make_app
            self._app = make_app()
        return self._app(environ, start_response)

application = Application()

(The zope.hookable example is essentially the same, just less overhead in the steady-state condition.)

[davidism]

Having a WSGI app that creates the real WSGI app is not ideal. It's now an extra function call on every request to proxy to the real entry point. Setup should be done before the first request, but now it's deferred until then, making the first request take (potentially a lot) longer.

The functionality in question here is a factory function that creates that object based on runtime / environment config, which is useful for decoupling the application parts, avoiding circular imports, and easier test isolation. Having somewhere in the code that explicitly calls the factory sort of defeats the decoupling purpose, as I guarantee you users will then think "oh, I should import this app object around now" when they should instead use the features available to them in Flask.

At this point, all we're asking for is "if the import string ends with parens, call the imported name to get the app".

[tilgovi]

I think we have an abundance of ways around this, but that just means we're not the target audience. I know that I can do things like ship a script that's outside the package as an entrypoint for my container and configure pytest to ignore it, etc, etc, but we want to care about the people for whom this breaks who are following tutorials and might not understand the traceback.

A very limited "if the app object is callable with zero arguments, then invoke it as a factory" pattern might work, but it fails if the callable is actually a WSGI application that's decorated badly and doesn't reveal its arguments so easily from introspection. If we want to be generous, we should support everything we had before while avoiding eval, so I think that's the path we should do.

I really appreciate all the suggestions and help resolving this, everyone.

[tilgovi]

I like both the suggestions from @davidism and @connorbrinton using literal_eval.

[benoitc]

That causes application to always be evaluated when importing the code, defeating the purpose of the factory.

Well it would init the app on runtime and return a callable used by the workers. That's not that different.

My main reserve about that pattern is it encourage people to run some code pre-spawn which can break the expectations on HUP or USR2. Also it breaks the current UI. Will it work with future usages of gunicorn?

Anyway the choices are the following then:

1. we can consider this behaviour was unsupported, undocumented (in gunicorn). The change that was done based on it.
2. some users were relying on it and now we want to support that behaviour

(1) is a hard take, but also the logical path, considering we never supported it.
(2) some kind of aesthetic and breaks the command line UI, it needs some tests/examples to test in gunicorn, use something like literal_evals

We can support (2) but I would like to have some testing . Also should we document it?

@tilgovi @jamadden @berkerpeksag @sirkonst what is your preference?

[davidism]

Looks like another breaking use case was attribute access, which won't be solved by literal_eval.

For example, in Plotly Dash you use the Dash object, which internally has a Flask instance as the server attribute. Some people were using:

gunicorn "module:app.server"

I'm not sure this should be supported though. flask run doesn't support it either. Seems like the Dash object should have a __call__ method that passes to Flask.__call__. Additionally, Dash's docs say to do server = app.server and point Gunicorn at that, so this seems to be mostly a case of some incorrect information being passed around.

[benoitc]

@davidism i have been sick today but I will have a look this sunday about it for a release on monday. The suggestion from @tilgovi is good and the general plan is to have a safe eval replacing the old eval.

Those I think we should warn the user he's using such initializtion. Thoughts ? cc @tilgovi

[davidism]

I'll try to turn the branch I linked above into a PR with tests on Saturday, unless you wanted to create a different implementation.

[benoitc]

@davidism go ahead. I will be back on sunday where I will review it if neeed :) Thanks!

[davidism]

A little behind, working on this now.

@connorbrinton cool idea to use ast.parse, I'll try that out and include you as a co-author on the commit if I go with it.

[di]

Just wanted to chime in that there is a somewhat popular (and quite old) Stack Overflow answer which is directing users towards the v19 behavior, which might need an update depending on what choice is made: https://stackoverflow.com/questions/8495367/using-additional-command-line-arguments-with-gunicorn

[benoitc]

fixed in master. thanks @davidism for the patch!

all cases handled are in this tests: 19cb68f#diff-5832adf374920d75d4bf48e546367f53R67