nginx / unit

NGINX Unit - universal web app server - a lightweight and versatile open source server that simplifies the application stack by natively executing application code across eight different programming language runtimes.
https://unit.nginx.org
Apache License 2.0
5.27k stars 322 forks source link

Feature request: Support application-factory pattern for Python apps #1106

Closed bunny-therapist closed 3 days ago

bunny-therapist commented 5 months ago

"Application factory" means that the "app" object in your python code that you are passing to unit is not the app itself, but a callable that needs to be called to create the app, i.e. the_actual_app = app().

Uvicorn supports this with the --factory option: https://www.uvicorn.org/settings/ Gunicorn also supports this by specifying the app with parentheses: "app()": https://flask.palletsprojects.com/en/3.0.x/deploying/gunicorn/#running

It would be nice if unit supported this for Python apps (WSGI and ASGI). I looked in the documentation but could not find anything about this, and I could not get it to work, so I assume it is currently not supported by unit.

bunny-therapist commented 5 months ago

Flask supports factory thru the magic name "create_app" or "make_app": https://flask.palletsprojects.com/en/2.3.x/patterns/appfactories/

Personally, I would much prefer factories having zero arguments and then there is a "factory" boolean in the configuration, similar to uvicorn. (I find gunicorn's solution too obscure and flask's too complicated).

callahad commented 4 months ago

Thank you for filing this; we definitely want to support the application factory pattern. I especially appreciate your linking to gunicorn / uvicorn / Flask docs here.

We'll need to have a discussion to decide on the best pattern, but consider this officially on our backlog :)

gourav-kandoria commented 2 weeks ago

@callahad Does this issue need work. would be happy to contribute to it.

I have a approach which I tested on my local. Not sure , This is 100% correct or not. Here it is. Allow, to add a new parameter in the config like below:

{
    "applications": {
        "python-app": {
            "type": "python",
            "processes": 4,
            "path": "/www/",
            "targets": {
                "front": {
                    "module": "app",
                    "factory": "application_factory"
                },
                "back": {
                    "module": "app",
                    "factory": "application_factory"
                }
            }
        }
    },
    "listeners": {
        "127.0.0.1:8080": {
            "pass": "applications/python-app/front"
        }
    }
}

For, this need to make change in python related validation object like in nxt_conf_vldt_python_notargets_members and nxt_conf_vldt_python_target_members. which will validate this field.

and now in the nxt_python_set_target, we can set target, if factory option is there. by calling the application factory using PyObject_CallObject and setting returned object to the target.

If support for arguments for application factory need to be given, that can also be achieved with some additional changes

ac000 commented 2 weeks ago

I think what's missing is the why this is needed? and how does it relate to "app"? if you specify "factory" do you need to specify "app"?

gourav-kandoria commented 2 weeks ago

If you are asking about why the separate "factory" option is needed. So my point is that "callable" by its name suggests that it is some callable which have interface as per wsgi or asgi standard. So, if "factory" as a different option is introduced that would mean it is some callable, if called would return callable supporting wsgi or asgi standard.

There can be other way of doing this like, if factory option is passed as true, then the callable should be considered as which factory which when called should return wsgi or asgi supporting callable

So, if we go by first way- then below is more explanation It can be like either have callable option in the config or factory option . So both must be mutually exclusive. and in both cases , module have to be specified.

so , both the below given config can be said to correct

   "front": {
                    "module": "front",
                    "factory": "application_factory"
                },
                "front": {
                    "module": "front",
                    "callable": "app"
                },

If we go by the second way then , both the below given config can be said to correct

   "front": {
                    "module": "front",
                    "callable": "application_factory",
                    "factory": true
                },
                "front": {
                    "module": "front",
                    "callable": "app"
                },
gourav-kandoria commented 2 weeks ago

If you are asking about why the separate "factory" option is needed. So my point is that "callable" by its name suggests that it is some callable which have interface as per wsgi or asgi standard. So, if "factory" as a different option is introduced that would mean it is some callable, if called would return callable supporting wsgi or asgi standard.

There can be other way of doing this like, if factory option is passed as true, then the callable should be considered as which factory which when called should return wsgi or asgi supporting callable

So, if we go by first way- then below is more explanation It can be like either have callable option in the config or factory option . So both must be mutually exclusive. and in both cases , module have to be specified.

so , both the below given config can be said to correct

   "front": {
                    "module": "front",
                    "factory": "application_factory"
                },
                "front": {
                    "module": "front",
                    "callable": "app"
                },

If we go by the second way then , both the below given config can be said to correct

   "front": {
                    "module": "front",
                    "callable": "application_factory",
                    "factory": true
                },
                "front": {
                    "module": "front",
                    "callable": "app"
                },

any point or suggestion on it

ac000 commented 2 weeks ago

OK, thanks, (it didn't help I mixed callable and app up...). But, yes, you wouldn't specify callable & factory (unless you specify factory as true..)

So I take it "factory" is some Python terminology? I probably need to go read up on it sometime...

bunny-therapist commented 2 weeks ago

"Factory" is just software terminology.

An "application factory" as input to a server runner is supported in a bunch of python server runners, as already described in the issue description. I am not sure about its prevalence in other languages since I don't have experience developing applications in those.

As for the discussion about format, I would personally prefer "callable" to be the path to both factory and app, and a "factory" boolean specifying if it is a factory or not. (The solution called "the second way" above.)

ac000 commented 2 weeks ago

Heh, not something I've come across in over 25 years in the world of C... ah, it's an OOP thing, well that explains everything...

callahad commented 2 weeks ago

If a Java design patterns textbook shows up on your doorstep, it wasn't me 🥸

callahad commented 2 weeks ago

@gourav-kandoria If you have a patch, would you mind opening a pull request? Even if it's unpolished, having something concrete is a great starting point.

If support for arguments for application factory need to be given, that can also be achieved with some additional changes

That's probably my biggest question at the moment. Honestly, my next step that I just haven't gotten around to yet is to survey how other WSGI/ASGI servers handle this pattern. If someone wants to do that research and write up a quick summary, I'd really appreciate it.

I guess I'd at least look at gunicorn, uWSGI, and mod_wsgi on the WSGI side, and uvicorn, hypercorn, and daphne on the ASGI side?

callahad commented 2 weeks ago

(Admittedly, I'm just cribbing from memory on the WSGI side and riffing off the Starlette docs for ASGI. If there are other popular servers not represented, do let me know)

ac000 commented 2 weeks ago

If a Java design patterns textbook shows up on your doorstep, it wasn't me 🥸

I could use a door stop!

callahad commented 2 weeks ago
callahad commented 2 weeks ago
callahad commented 2 weeks ago

So, it seems:

  1. Support for factories isn't ubiquitous (gunicorn, uvicorn, and maybe uwsgi?)
  2. Only gunicorn allows passing arguments to the factory

I find myself agreeing with @bunny-therapist's sensibilities:

I would much prefer factories having zero arguments and then there is a "factory" boolean in the configuration, similar to uvicorn. (I find gunicorn's solution too obscure and flask's too complicated).

"Explicit is better than implicit," right?

callahad commented 2 weeks ago

Concretely: Let's add an optional factory boolean that defaults to false but can be used like:

"hello": {
    "module": "hello",
    "callable": "create_app",
    "factory": true
}

To align with common usage, we may want to consider supporting module:app notation instead of separate module and callable keys, but that's out of scope for this discussion.

callahad commented 2 weeks ago

Amusingly, uWSGI does seem to support factories in the gunicorn style (appending () to the module name)