Pylons / waitress

Waitress - A WSGI server for Python 3
https://docs.pylonsproject.org/projects/waitress/en/latest/
Other
1.44k stars 164 forks source link

Make MODULE:APP non-positional (but still required) argument #439

Open tribals opened 3 months ago

tribals commented 3 months ago

Currently, the usage of waitress-serve is as follows:

$ waitress-serve --help
Usage:

    waitress-serve [OPTS] MODULE:OBJECT
<...>

This is rather inconvenient, as "application" argument is rarely change over the lifetime of project. But, this forces to pass all the arguments that definitely change before, but still keep "app" somewhere and append it in the end. This makes changing those arguments difficult.

For example, when deploying Flask application (running in dev. server) to Docker, one can write Dockerfile like that:

ENTRYPOINT ["/app/bin/flask", "--app", "your.awesome:app"]
CMD ["run", "--host", "::"]

This makes resulting image something like "runnable artifact" - you can docker run -it your/awesome:latest - and it will work out of the box. Besides that, you can also docker run -it your/awesome shell - and it will throw you right into "application" internals, inside plain old Python REPL. Among those, you can just pass Flask arguments directly to container - and it will work "as expected".

But this is all for development. What about production? Translating this example to waitress-serve yields this:

ENTRYPOINT ["/app/bin/waitress-serve", "your.awesome:app"]
CMD ["oops, nothing to write here..."]

This will fail, because you must pass "arguments" (CMD in Docker vocabulary) before MODULE:APP - and this is unachievable with waitress-serve.

Although one still can implement this logic in "his own CLI, wrapping waitress API directly" - but I argue exactly against that - this is unfortunate that every Python Web programmer need to implement it's own "wrapper" for so much common things - this leads to duplication of logic, maintenance burden and creeps design of application to the dark. The logic is so simple that waitress-serve should be used literally for all deployments.

So, it would be nice to have non-positional, required argument to pass application module/instance to waitress-serve.

Side-note on configuration files: they make configuration harder. Better to pass arguments and set environment variables.

mmerickel commented 3 months ago

I'd accept a PR to allow --app to be used to make the positional arg optional, so long as it remains bw-compat and validates properly that only one of the args can be supplied.

Hard disagree on config files vs CLI args tho, as far as version controlling config files tend to be far superior. :-) I do understand the desired flexibility between CMD and ENTRYPOINT in the case of a Dockerfile, and hence would support the feature request.

kgaughan commented 3 months ago

Hi! So, I'm so so involved in waitress these days, but I still make heavy use of it. I was the person who committed runner.py, and everything I state subsequently should be be taken through that lens, for good or ill.

waitress-serve was implemented the way it was for a specific set of reasons. There were certain compatibility requirements it had surrounding Python 2 that meant getopt was the only reasonable option. If I could've used argparse or something more flexible at the time, I would've. With the transition to Python 3, those reasons disappeared. I'd fully encourage you to submit updated command-line parsing using a more modern parser. It's long overdue. There's literally code hanging around to cope with issues in Python 2.6 quirks still there! That very much needs to go.

Secondly, there was the requirement to keep compatibility with the Adjustments class. This limited what I could do a lot at the time, but it at least meant that I had to pay extra attention to keeping the public interface friendly.

There is a lot of code in runner.py and adjustments.py that could go away, and I'd very much encourage you to submit a PR to fix it all. It wouldn't even necessarily be all that difficult: you'd probably end up with fewer lines of code these days.

kgaughan commented 3 months ago

You could also implement what you're looking for trivially around line 271-276 (also lines 261-263) by checking kw for your new argument and skipping much of that code. I would be very straightforward to do, and between runner.py and adjustments.py, the code is pretty straightforward.

tribals commented 3 months ago

Hard disagree on config files...

I will add up a little, if you please. 😊

The point is that you already have those "configs" in files, somewhere. And when you keep propagating them from files ("sources") to another files, in other "pipeline stages" and environments, eventually you will miss some file - it will go out of sync with source. Crazy things will happen.

Instead, you need to eliminate as much "(configuration) files" as you can, and reconstitute everything from one single source of truth, providing applications and services it's configuration with chain-loading. More on that topic can be read here: https://www.skarnet.org/software/s6/overview.Html.

tribals commented 3 months ago

I'm planning to dive into code on weekends.

tribals commented 3 months ago

Could someone please assign this issue to me?

stevepiercy commented 3 months ago

@tribals in most open source software projects, no one assigns issues, including this one. You can make a comment to let other developers know you are working on it, and create a pull request.

tribals commented 3 months ago

Here is first take on this:

digitalresistor commented 3 months ago

Here is first take on this:

Feel free to create a PR and mark it as draft. This will make it easier to track progress for us as maintainers.