search

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

Trying to refactor CLI arguments parsing in order to support required, non-positional `--app` argument #441

Open tribals opened 3 months ago

tribals commented 3 months ago

Closes #439.

At this time, it contains only re-write of parsing to stdlib's argparse. You can test overall "look & feel" just by running waitress-serve with --help, and other arguments.

Not all parsing is done yet, but I think the whole Adjustments is not needed anymore - even argparse is powerful enough to handle common types, but "not so common" also - take a look at -4/-6 args (those was not there already, but they go in the style of ip from iproute2 - is it good thing at all?)

Another observation is that there is "boolean" options, but not all of them passed/works the same: some use --arg/--no-arg notation, but some works just as "if it passed then it's true" (but there also "reverse", notable example is --call). Should they all be reworked to --[no]-arg form, or something other?

digitalresistor commented 3 months ago

I am not a fan of adding -4 and -6, you can already specify this by setting what address to bind to.

Also, Adjustments is also used when not using the runner directly, but using https://github.com/Pylons/waitress/blob/main/src/waitress/__init__.py#L6, so unless everything can go through the argparse changes you made, and all the code is updated that uses Adjustments currently (there's a lot of uses throughput the code base) I am not sure we can so easily get rid of it.

Remember that Waitress is not always run from the command line, but also run through things like Paste.

tribals commented 3 months ago

I am not a fan of adding -4 and -6...

The whole point is that they already exist in long form, so -4 is nothing more than a shortcut. Personally I argue that they not needed at all - as you said, one always specify that by passing address explicitly.

Remember that Waitress is not always run from the command line, but also run through things like Paste.

👌

tribals commented 2 months ago

I've finished porting args parsing to, you know, argparse.

I've not touched much Adjustments, and instead just passed already parsed arguments as is, without feeding them to "adjustments processing". On the other hand, this get's activated only when calling create_server from runner - other times everything works as it was.

It works! 😁

Also, I've fixed almost all tests, but some runner tests fail due to incompatibility of exit codes - I didn't find simple way to fix that.

digitalresistor commented 4 weeks ago

I am not opposed to this. I am going to need to chew on it for a little bit and then we may want to figure out how to make this work cleaner with the existing Adjustments and or make all of that cleaner. There's just a lot of tendrils in many different locations.