feat: Add dynamic port binding to TCPSite#12167
feat: Add dynamic port binding to TCPSite#12167Dreamsorcerer merged 10 commits intoaio-libs:masterfrom
Conversation
Co-authored-by: J. Nick Koston <[email protected]>
Finishes PR aio-libs#10697, replacing unused_port utility in examples and correctly tracking _bound_port against the resolved socket without mutating _port.
b396198 to
4a9bac4
Compare
d12c727 to
33ce06b
Compare
|
|
dd3d778 to
ee960d7
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #12167 +/- ##
=======================================
Coverage 98.78% 98.78%
=======================================
Files 128 128
Lines 45327 45348 +21
Branches 2405 2407 +2
=======================================
+ Hits 44777 44798 +21
Misses 390 390
Partials 160 160
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
ee960d7 to
0e1ab8f
Compare
bc1a324 to
83062e3
Compare
Per maintainer feedback, the dynamic port binding explanation and example is more useful in the TCPSite reference docs where users looking up TCPSite will naturally find it, rather than buried in the advanced guide.
97ebc1c to
7553ebe
Compare
9b49f22 to
4888fa8
Compare
Backport to 3.14: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply b08d909 on top of patchback/backports/3.14/b08d909a0cb303498513be6916ea4e8a50792f04/pr-12167 Backporting merged PR #12167 into master
🤖 @patchback |
Co-authored-by: Tom Whittock <[email protected]> Co-authored-by: Sam Bull <[email protected]> Co-authored-by: Tom Whittock <[email protected]> Co-authored-by: J. Nick Koston <[email protected]> Co-authored-by: rodrigo.nogueira <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> (cherry picked from commit b08d909)
…12184) Co-authored-by: Tom Whittock <[email protected]> Co-authored-by: Sam Bull <[email protected]> Co-authored-by: Tom Whittock <[email protected]> Co-authored-by: J. Nick Koston <[email protected]> Co-authored-by: rodrigo.nogueira <[email protected]>
| self._reuse_port = reuse_port | ||
|
|
||
| @property | ||
| def port(self) -> int: |
There was a problem hiding this comment.
@rodrigobnogueira @Dreamsorcerer I was thinking that maybe there would be an additional requested_port or smth like that, perhaps. Have you considered such an API?
There was a problem hiding this comment.
the property port in this PR is
@property
def port(self) -> int:
if self._bound_port is not None:
return self._bound_port
return self._portWanna add a property such as below?
@property
def requested_port(self) -> int:
return self._portThere was a problem hiding this comment.
Or... maybe, for the sake of clarity, we could delete property port and have properties bound_port (returns _bound_port) and requested_port (returns _port)
There was a problem hiding this comment.
bound port == requested port unless you specifically said you didn't care what port you get (requested port == 0)
There was a problem hiding this comment.
I'm not clear it would be particularly useful to have a separate requested_port. In most cases the developer will probably know the requested port anyway.
There was a problem hiding this comment.
I was thinking of cases where some other system interfaces w/ aiohttp and would want to know if it's safe to cache the port. But that could be discussed later, when there's a better use case, I suppose.
What do these changes do?
This PR continues #10697 by @twhittock-disguise, adding a
portproperty toweb.TCPSitethat returns the dynamically assigned port when the site is created withport=0.Key changes:
_bound_portto track the actual runtime port without mutating_port(addressing @webknjaz's feedback).portproperty returns_bound_portafterstart(), or the requested_portbefore that.nameproperty to useself.portso logging reflects the real bound port.examples/fake_server.pyto useport=0+site.portinstead of the race-proneunused_port()helper.docs/web_advanced.rstanddocs/web_reference.rst.test_tcpsite_ephemeral_portintests/test_web_runner.py.Are there changes in behavior for the user?
Yes. Users can now retrieve the actual bound port via
site.portafter callingawait site.start(). This is especially useful whenport=0is used for ephemeral port allocation. Previously, users had to resort to the internalsite._server.sockets[0].getsockname()[1]hack.Is it a substantial burden for the maintainers to support this?
No. This is a small, self-contained change to
TCPSitethat adds a read-only property backed by a single new slot (_bound_port). It does not change any existing public API or break backward compatibility. The maintenance cost is minimal.Related issue number
Fixes #10665
Continues #10697
Checklist
CONTRIBUTORS.txtCHANGES/folder