Skip to content

Conversation

@harishch1998
Copy link
Contributor

This PR includes code changes to extend the capability of the existing rest HTTPClient to allow setting a proxy server with basic username/password authentication. The requests made by the HTTPClient will be routed via the proxy server if the HTTPClient is configured with one. If the HTTPClient is not configured with a proxy we default to the current behavior.

@github-actions github-actions bot added the core label Mar 28, 2024
Copy link
Contributor

@amogh-jahagirdar amogh-jahagirdar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @harishch1998 ! I think this is close just a few nits and some additional validation I think we should add.

@harishch1998 harishch1998 requested a review from nastra April 10, 2024 23:23
Copy link
Contributor

@nastra nastra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I'll wait in case @amogh-jahagirdar has any other comments

@nastra
Copy link
Contributor

nastra commented Apr 11, 2024

@harishch1998 can you please rebase the PR and fix the merge conflict(s)?

@amogh-jahagirdar
Copy link
Contributor

Once it's rebased, I'll run the checks and after they pass, I'll merge! Thanks for contributing @harishch1998 , and thanks @nastra for the review

harishch1998 and others added 7 commits April 11, 2024 17:55
* Extend httpclientbuilder to allow setting proxy

* Add tests and address comments

* Fix preconditions message

* Add basic authentication for proxy
@harishch1998 harishch1998 force-pushed the snow-extend-httpclientbuilder-to-allow-setting-proxy branch from 0753883 to edfade3 Compare April 11, 2024 18:17
@amogh-jahagirdar amogh-jahagirdar merged commit 0bc6dfa into apache:main Apr 11, 2024
sasankpagolu pushed a commit to sasankpagolu/iceberg that referenced this pull request Oct 27, 2024
zachdisc pushed a commit to zachdisc/iceberg that referenced this pull request Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants