Add dynamic port binding #10697
Add dynamic port binding #10697
Conversation
psf-chronographer
bot
added
the
bot:chronographer:provided
|
It was unclear if & where any tests should be added. When I attempted to build the docs locally, the reference to the new port attribute wasn't picked up properly, and I couldn't work out why. |
CodSpeed Performance ReportMerging #10697 will not alter performanceComparing Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #10697 +/- ##
=======================================
Coverage 98.71% 98.71%
=======================================
Files 125 125
Lines 37445 37459 +14
Branches 2075 2076 +1
=======================================
+ Hits 36963 36977 +14
Misses 335 335
Partials 147 147
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Dreamsorcerer
added
the
backport-3.12
Because it's not been added to the reference, so there's nothing to link to.
test_web_runner.py would seem like the obvious place. A test validating the port property works is probably what we want. |
|
Thanks, I had thought the reference docs were generated from the class docstrings. |
|
I have no clue why unrelated tests started failing / hanging after I added a test. If anyone understands this, please let me know and I'll fix it. The added test completes correctly locally, runinng all tests never completed for me locally, even when running in the master branch. Presumably because there's some part of the development setup I'm unaware of. |
Co-authored-by: J. Nick Koston <nick+github@koston.org>
| if self._port == 0: | ||
| # Port 0 means bind to any port, so we need to set the attribute | ||
| # to the port the server was actually bound to. | ||
| self._port = self._server.sockets[0].getsockname()[1] |
There was a problem hiding this comment.
I wouldn't reassign this but track the bound port separately. This is so starting and stopping the TCP site multiple times would be able to get a new port allocated each time.
|
|
||
| await site.start() | ||
| assert site.port != 0 | ||
| assert site.name.startswith("http://0.0.0.0:") |
There was a problem hiding this comment.
Since we know the port, this could just compare the entire string..
| @@ -113,6 +113,11 @@ def name(self) -> str: | |||
| host = "0.0.0.0" if not self._host else self._host | |||
| return str(URL.build(scheme=scheme, host=host, port=self._port)) | |||
|
|
|||
| @property | |||
| def port(self) -> int: | |||
| """Return the port number the server is bound to, useful for the dynamically allocated port (0).""" | |||
There was a problem hiding this comment.
Please, avoid having the docstring title line exceed 72 chars. You can have a longer paragraph below.
Also, referencing a magic number is not useful to readers unfamiliar with it. Use the ephemeral terminology so people could at least google it. Although, I would probably not even mention it since the underlying implementation details aren't really important to the end-users who'd just use this unconditionally.
What do these changes do?
As per #10665 the TCPSite has a public
portproperty which is updated with the bound port number if it was dynamically allocatedAre there changes in behavior for the user?
Users are now able to rely on the port property and name property on TCPSite having the correct port number if they were relying on port
0dynamic allocation.Is it a substantial burden for the maintainers to support this?
Port 0 is a standard feature in all supported OSs, and leverages the OS capabilities, rather than any specific logic in aiohttp, so the burden should be small.
Related issue number
Fixes #10665
Checklist
CONTRIBUTORS.txtCHANGES/foldername it
<issue_or_pr_num>.<type>.rst(e.g.588.bugfix.rst)if you don't have an issue number, change it to the pull request
number after creating the PR
.bugfix: A bug fix for something the maintainers deemed animproper undesired behavior that got corrected to match
pre-agreed expectations.
.feature: A new behavior, public APIs. That sort of stuff..deprecation: A declaration of future API removals and breakingchanges in behavior.
.breaking: When something public is removed in a breaking way.Could be deprecated in an earlier release.
.doc: Notable updates to the documentation structure or buildprocess.
.packaging: Notes for downstreams about unobvious side effectsand tooling. Changes in the test invocation considerations and
runtime assumptions.
.contrib: Stuff that affects the contributor experience. e.g.Running tests, building the docs, setting up the development
environment.
.misc: Changes that are hard to assign to any of the abovecategories.
Make sure to use full sentences with correct case and punctuation,
for example:
Use the past tense or the present tense a non-imperative mood,
referring to what's changed compared to the last released version
of this project.