dimas-b commented on code in PR #10753:
URL: https://github.com/apache/iceberg/pull/10753#discussion_r1808837493


##########
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:
##########
@@ -273,35 +227,11 @@ public void initialize(String name, Map<String, String> 
unresolved) {
       this.endpoints = ImmutableSet.copyOf(config.endpoints());
     }
 
-    this.sessions = newSessionCache(mergedProps);
-    this.tableSessions = newSessionCache(mergedProps);
-    this.keepTokenRefreshed =
-        PropertyUtil.propertyAsBoolean(
-            mergedProps,
-            OAuth2Properties.TOKEN_REFRESH_ENABLED,
-            OAuth2Properties.TOKEN_REFRESH_ENABLED_DEFAULT);
     this.client = clientBuilder.apply(mergedProps);
     this.paths = ResourcePaths.forCatalogProperties(mergedProps);
 
-    String token = mergedProps.get(OAuth2Properties.TOKEN);
-    this.catalogAuth =
-        new AuthSession(
-            baseHeaders,
-            AuthConfig.builder()
-                .credential(credential)
-                .scope(scope)
-                .oauth2ServerUri(oauth2ServerUri)
-                .optionalOAuthParams(optionalOAuthParams)
-                .build());
-    if (authResponse != null) {
-      this.catalogAuth =
-          AuthSession.fromTokenResponse(
-              client, tokenRefreshExecutor(name), authResponse, 
startTimeMillis, catalogAuth);
-    } else if (token != null) {
-      this.catalogAuth =
-          AuthSession.fromAccessToken(
-              client, tokenRefreshExecutor(name), token, 
expiresAtMillis(mergedProps), catalogAuth);
-    }
+    authManager.initialize(name, client, mergedProps);

Review Comment:
   Conceptually, the client credential flow is the riskiest part because it 
puts long-term credentials on the wire. Ideally, I think the user should be 
able to control where and when that happens.
   
   For example, it may be desirable for the Spark driver to execute the client 
credentials flow (once) and only send the resulting access and refresh tokens 
to worker nodes.
   
   I'm not sure how feasible that is in the current state of the code, but I'm 
bringing this up for consideration as I think it is an important feature.



-- 
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

Reply via email to