-
Notifications
You must be signed in to change notification settings - Fork 198
Explicit proxy support in RequestsHttpConnection #908
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Explicit proxy support in RequestsHttpConnection #908
Conversation
b7c1dbc
to
e0e40bf
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #908 +/- ##
==========================================
- Coverage 71.95% 70.40% -1.56%
==========================================
Files 91 125 +34
Lines 8001 9291 +1290
==========================================
+ Hits 5757 6541 +784
- Misses 2244 2750 +506 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fc8ab2b
to
1dba2cc
Compare
Signed-off-by: Filipe Pina <[email protected]>
1dba2cc
to
f308948
Compare
Sorry for the push force spam, I did not expect so many linting complains off a single lint 😓 Ready for review now |
Thanks. This needs tests, please. Integ test CI is failing, needs to be looked at. Likely the 3.0 upgrade in CI to be done first in a separate PR (please help). |
Signed-off-by: Filipe Pina <[email protected]>
64d6147
to
e31ace0
Compare
Unit test I added. I suppose it's ok like this (as codecov is doing something funky:
Thanks, looking forward to see this merged and stop using an external package 🙏 |
We can ignore codecov for now. @nathaliellenaa are you working on #905? @fopina feel free to take it over |
@dblock I have little to no context in this project setup, already took forever just to get tests to run locally, so I’ll have to pass. |
Looks like it's 2 weeks old, afaik nobody is working on it. |
Description
This PR allows specifying an explicit proxy to the underlying
requests.Session
, overriding environment configuration.This is useful when a specific proxy is required to access OpenSearch but nothing else (such as local tunnels).
Workaround
I've been using a very small/clean subclass across different projects for this, however I think it's clean and useful enough to be pushed to the main upstream class 😄
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.