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

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

Open
tribals opened this issue May 29, 2024 · 9 comments · May be fixed by #441 or #457
Open

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

tribals opened this issue May 29, 2024 · 9 comments · May be fixed by #441 or #457

Comments

@tribals
Copy link

tribals commented May 29, 2024

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Author

tribals commented May 29, 2024

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
Copy link
Author

tribals commented May 29, 2024

I'm planning to dive into code on weekends.

@tribals
Copy link
Author

tribals commented May 31, 2024

Could someone please assign this issue to me?

@stevepiercy
Copy link
Member

@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
Copy link
Author

tribals commented Jun 2, 2024

Here is first take on this:

@digitalresistor
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants