-
Notifications
You must be signed in to change notification settings - Fork 106
Fix proxy connection pool creation #158
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
Conversation
Signed-off-by: Sebastian Eckweiler <[email protected]>
Signed-off-by: Sebastian Eckweiler <[email protected]>
@susodapop please give the proper love to this PR. Thanks @sebbegg for coming up with this |
Thanks for the feedback. Fear not, this hasn’t escaped our notice. It
should merge and be released in the next week or so.
…On Tue, Jun 27, 2023 at 12:23 PM Daniele Antonio Maggio < ***@***.***> wrote:
@susodapop <https://github.com/susodapop> please give the proper love to
this PR.
We were having issues with latest 2.7.0 and I can confirm that when using
a proxy, the library requires the changes in this PR 👍
Thanks @sebbegg <https://github.com/sebbegg> for coming up with this
—
Reply to this email directly, view it on GitHub
<#158 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AECG7B33FFSCYHRBO5J5GK3XNMJI3ANCNFSM6AAAAAAZRPJAAI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Any update when this fix will be released @susodapop ? We are facing the same issue and are working with a patched version with @sebbegg 's fix ATM. |
It will be part of the next release this week.
…On Mon, Jul 10, 2023 at 2:01 AM Marc Gomez ***@***.***> wrote:
Any update when this fix will be released @susodapop
<https://github.com/susodapop> ? We are facing the same issue and are
working with a patched version with @sebbegg <https://github.com/sebbegg>
's fix ATM.
—
Reply to this email directly, view it on GitHub
<#158 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AECG7B4ANGHHCU5YK5GEDU3XPOSDRANCNFSM6AAAAAAZRPJAAI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Signed-off-by: Jesse Whitehouse <[email protected]>
Signed-off-by: Jesse Whitehouse <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks for this!
Signed-off-by: Sebastian Eckweiler <[email protected]> Signed-off-by: Jesse Whitehouse <[email protected]> Co-authored-by: Sebastian Eckweiler <[email protected]> Co-authored-by: Jesse Whitehouse <[email protected]>
Stumbled across this while trying to use the connection from within a proxy environment.
As far as I can tell the proxy cool creation is passed the wrong host/port and was missing the
https
scheme.Without the proper schema urllib won't do a proper
CONNECT
call and the requests later on will fail.It appears
realhost
andrealport
in theTHttpClient
were not used anywhere, but were probably intended for the proxy use-case?Tested this locally against our databricks subscription.
Sebastian Eckweiler ([email protected]), Mercedes-Benz AG on behalf of MBition GmbH (Imprint)