harishch1998 commented on code in PR #10052:
URL: https://github.com/apache/iceberg/pull/10052#discussion_r1554322900
##########
core/src/main/java/org/apache/iceberg/rest/HTTPClient.java:
##########
@@ -468,6 +483,19 @@ public Builder uri(String path) {
return this;
}
+ public Builder withProxy(String hostname, int port) {
+ Preconditions.checkNotNull(hostname, "Invalid hostname for http client
proxy: null");
+ this.proxy = new HttpHost(hostname, port);
+ return this;
+ }
+
+ public Builder withProxyCredentialsProvider(CredentialsProvider
credentialsProvider) {
+ Preconditions.checkNotNull(
+ credentialsProvider, "Invalid credentials provider for http client
proxy: null");
+ this.credentialsProvider = credentialsProvider;
Review Comment:
So I also thought of failing the build in such cases but another option was
to simply end up in a no-op if a proxyCredentialsProvider is specified but not
the proxyServer. I ended up doing the latter in the private constructor.
```
if (proxy != null) {
if (credsProvider != null) {
clientBuilder.setDefaultCredentialsProvider(credsProvider);
}
clientBuilder.setProxy(proxy);
}
```
But I'm happy to add an additional notNull check. Will fix the naming.
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
