Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-02-06 Thread via GitHub
adutra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1945236123 ## aws/src/main/java/org/apache/iceberg/aws/s3/VendedCredentialsProvider.java: ## @@ -75,10 +76,13 @@ private RESTClient httpClient() { if (null == client) {

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-02-06 Thread via GitHub
danielcweeks merged PR #11992: URL: https://github.com/apache/iceberg/pull/11992 -- 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...@iceb

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-02-06 Thread via GitHub
danielcweeks commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1945150977 ## aws/src/main/java/org/apache/iceberg/aws/s3/VendedCredentialsProvider.java: ## @@ -75,10 +76,13 @@ private RESTClient httpClient() { if (null == client)

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-02-05 Thread via GitHub
adutra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1942704258 ## aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java: ## @@ -192,6 +192,7 @@ private RESTClient httpClient() { HTTPClient.bui

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-02-05 Thread via GitHub
adutra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1942704258 ## aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java: ## @@ -192,6 +192,7 @@ private RESTClient httpClient() { HTTPClient.bui

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-02-05 Thread via GitHub
adutra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1942701609 ## aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java: ## @@ -192,6 +192,7 @@ private RESTClient httpClient() { HTTPClient.bui

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-02-04 Thread via GitHub
danielcweeks commented on PR #11992: URL: https://github.com/apache/iceberg/pull/11992#issuecomment-2635399088 I feel like there may have been a small understanding with how we handle the initial authSession. We don't want to default it to empty otherwise if someone adds a request like `cl

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-02-04 Thread via GitHub
danielcweeks commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1942068160 ## core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java: ## @@ -242,7 +242,10 @@ public void initialize(String name, Map unresolved) { }

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-02-04 Thread via GitHub
danielcweeks commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1942062420 ## aws/src/main/java/org/apache/iceberg/aws/s3/VendedCredentialsProvider.java: ## @@ -74,7 +75,11 @@ private RESTClient httpClient() { if (null == client) {

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-02-04 Thread via GitHub
danielcweeks commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1942060653 ## aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java: ## @@ -192,6 +192,7 @@ private RESTClient httpClient() { HTTPClie

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-02-04 Thread via GitHub
jbonofre commented on PR #11992: URL: https://github.com/apache/iceberg/pull/11992#issuecomment-2634301895 I see @nastra approved. I did a new pass and it looks good to me. @danielcweeks wdyt ? -- This is an automated message from the Apache Git Service. To respond to the message, please

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-30 Thread via GitHub
adutra commented on PR #11992: URL: https://github.com/apache/iceberg/pull/11992#issuecomment-2624547764 @danielcweeks @nastra is there anything else I need to change? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use th

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-29 Thread via GitHub
nastra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1933758553 ## core/src/main/java/org/apache/iceberg/rest/HTTPClient.java: ## @@ -214,92 +227,64 @@ private static void throwFailure( throw new RESTException("Unhandled error

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-29 Thread via GitHub
adutra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1933705991 ## aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java: ## @@ -192,7 +192,8 @@ private RESTClient httpClient() { HTTPClient.bui

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-28 Thread via GitHub
danielcweeks commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1932536821 ## aws/src/main/java/org/apache/iceberg/aws/s3/VendedCredentialsProvider.java: ## @@ -84,6 +85,7 @@ private RESTClient httpClient() { private LoadCredential

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-28 Thread via GitHub
gaborkaszab commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1932459105 ## aws/src/main/java/org/apache/iceberg/aws/s3/VendedCredentialsProvider.java: ## @@ -84,6 +85,7 @@ private RESTClient httpClient() { private LoadCredentials

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-28 Thread via GitHub
nastra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1932371163 ## aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java: ## @@ -192,7 +192,8 @@ private RESTClient httpClient() { HTTPClient.bui

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-28 Thread via GitHub
adutra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1932146669 ## aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java: ## @@ -206,21 +206,24 @@ private AuthSession authSession() { return authSession

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-28 Thread via GitHub
nastra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1932128629 ## aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java: ## @@ -206,21 +206,24 @@ private AuthSession authSession() { return authSession

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-28 Thread via GitHub
adutra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1931940876 ## core/src/main/java/org/apache/iceberg/rest/HTTPClient.java: ## @@ -88,33 +84,30 @@ public class HTTPClient implements RESTClient { @VisibleForTesting static fi

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-27 Thread via GitHub
danielcweeks commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1931252722 ## core/src/main/java/org/apache/iceberg/rest/HTTPClient.java: ## @@ -88,33 +84,30 @@ public class HTTPClient implements RESTClient { @VisibleForTesting sta

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-27 Thread via GitHub
adutra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1930539690 ## core/src/main/java/org/apache/iceberg/rest/HTTPClient.java: ## @@ -88,33 +84,30 @@ public class HTTPClient implements RESTClient { @VisibleForTesting static fi

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-27 Thread via GitHub
nastra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1930517106 ## core/src/main/java/org/apache/iceberg/rest/HTTPClient.java: ## @@ -88,33 +84,30 @@ public class HTTPClient implements RESTClient { @VisibleForTesting static fi

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-27 Thread via GitHub
adutra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1930471448 ## core/src/main/java/org/apache/iceberg/rest/BaseHTTPClient.java: ## @@ -0,0 +1,220 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-27 Thread via GitHub
nastra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1930359628 ## core/src/main/java/org/apache/iceberg/rest/BaseHTTPClient.java: ## @@ -0,0 +1,220 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-22 Thread via GitHub
danielcweeks commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1926123223 ## core/src/main/java/org/apache/iceberg/rest/HTTPClient.java: ## @@ -214,92 +225,63 @@ private static void throwFailure( throw new RESTException("Unhandled

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-22 Thread via GitHub
danielcweeks commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1926121826 ## core/src/main/java/org/apache/iceberg/rest/HTTPClient.java: ## @@ -214,92 +225,63 @@ private static void throwFailure( throw new RESTException("Unhandled

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-22 Thread via GitHub
danielcweeks commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1926113976 ## core/src/main/java/org/apache/iceberg/rest/HTTPClient.java: ## @@ -88,33 +84,30 @@ public class HTTPClient implements RESTClient { @VisibleForTesting sta

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-22 Thread via GitHub
danielcweeks commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1926105323 ## core/src/main/java/org/apache/iceberg/rest/BaseHTTPClient.java: ## @@ -0,0 +1,220 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-22 Thread via GitHub
danielcweeks commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1926099736 ## core/src/main/java/org/apache/iceberg/rest/HTTPClient.java: ## @@ -214,92 +225,63 @@ private static void throwFailure( throw new RESTException("Unhandled

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-20 Thread via GitHub
nastra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1922259637 ## core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java: ## @@ -1486,12 +1349,14 @@ public void testCatalogTokenRefresh(String oauth2ServerUri) {

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-20 Thread via GitHub
nastra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1922265637 ## core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java: ## @@ -2122,23 +1948,18 @@ public void testCatalogTokenRefreshDisabledWithCredential(String oauth2

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-20 Thread via GitHub
nastra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1922263736 ## core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java: ## @@ -1767,29 +1635,23 @@ public void testCatalogValidBearerTokenIsNotRefreshed() { Mockit

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-20 Thread via GitHub
nastra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1922262816 ## core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java: ## @@ -1653,7 +1517,6 @@ public void testCatalogExpiredBearerTokenIsRefreshedWithCredential(String

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-20 Thread via GitHub
nastra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1922260527 ## core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java: ## @@ -1508,20 +1373,21 @@ public void testCatalogTokenRefresh(String oauth2ServerUri) {

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-20 Thread via GitHub
nastra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1922258470 ## core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java: ## @@ -1324,89 +1222,67 @@ public void testTableAuth( Mockito.verify(adapter) .exec

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-20 Thread via GitHub
nastra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1922256880 ## core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java: ## @@ -489,39 +463,29 @@ public void testCatalogBearerTokenWithClientCredential(String oauth2Serve

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-20 Thread via GitHub
nastra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1922256104 ## core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java: ## @@ -337,27 +337,20 @@ public void testCatalogBasicBearerToken() { // the bearer token shoul

Re: [PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-20 Thread via GitHub
nastra commented on code in PR #11992: URL: https://github.com/apache/iceberg/pull/11992#discussion_r1922256503 ## core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java: ## @@ -373,39 +366,29 @@ public void testCatalogCredentialNoOauth2ServerUri() { // no token or

[PR] Auth Manager API part 4: RESTClient, HTTPClient [iceberg]

2025-01-17 Thread via GitHub
adutra opened a new pull request, #11992: URL: https://github.com/apache/iceberg/pull/11992 4th PR for the Auth Manager API. Previous ones: * #11844 * #11809 * #11769 This PR introduces the required changes to `RESTClient` and `HTTPClient`. It also introduces a `BaseHTTP