gh-76646: Fix proxy_bypass_registry trailing semicolon on Windows#138172
gh-76646: Fix proxy_bypass_registry trailing semicolon on Windows#138172PrinceNaroliya wants to merge 9 commits intopython:mainfrom
Conversation
- _proxy_bypass_winreg_override now strips empty values from ProxyOverride - Added unit test ProxyBypassRegistryTests to check trailing semicolon behavior
|
Hi, I’ve implemented the fix for #76646 (_proxy_bypass_winreg_override trailing semicolon issue) and added the corresponding unit test in test_urllib.py.
However, the CI keeps failing with bedevere/news — News entry not in an appropriate directory even though the file is correctly placed. Could you please guide me on what I might be missing or if there’s a way to get the NEWS check to pass? Any help would be greatly appreciated! |
AA-Turner
left a comment
There was a problem hiding this comment.
Review PEP 8, the style guide for the standard language. Notably, we use underscores to separate words in variable names, which are themselves lowercase.
You may also find the Python Developer's Guide instructive.
A
|
|
||
| # Same as _proxy_bypass_macosx_sysconf, testable on all platforms | ||
| def _proxy_bypass_winreg_override(host, override): | ||
| def _proxy_bypass_winreg_override(host, proxyOverride): |
| proxy_override = override.split(';') | ||
| for test in proxy_override: | ||
| test = test.strip() |
There was a problem hiding this comment.
This code is clearer. If the fix is to skip empty entries, just add ``if not test: continue`.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
You must use the blurb tool. I advise a close reading of the devguide. |
|
Thank you for the review and helpful feedback. |
|
Hi, I tried my best to fix this issue, but as a beginner, I couldn’t resolve it completely. I’m happy to learn from feedback or if needed, this PR can be reassigned to someone else who can tackle it. Thanks for your guidance! |
|
Ok, closing. Well done for having a go. A |
Problem:
Fix:
_proxy_bypass_winreg_override, strip empty entries from ProxyOverride.proxy_bypassbehaves correctly on Windows when registry value ends with ';'.Test:
ProxyBypassRegistryTestsintest_urllib_proxy_bypass.pynotmatching.com) returns Falselocalhostandsub.example.com) return TrueAll tests pass locally:
python -m test test_urllib -v✅