examples/network: Improve HTTP client examples to work with IPv4 and IPv6, and add more comments #15588
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #15588 +/- ##
=======================================
Coverage 98.43% 98.43%
=======================================
Files 163 163
Lines 21295 21295
=======================================
Hits 20961 20961
Misses 334 334 ☔ View full report in Codecov by Sentry. |
dpgeorge
force-pushed
the
examples-network-improve-client-examples
branch
from
548a00d to
242b47a
Compare
dpgeorge
force-pushed
the
examples-network-improve-client-examples
branch
from
242b47a to
20952c7
Compare
projectgus
approved these changes
Aug 27, 2024
Contributor
projectgus
left a comment
projectgus
left a comment
There was a problem hiding this comment.
These changes look good!
The only other thing that I'd suggest is to add an extra comment along these lines to both examples:
# To implement an HTTP client using less code, use mip to install the requests package:
# https://github.com/micropython/micropython-lib/tree/master/python-ecosys/requests
dpgeorge
force-pushed
the
examples-network-improve-client-examples
branch
from
20952c7 to
1a0f9ac
Compare
Member
Author
|
Thanks for the review. I've added that comment to the HTTP and HTTPS client examples (not the non-blocking one, because that doesn't generalise to requests). |
The main changes here are to pass the address family and socket type to `getaddrinfo()`, and then use the result of the address lookup when creating the socket, so it has the correct address family. This allows both IPv4 and IPv6 to work, because the socket is created with the correct AF_INETx type for the address. Also add some more comments to the examples to explain what's going on. Fixes issue micropython#15580. Signed-off-by: Damien George <damien@micropython.org>
`ssl.wrap_socket()` is deprecated in CPython, so use `SSLContext` instead, so the example is a good example to copy. Signed-off-by: Damien George <damien@micropython.org>
Not just the domain name. This gives better HTTP 1.0 examples if someone wants to copy them. Signed-off-by: Damien George <damien@micropython.org>
dpgeorge
force-pushed
the
examples-network-improve-client-examples
branch
from
1a0f9ac to
fd03a05
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The main changes here are to pass the address family and socket type to
getaddrinfo(), and then use the result of the address lookup when creating the socket, so it has the correct address family.This allows both IPv4 and IPv6 to work, because the socket is created with the correct AF_INETx type for the address.
Also add some more comments to the examples to explain what's going on.
And use
SSLContextinstead ofssl.wrap_socket()because the latter is deprecated (in CPython at least).Fixes issue #15580.
Testing
Tested on CPython (only
http_client.pyandhttps_client.py), MicroPython unix and Pico W.