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.

Sign up for GitHub

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

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5c4b2e0
Add basic argument parsing with `argparse`
Jun 2, 2024
c009ac9
Revert removal of `match` function
Jun 8, 2024
1d959d0
Finish porting CLI to `argparse`
Jun 22, 2024
e13cfab
Fix argv passing in runner
Jun 22, 2024
5ce4d49
Merge branch 'main' into refactor-cli
kgaughan Nov 19, 2024
ed63be4
Fix botched merge
kgaughan Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions docs/runner.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ underscores, ``waitress-serve`` uses hyphens. Thus::

import myapp

waitress.serve(myapp.wsgifunc, port=8041, url_scheme='https')
waitress.serve(app=myapp.wsgifunc, port=8041, url_scheme='https')

Is equivalent to::

waitress-serve --port=8041 --url-scheme=https myapp:wsgifunc
waitress-serve --app=myapp:wsgifunc --port=8041 --url-scheme=https

The full argument list is :ref:`given below <invocation>`.

Expand Down Expand Up @@ -47,7 +47,7 @@ A number of frameworks, *web.py* being an example, have factory methods on
their application objects that return usable WSGI functions when called. For
cases like these, ``waitress-serve`` has the ``--call`` flag. Thus::

waitress-serve --call myapp.mymodule.app.wsgi_factory
waitress-serve --app=myapp.mymodule.app.wsgi_factory --call

Would load the ``myapp.mymodule`` module, and call ``app.wsgi_factory`` to get
a WSGI application function to be passed to ``waitress.server``.
Expand All @@ -64,7 +64,12 @@ Invocation

Usage::

waitress-serve [OPTS] MODULE:OBJECT
waitress-serve [OPTS] --app=MODULE:OBJECT

Required options:

``--app``
Specify WSGI application to run.
Comment on lines +67 to +72
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll break the existing external interface this way.

You should be able to accept the application using both the --app= flag and by passing it as a positional argument.

Copy link
Author

@tribals tribals Jul 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will - but first, I need a general agreement of how everything is implemented, and is argparse "enough" to solve the problem.

On the "positional app argument" - it is not clear now, should this argument be "still first", and for passing app eg. last one should use --app form. Or app should be accept eg. first or last - but this will complicate parsing.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On "argparse" side, right now there are some cases where it is hard to achieve desired results, eg. mutually exclusive groups of options. For now it can be solved as external "validation" step after parsing, and it will suffice, I think.


Common options:

Expand Down
6 changes: 3 additions & 3 deletions src/waitress/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
from waitress.server import create_server


def serve(app, **kw):
def serve(app, opts, **kw):
_server = kw.pop("_server", create_server) # test shim
_quiet = kw.pop("_quiet", False) # test shim
_profile = kw.pop("_profile", False) # test shim
if not _quiet: # pragma: no cover
# idempotent if logging has already been set up
logging.basicConfig()
server = _server(app, **kw)
server = _server(app, opts, **kw)
if not _quiet: # pragma: no cover
server.print_listen("Serving on http://{}:{}")
if _profile: # pragma: no cover
Expand All @@ -20,7 +20,7 @@ def serve(app, **kw):


def serve_paste(app, global_conf, **kw):
serve(app, **kw)
serve(app, {}, **kw)
return 0


Expand Down
13 changes: 11 additions & 2 deletions src/waitress/adjustments.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ class Adjustments:
# (or when using the Proxy settings, without forwarding a Host header)
server_name = "waitress.invalid"

def __init__(self, **kw):
def __init__(self, opts, **kw):
if "listen" in kw and ("host" in kw or "port" in kw):
raise ValueError("host or port may not be set if listen is set.")

Expand All @@ -313,6 +313,12 @@ def __init__(self, **kw):
"send_bytes will be removed in a future release", DeprecationWarning
)

# opts are already validated, so that we set them as is
for k, v in opts.items():
if k not in self._param_map:
raise ValueError("Unknown adjustment %r" % k)
setattr(self, k, v)

for k, v in kw.items():
if k not in self._param_map:
raise ValueError("Unknown adjustment %r" % k)
Expand Down Expand Up @@ -346,7 +352,10 @@ def __init__(self, **kw):
if "]" in port: # pragma: nocover
(host, port) = (i, str(self.port))
else:
(host, port) = (i, str(self.port))
if isinstance(i, tuple):
host, port = i
else:
host, port = (i, str(self.port))

if WIN: # pragma: no cover
try:
Expand Down
Loading