Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update the proxy authentication #354

Merged
merged 1 commit into from
May 23, 2024
Merged

Conversation

amir-haroun
Copy link
Contributor

@amir-haroun amir-haroun commented Feb 15, 2024

This PR solves the proxy auth issue reported in #353.
The idea is to use the proxy_headers argument of the ProxyManager instead of the Proxy-Authorization http header according to the API docs

@amir-haroun
Copy link
Contributor Author

@rcypher-databricks, @arikfr , @yansonggao-db , @andrefurlan-db , @jackyhu-db ,@benc-db
Can anyone take a look to this PR?

@kravets-levko
Copy link
Contributor

@amir-haroun I was testing these changes a lot, and the code seems to work fine. I would like to ask you - can you add some tests for this code? Or maybe you can suggest some ideas on how to avoid regressions in future. Thank you!

@amir-haroun
Copy link
Contributor Author

@kravets-levko,
Not sure what can be added as unit tests here (especially for auth...) but we can verify if the proxies are set using urllib.getproxies().get('http')

@kravets-levko kravets-levko merged commit d63b71b into databricks:main May 23, 2024
@kravets-levko kravets-levko mentioned this pull request May 23, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants