Never compare bytes and str in putheader() #2141
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2141 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 25 25
Lines 2216 2216
=========================================
Hits 2216 2216
Continue to review full report at Codecov.
|
|
I agree it's ugly-ish but nevertheless kudos for finding some way to solve this. I've been pondering on it for some time and couldn't come up with anything even that good. |
|
Actually with the second commit it's not that bad! I wonder if I should add a comment. |
| @@ -184,7 +184,7 @@ def putrequest(self, method, url, *args, **kwargs): | |||
|
|
|||
| def putheader(self, header, *values): | |||
| """""" | |||
| if SKIP_HEADER not in values: | |||
| if not any(isinstance(v, str) and v == SKIP_HEADER for v in values): | |||
There was a problem hiding this comment.
Thoughts on using type(v) is str instead of isinstance()? Also can we use the -bb option as you've done in brotlicffi to ensure these error in the future?
There was a problem hiding this comment.
As mentioned in the initial comment, I can't enable -bb because of brotlicffi (and other issues that I haven't investiaged yet).
I think isinstance() is the pythonic way to do this, and it performs just as well as type() is str (consistently less than 100nsec as mesured by timeit on my laptop).
There was a problem hiding this comment.
Was looking for a speed win in this often-called function :) It's all good though, let's merge this!
Fixes #2071. It's ugly, but I don't know how else to fix that.
I can't enable
-bbbefore python-hyper/brotlicffi#177 is merged and released. (edit: disregard, there are other issues to fix before we can enable-bb.)