python-trio / python-trio/trio
Should serve_* take positional *args and pass them on to the handler?
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 7.3k
- Forks
- 431
- Avg merge
- 2d 17h
- Merged PRs (30d)
- 6
Description
Apparently @dstufft finds it annoying that you can't pass positional arguments through serve_tcp. He has a point! It is weird, when basically every other function like this in trio does allow you to pass-through positional arguments.
There are two issues. First, say we pass handler and *args to serve_tcp. Does this ultimately call:
handler(stream, *args)
or does it call
handler(*args, stream)
? @dstufft suggests that it should call handler(stream, *args), because this allows you to easily write code like:
async def handler(stream, *args):
pass_arguments_through_to_someone_else(*args)
which is much more awkward with the handler(*args, stream) form. That seems fairly convincing, though I'm still a bit nervous that people will be confused. (Even if one option is better once you think about it a lot, that doesn't mean it's obvious to a new user!)
The other issue is that if we added this right now, we'd have calls like:
await serve_tcp(handler, port, *args)
→ await handler(stream, *args)
await serve_ssl_over_tcp(handler, port, ssl_context, *args)
→ await handler(stream, *args)
await serve_listeners(handler, listeners, *args)
→ await handler(stream, *args)
It's weird how the the final call gets assembled by taking the first argument, and then skipping some arguments, and then taking the rest.
Here @dstufft also points out that we could make these kwargs, so it'd be:
await serve_tcp(handler, *args, port=port)
await serve_ssl_over_tcp(handler, *args, port=port, ssl_context=ssl_context)
await serve_listeners(handler, *args, listeners=listeners)
This is... pretty compelling actually, even on its own merit. Trio's general rule is that in runner(callable, *args, **kwargs), the positional args belong to callable, and the kwargs belong to runner. Moving serve_*'s configuration arguments to kwargs would make them much more consistent with the general rule. Plus writing serve_tcp(handler, 80) has always kind of bothered me (what's that random magic constant?). But serve_tcp(handler, port=80) looks quite nice. I'm actually not sure why I didn't do it this way in the first place. Maybe because I'm still stuck in the past and forgot we can actually use mandatory kwargs now?
If we do implement both of these changes then it'd be a kind of disruptive transition, where every serve_tcp call needs to convert from serve_tcp(handler, port) → serve_tcp(handler, port=port). But I do think we can do a regular deprecation warning, so it's not wildly out of line with other API adjustments we've made.
So I'm... tentatively kind of thinking this might be a good idea? Not entirely sure yet. What do y'all think?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing the serve_tcp, serve_ssl_over_tcp, and serve_listeners entry points and their existing callers. Before changing them, resolve the handler argument order, keyword-only configuration, and deprecation transition; the work is done only when the API decision, compatibility behavior, and affected tests are agreed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- api, networking
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100