-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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.
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
Binding to network interfaces #7132
base: master
Are you sure you want to change the base?
Conversation
Finally added support to bind directly to network interfaces.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Codecov Report
@@ Coverage Diff @@
## master #7132 +/- ##
==========================================
- Coverage 97.38% 96.95% -0.44%
==========================================
Files 106 106
Lines 31093 31110 +17
Branches 3875 3878 +3
==========================================
- Hits 30280 30162 -118
- Misses 612 735 +123
- Partials 201 213 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Co-authored-by: Sam Bull <aa6bs0@sambull.org>
for more information, see https://pre-commit.ci
Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>
fallback by catching the :exc:`RuntimeError` on creation:: | ||
|
||
try: | ||
conn = aiohttp.TCPConnector(network_interface="eth0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use the local_addr
argument here too so it's closer to the fallback?
|
||
If your system has several IP interfaces, you may choose one which will | ||
be used used to bind a socket locally:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently, both the original and the updated text have the double used used
bit.
Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>
for more information, see https://pre-commit.ci
Changed it to static |
@@ -954,7 +970,9 @@ def _get_fingerprint(self, req: "ClientRequest") -> Optional["Fingerprint"]: | |||
|
|||
async def _wrap_create_connection( | |||
self, | |||
*args: Any, | |||
factory: functools.partial[ResponseHandler], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actual requirement is this: https://github.com/python/typeshed/blob/main/stdlib/asyncio/base_events.pyi#L117
So, I'd suggest doing the same thing with Callable[[], _ResponseHandlerT]
. Then we can also update the return type so this becomes generic.
To fix the typing error, I'd suggest renaming the first raw_host = req.url.raw_host
assert raw_host is not None
port = req.port
assert port is not None
host_resolved = asyncio.ensure_future(
self._resolve_host(raw_host, port, traces=traces), loop=self._loop
) Lines 1112 to 1118 in f0073af
|
(AF_INET, AF_INET6, AF_NETLINK, AF_TIPC) | ||
or strings (AF_UNIX). | ||
""" | ||
sock = socket.socket(family, socket.SOCK_STREAM) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It probably makes sense to add
sock = socket.socket(family, socket.SOCK_STREAM) | |
assert SOCKET_SUPPORTS_BINDING_TO_DEVICE | |
sock = socket.socket(family, socket.SOCK_STREAM) |
Sorry to bump an old PR but is this stale / is there another way to select a particular network interface when using aiohttp.ClientSession? |
Looks like the PR didn't get finished off. Feel free to fork it and create a new PR with the final changes. |
Hey @Dreamsorcerer Exactly, what is needed to finish this PR? |
The tests are failing, there are conflicts, and there are unresolved conversations above. |
What do these changes do?
Finally added support to bind directly to network interfaces.
Are there changes in behavior for the user?
No, a optional keyword argument has been added. This will allow end-users to specify the network interface, a feature many related projects lacks without monkey-patching.
Related issue number
#6383
Checklist
CONTRIBUTORS.txt
CHANGES
folder<issue_id>.<type>
for example (588.bugfix)issue_id
change it to the pr id after creating the pr.feature
: Signifying a new feature..bugfix
: Signifying a bug fix..doc
: Signifying a documentation improvement..removal
: Signifying a deprecation or removal of public API..misc
: A ticket has been closed, but it is not of interest to users.