Preserve header casing. Take two. 🎬#104
Conversation
Codecov Report
@@ Coverage Diff @@
## master #104 +/- ##
==========================================
+ Coverage 99.14% 99.15% +0.01%
==========================================
Files 21 21
Lines 937 952 +15
Branches 173 175 +2
==========================================
+ Hits 929 944 +15
Misses 7 7
Partials 1 1
Continue to review full report at Codecov.
|
|
Okay, I think this is about the best we can do here. Unavoidably it does benchmark marginally slower that the current Before... After... Averaging to: 7295 requests/sec -> 7083 requests/sec What do we think about this folks? @njsmith, @pgjones? (From my POV I'd really like to get this resolved - we're currently treating it as one of our HTTPX 1.0 blockers.) 🙏💚😇 |
|
|
||
| original_headers = [("Host", "example.com")] | ||
| req = h11.Request(method="GET", target="/", headers=original_headers) | ||
| req.headers.raw_items() |
There was a problem hiding this comment.
Will there be output demonstrating the result?
There was a problem hiding this comment.
Yup, see https://h11.readthedocs.io/en/latest/api.html - but I wasn't able to figure out the docs build locally just yet, so I've not seen the rendered docs myself.
There was a problem hiding this comment.
You can see the docs rendered for this pull request at https://h11--104.org.readthedocs.build/en/104/
There was a problem hiding this comment.
Ah neat, thanks @pquentin.
Not 100% obvious what repr we'd want for the Headers there:
- Use
<Headers([(b'host', b'example.org')])>to make it clear it's not (quite) a plain list. - Use
[(b'host', b'example.org')]to keep it looking simple. - Use
((b'host', b'example.org'),)since it's a non-mutable sequence. - Something else?
Some examples as currently rendered...
Co-authored-by: Stephen Brown II <Stephen.Brown2@gmail.com>
|
@StephenBrown2 @pgjones Would one of you be able to review this fine work? I'm not qualified (and not part of python-hyper anyways) and @njsmith isn't currently available for OSS reviews (even on Trio) |
…stie/h11 into preserve-header-casing-2
|
Minded to merge - happy to be corrected - will merge and release over the weekend if not. |
|
@pgjones Fantastic - pretty excited to get this one resolved. 🤗 |


Closes #31
Based on #103, focusing on minimising benchmark regressions.
event.headersis a sequence-like instance, containing two-tuples of bytes.event.headers.raw_items()is a list of two-tuples of(raw_name, value).Three benchmark runs here against each case.
Running against current master...
7330.1 requests/sec
7357.9 requests/sec
7363.4 requests/sec
7310.5 requests/sec
7363.2 requests/sec
7354.3 requests/sec
7363.3 requests/sec
7321.4 requests/sec
7355.2 requests/sec
7353.7 requests/sec
7375.7 requests/sec
7352.6 requests/sec
7381.0 requests/sec
7410.0 requests/sec
7234.4 requests/sec
7274.0 requests/sec
7231.8 requests/sec
7272.5 requests/sec
7263.7 requests/sec
7268.9 requests/sec
7273.7 requests/sec
Running against this PR...
7022.7 requests/sec
7041.2 requests/sec
7052.2 requests/sec
7038.3 requests/sec
7038.5 requests/sec
7042.8 requests/sec
7053.2 requests/sec
7129.9 requests/sec
7145.6 requests/sec
7152.4 requests/sec
7158.3 requests/sec
7154.1 requests/sec
7116.6 requests/sec
7146.2 requests/sec
7080.6 requests/sec
7077.9 requests/sec
7100.3 requests/sec
7051.9 requests/sec
7091.7 requests/sec
7072.6 requests/sec
7056.3 requests/sec