#12243 wsgi: drop Python 2 variants of _wsgiString and _wsgiStringToBytes#12244
Merged
itamarst merged 3 commits intotwisted:trunkfrom Jul 20, 2024
Merged
#12243 wsgi: drop Python 2 variants of _wsgiString and _wsgiStringToBytes#12244itamarst merged 3 commits intotwisted:trunkfrom
_wsgiString and _wsgiStringToBytes#12244itamarst merged 3 commits intotwisted:trunkfrom
Conversation
CodSpeed Performance ReportMerging #12244 will not alter performanceComparing Summary
|
Contributor
Author
|
please review I don't see how these changes can explain the benchmark results. There should be no functional change. |
_wsgiString and _wsgiStringToBytes_wsgiString and _wsgiStringToBytes
Contributor
|
The benchmarks have some noise, if you rerun probably it'll go away. |
'str is bytes' is always False now because Python 2 is no longer supported. Let's drop the Python 2 code and only keep the Python 3 variants of the functions _wsgiString and _wsgiStringToBytes Also added type annotation to the changed code
f94482e to
584cb27
Compare
Contributor
Author
|
Spotted some more Python 2 code on the 2nd pass 😅 |
I spotted another instance in the code where Twisted is checking if str equals bytes. This is only true on Python 2. Drop the unnesaccery 'if str is bytes:' statement to simplify the code. Additionally I added type annotation to the modifed functions.
Today it's always Python 3.
c57b253 to
326ba02
Compare
Contributor
Author
|
Benchmark tests now pass and I've cleaned up the fixup commits with a rebase. The changes are a bit more than I initially intended, please let me know if you want me to write the newsfragment. I'm not sure if this crosses the line of being 'misc'. |
Contributor
Author
|
please review |
Contributor
|
Thank you! |
itamarst
approved these changes
Jul 20, 2024
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Scope and purpose
Fixes #12243
str is bytesis alwaysFalsetoday because Python 2 is no longer supported. This pull request cleans up a few instances where this type check is performed intwisted.web.wsgi._wsgiString()and_wsgiStringToBytes()methods which were only used for Python 2. Python 3 variants of these functions are not changed.str is bytescheck in_ErrorStream'swrite()method as it is redundant in Python 3._wsgiString(),_wsgiStringToBytes(),_ErrorStream.write()and_ErrorStream.writelines()The purpose is to simply help with cleanup :)