Open Ousret opened 9 months ago
update:
debug + verbose output
The remaining 9 cases run fine if standalone. something tampers with the exception management. thus making assertions fail consistently. need investigation.
:warning: Please install the to ensure uploads and comments are reliably processed by Codecov.
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 0.00%. Comparing base (
4d7d6b6
) to head (7cd6579
). Report is 370 commits behind head on master.:exclamation: Current head 7cd6579 differs from pull request most recent head 55aea9c
Please upload reports for the commit 55aea9c to get more accurate results.
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Update:
Now everything has passed. I identified some parts of the code that could be removed.
https GET pie.dev/get
https -v GET pie.dev/get
https -v --debug GET pie.dev/get
https --ssl=tls1.2 -v --debug GET pie.dev/get
http -v --debug GET pie.dev/get
Tests / test (macos-latest, 3.11) (pull_request) Failing after 2m Details @github-actions Tests / test (macos-latest, 3.12) (pull_request) Failing after 4m
it is about CI flakyness, a restart will fix that.
So far, I identified this:
We can add the following arguments and document them:
--disable-http2
--disable-http3
--http3
, e.g. force trying it.I've added and documented it.
--disable-http2
--disable-http3
--http3
, e.g. force trying it.Ideally, the connection information (TLS, Cipher, Peer/Issuer Certificate, and OCSP) should be in the request metadata and with a lexer instruction.
Now, the result is pleasant to the eye. I have implemented the metadata for Request and the result is as follows:
Niquests now support tracking upload progress, this can also be done here. It should be straightforward enough.
Damn this looks so tantalizing. I'd love for this to get packaged as a tryout / beta. Something installable via e.g. pip install httpie[niquest]
perhaps?
Hello there,
@dwt
Damn this looks so tantalizing.
Couldn't agree more, wait until you see what just landed in this PR.
I'd love for this to get packaged as a tryout / beta.
That's the main idea I had in mind. If you or anyone you know is willing to give this a try, then feel free to share and ask for feedbacks.
@jkbrzt
I, again, am still hopeful you could spare a bit of time to steer this proposal. With the most recent push, there's some interesting news.
Starting now, HTTPie PR/1531 can:
$ https --resolver "doh+cloudflare://" pie.dev/get
or..
$ https --resolver "in-memory://default/?hosts=pie.dev:10.11.11.1" pie.dev/get
or..using the shortcut:
$ https --resolver "pie.dev:10.11.11.1" pie.dev/get
$ https --interface 172.17.0.1 pie.dev/get
$ https -4 pie.dev/get
This is by far, according to me, the most exiting upgrade HTTPie can land and justify the major bump. I took the liberty to draft a changelog for a "potential" 4.0.0 beta 1.
Regards,
Well, that would be the most interesting update to httpie in some time according to me too. :)
@Ousret I'm trying out the http3 support and wasn't able to get it working. Could you perhaps provide some guidance on how to best debug this?
❯ https --http3 -vvv pie.dev/get
Connected to: 188.114.96.4 port 443
Connection secured using: TLSv1.3 with AES-256-GCM-SHA384
Server certificate: commonName="pie.dev"; DNS="*.pie.dev"; DNS="pie.dev"
Certificate validity: "Nov 11 01:14:24 2023 GMT" to "Feb 9 01:14:23 2024 GMT"
Issuer: countryName="US"; organizationName="Let's Encrypt"; commonName="E1"
Revocation status: Good
GET /get HTTP/2
Accept: */*
Accept-Encoding: gzip, deflate
Connection: keep-alive
Host: pie.dev
User-Agent: HTTPie/4.0.0.b1
```shell ❯ https --http3 -vvv pie.dev/get Connected to: 188.114.96.4 port 443 Connection secured using: TLSv1.3 with AES-256-GCM-SHA384 Server certificate: commonName="pie.dev"; DNS="*.pie.dev"; DNS="pie.dev" Certificate validity: "Nov 11 01:14:24 2023 GMT" to "Feb 9 01:14:23 2024 GMT" Issuer: countryName="US"; organizationName="Let's Encrypt"; commonName="E1" Revocation status: Good GET /get HTTP/2 Accept: */* Accept-Encoding: gzip, deflate Connection: keep-alive Host: pie.dev User-Agent: HTTPie/4.0.0.b1 HTTP/2 200 OK Access-Control-Allow-Credentials: true Access-Control-Allow-Origin: * Alt-Svc: h3=":443"; ma=86400 Cf-Cache-Status: DYNAMIC Cf-Ray: 83fa5064bee41d9a-FRA Content-Encoding: gzip Content-Type: application/json Date: Wed, 03 Jan 2024 09:46:20 GMT Nel: {"success_fraction":0,"report_to":"cf-nel","max_age":604800} Report-To: {"endpoints":[{"url":"https:\/\/a.nel.cloudflare.com\/report\/v3?s=Ca0pVbLbx9QmhpCG%2Fq03dTYDMeHurFMramSLFHceVhaq6dMGWBep340o4G9B3t%2FlHxfF5Agpp7UHgkxGa5AMFVyvov%2BhOGIsDikuFWJMVFWp4lN9Xnd%2FILz9"}],"group":"cf-nel","max_age":604800} Server: cloudflare { "args": {}, "headers": { "Accept": "*/*", "Accept-Encoding": "gzip", "Cdn-Loop": "cloudflare", "Cf-Connecting-Ip": "91.65.247.116", "Cf-Ipcountry": "DE", "Cf-Ray": "83fa5064bee41d9a-FRA", "Cf-Visitor": "{\"scheme\":\"https\"}", "Connection": "Keep-Alive", "Host": "pie.dev", "User-Agent": "HTTPie/4.0.0.b1" }, "origin": "91.65.247.116", "url": "https://pie.dev/get" } Elapsed DNS: 0.002622s Elapsed established connection: 0.026514s Elapsed TLS handshake: 0.079068s Elapsed emitting request: 0.000366s Elapsed time: 0.254063667s⏎ ~/C/P/httpie 🐍 tempenv-13fd235984bf4 🌱 feature-tryout-niquests ❯ macosver 10.16 ~/C/P/httpie 🐍 tempenv-13fd235984bf4 🌱 feature-tryout-niquests ❯ python --version Python 3.12.1 ~/C/P/httpie 🐍 tempenv-13fd235984bf4 🌱 feature-tryout-niquests ❯ pip list Package Version Editable project location ------------------ ---------- ------------------------------- certifi 2023.11.17 cffi 1.16.0 charset-normalizer 3.3.2 cryptography 41.0.7 defusedxml 0.7.1 h11 0.14.0 h2 4.1.0 hpack 4.0.0 httpie 4.0.0b1 /Users/dwt/Code/Projekte/httpie hyperframe 6.0.1 idna 3.6 kiss-headers 2.4.3 markdown-it-py 3.0.0 mdurl 0.1.2 multidict 6.0.4 niquests 3.4.0 pip 23.3.2 pycparser 2.21 Pygments 2.17.2 PySocks 1.7.1 python-socks 2.4.4 qh3 0.14.0 requests 2.31.0 requests-toolbelt 1.0.0 rich 13.7.0 setuptools 69.0.3 urllib3 2.1.0 urllib3-future 2.4.902 wassima 1.0.3 ```
OK, I can try. First, running the exact same command locally, work fine.
You don't have to specify --http3
as pie.dev is covered by Cloudflare preemptive QUIC declaration in DNS records.
According to your dependencies listening, you have qh3 installed, so it should work and everything is up to date.
Something is preventing it on your environment. Let's try to understand. Are you using conda? or poetry? or..?
Try to run the following, standalone:
from niquests import Session
with Session(resolver="doh+google://") as s:
print(s.get("https://pie.dev/get"))
and then:
from urllib3.contrib.hface import HTTPProtocolFactory, HTTP3Protocol
# or
from urllib3_future.contrib.hface import HTTPProtocolFactory, HTTP3Protocol
print(HTTPProtocolFactory.new(HTTP3Protocol))
Create a specific issue at Niquests tracker, we shall continue there to avoid spamming this thread.
Thanks to @dwt testing, we discovered an edge case where http3 would be silently disabled, it depended on the ssl ctx created with the system default ciphers list, which vary. With this, I discovered a unusual bug that is present in the 3.x major, see https://github.com/httpie/cli/issues/1551 for more details. It is fixed now (both cases).
Now, this closes 20+ issues. Just added support for --local-port
args. Accept single port or range.
Also fixed a bunch of warnings from v3.x, from 120+ warnings to 24.
gentle ping @jkbrzt
@jkbrzt hey, this code seems like a phenomenal update to the capabilities of httpie. Could you please take a look or recommend someone else you trust to take one?
@Ousret It seems to be hard to reach @jkbrzt - any Idea how to get him to notice? I didn't find any social addresses outside of twitter, and it seems they want me to pay now to send private messages there.
I understand the frustration.
There's almost no chance that @jkbrzt didn't saw this (amongst other PR and issue piling up of course), so there's really no reason for us to ping him any more. This PR is 4 months old already, and we didn't get the slightest sign of whether it is a desirable PR or not.
According to stats at my disposal we have a significant amount of "git clone" that must originate from people that can't setup HTTPie with Python 3.12+ around 10 a week, and I expect that number to quickly raise, this is undesirable, maintenance wise.
I tend to wait 5 to 6 months before giving up on a PR, usually. If, HTTPie would be stuck this way, someone would have to assume a fork, and, well, it won't be me as I have no bandwidth to allocate for long term maintenance.
I have to admit that I'm already very impressed that you invested all of. this work without having any word from the project owner whether he likes this or not. I sure hope that we can get him to reply at some point, because this looks like a big present he got for free for this project.
@jkbrzt How can we reach you? It would be really nice (but after this long a time also neccessary) for you to chime into this pull request and give some information what you would require to get this merged.
@Ousret Impressive stuff, and sorry for the lack of feedback. The initial effort looked like a speculative experiment, but it’s evolved quite a bit since then! I have some questions. Would you be up for connecting over Zoom/Meet to discuss the code, niquests, and the potential switch from requests? If it sounds good, please shoot me a message via jakub at httpie.io.
Thank you for your answer. I would gladly answer any question you have.
Would you be up for connecting over Zoom/Meet
Excellent idea, let's do that. I will email you so that we can schedule a meet.
Regards,
Made some general (minor) code cleanup and fixed almost all warnings.
Passed from (master) ===== 1019 passed, 5 skipped, 4 xfailed, 135 warnings in 71.36s (0:01:11) ======
to (this PR) ====== 1037 passed, 5 skipped, 1 xfailed, 5 warnings in 77.09s (0:01:17) =======
The 5 warnings remaining are going to be annoying to track down and fix for now.
I've applied the initial batch of suggestions @jkbrzt if I missed anything, let me know. Will be waiting for further instructions.
@Ousret thank you for the call yesterday.
I’ve started diving into the code and doing some manual QA.
Here are a few cosmetic issues I’ve noticed so far:
# Test command
$ https -vv pie.dev/get
--vv
output. Currently the output is Elapsed time: 0.118802041s$
where $
is my shell prompt.Elapsed established connection: 4.8e-05s
)OK! The cosmetic issues are now fixed. I took the liberty to add a single assert that verify we've not missed the extraneous RC for the metadata.
For now, all clear.
Did found another cosmetic issue (when follow redirects) the meta was glued to the next request line.
Now fixed. And made the metadata look even nicer by replacing "0.0s" with "0s". (Found it in same scenario where redirect did not require DNS resolve and such..)
Finally, I could resolve 4 warnings, remain 1! ======= 1039 passed, 5 skipped, 1 xfailed, 1 warning in 77.85s (0:01:17) =======
So, I did some additional testing on my end and found nothing. Additionally, to mark this milestone, the very last warning is now fixed!
This branch now have 0 warning. ============ 1039 passed, 5 skipped, 1 xfailed in 81.02s (0:01:21) =============
For a side note, Niquests just landed Happy Eyeballs feature in the last version, if this is of any interest to you, let me know.
and that commit https://github.com/httpie/cli/pull/1531/commits/cce24fea7f933b5978461b31da5bdc5a93c590ae has nothing to do with Niquests or HTTPie really, you must have encountered this situation where a single MacOS test was flaky, this circumvent the random CI failures.
I applied a minor patch to improve how the QuicCapabilityCache blend into HTTPie internals (the path is now generated within env::config and propagated appropriately). Tests added to ensure no regression happen in between.
@jkbrzt @Ousret is there anything I can do to help test this more? I really want to see this land. :)
@dwt You can help by proof reading the modified documentation and look if I missed something regarding this PR in it: https://github.com/Ousret/httpie/blob/feature-tryout-niquests/docs/README.md
Other than that, I think the testing part is pretty much covered.
Finally I have added a test case for the following scenario: "A peer was H3 compatible but not anymore and expect a error msg that explain why you just got timeout and how to remedy".
Issue https://github.com/httpie/cli/issues/1401 was trivial to fix, so, it's fixed and tested. Also https://github.com/httpie/cli/issues/1527 this one was kind of tricky to track but fortunately fixed. We've reached a total of 23 issues fixed :+1:
@dwt You can help by proof reading the modified documentation and look if I missed something regarding this PR in it: https://github.com/Ousret/httpie/blob/feature-tryout-niquests/docs/README.md
I've had a look at the docs and so far everything seems fine. However I noticed the absence of a --http2
flag to force http2. Could you touch on the rationale behind that omission? I am currently debugging a server who has trouble spitting out correct http2, so this would be a useful feature to me.
However I noticed the absence of a --http2 flag to force http2. Could you touch on the rationale behind that omission?
You cannot force HTTP/2 using Niquests. It is negotiated via ALPN/TLS. So if the server misinterpret ALPN or discard it, HTTP/2 won't come. And yes, RFC allows it, see https://datatracker.ietf.org/doc/html/rfc9113#name-starting-http-2-with-prior- but I decided to follow the simplest path forward (ALPN/TLS), as browsers do (https://stackoverflow.com/questions/34076231/why-do-browser-implementations-of-http-2-require-tls). I might reconsider it in the future, but the use cases are rare.
I may be missing something here, but does that imply that the client has no way to force http1 2 or 3 according to its wishes? When setting up server support for these protocols, I would like to have the ability to force all of these protocols for debugging purposes?
I may be missing something here, but does that imply that the client has no way to force http1 2 or 3 according to its wishes?
There's many possibilities, the only missing piece is "force http2" or "disable http1".
Hope that clarify.
Just updated the docs to include previous comment about protocol combo and the availability of HTTP/3 or not. Since last time Niquests made HTTP/3 support far more enjoyable with less dependencies and a faster execution time.
Finally, we had to downgrade macos-14 (macos-latest) to macos-13 because it's relatively new and some dependencies like brotlicffi aren't ready for it.
Hello @Ousret , nice work here ... is there an ETA on when to expect this release to become available? Thank you
No precise ETA. We are waiting upon the owner approval.
Looks like it could be a big step forward if @jkbrzt approves it. Last week I wanted to test resolving over IPv4 vs IPv6. I found that the released version could not help with this task, but it worked perfectly in the fork, with the same -4
and -6
flags that dig
uses.
Excited to see HTTP/2 and HTTP/3 soon landing in HTTPie 🎉
I may be missing something here, but does that imply that the client has no way to force http1 2 or 3 according to its wishes?
There's many possibilities, the only missing piece is "force http2" or "disable http1". "--disable-http2" HTTP/1.1 or HTTP/3 "--disable-http2 --disable-http3" HTTP/1.1 "--disable-http3" HTTP/1.1 or HTTP/2 "--http3" HTTP/3
I was wondering if we could have an equivalent to cURL's http-version flags i.e --http1.0
,--http1.1
,--http2
,--http2-prior-knowledge
,--http3
? Alternatively, it could be a --http-version
flag that accepts either 1.0
, 1.1
, 2
, 2-prior-knowledge
or 3
Excited to see HTTP/2 and HTTP/3 soon landing in HTTPie 🎉
Likewise!
I was wondering if we could have an equivalent to cURL's http-version flags
I first need to add a feature into Niquests, which isn't top priority right now. But it is planed, will be in a future minor. Provided everything goes smooth with this.
regards,
In the latest commit:
This PR showcases how HTTPie could evolve outside of Requests.
Niquests is supposed to be a (mostly) compatible fork of Requests.
Try this preview:
Here are the biggest pros of this:
Obviously, cons:
Complete list of changes in the fork: https://github.com/jawah/niquests/blob/main/HISTORY.md
4.0.0.b1 (unreleased)
User-Agent
, andAccept-Encoding
headers. (#1502)--resolver
. DNS over HTTPS, DNS over TLS, DNS over QUIC, and DNS over UDP are accepted. (#99) (#1531)--interface
. (#1422) (#1531)--local-port
. (#1456) (#1531)-6
or-4
. (#94) (#1531)requests_toolbelt
in favor of directly includingMultipartEncoder
into HTTPie due to its direct dependency to requests. (#1531)multidict
in favor of implementing an internal one due to often missing pre-built wheels. (#1522) (#1531)-1
to retrieve packets as they arrive. (#1531).localhost
to the default loopback address. (#1458) (#1527)Existing plugins are expected to work without any changes. The only caveat would be that certain plugin explicitly require
requests
. Future contributions may be made in order to relax the constraints where applicable.