Migrate to GitHub Actions CI and resolve dependency issues #164
Conversation
This file contains 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
Also: - Remove older Python versions from CI - Switch to updated PSF httpbin
This avoids wsgiref incorrectly throwing an exception saying that the return data must be bytes.
- Use hookdns to route calls to proxy.com to localhost proxy now that requests no longer allows localhost:port proxies - Pin urrllib3 to 1.25.11, as more recent versions throw an SSL exception or hang if using a http url as an https proxy
wumpus
approved these changes
May 26, 2024
There was a problem hiding this comment.
The main thing I'd mention is that this PR doesn't have a https https proxy test. I don't know how hard it is to make that work.
Pinning urllib3 (for now) is reasonable if we think actual users of the proxy feature really want http proxies. I don't know if that's true. But it's much better than leaving CI broken.
👍👍
|
Very good point! I've added an issue for that: #165 I'll go ahead and merge this for now so that we can get CI running again for outstanding PRs. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #120
test_capture_http_proxymodule to account for breaking changes in requests and urllib3 (note that this currently means pinningurllib3==1.25.11, as more recent versions no longer allow using http:// scheme URLs with thehttpskey in the proxies dictionary; depending on whether and how this is resolved, we may need to come back to this in the future but for now it gets CI working again)There are some unresolved deprecation warnings that would be best handled in a separate issue/PR, particularly around using
setup.py test, which we'll likely want to resolve by switching to tox as a test runner.