Re: [PR] REST: honor OAuth config sent by the server [iceberg]

["flyrain (via GitHub)" <gi...@apache.org>]

flyrain commented on code in PR #10256:
URL: https://github.com/apache/iceberg/pull/10256#discussion_r1590181761


##########
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:
##########
@@ -215,6 +215,12 @@ public void initialize(String name, Map<String, String> unresolved) {
     this.paths = ResourcePaths.forCatalogProperties(mergedProps);
 
     String token = mergedProps.get(OAuth2Properties.TOKEN);
+    // re-resolve these variables in case they were overridden by the config endpoint
+    credential = mergedProps.get(OAuth2Properties.CREDENTIAL);
+    scope = mergedProps.getOrDefault(OAuth2Properties.SCOPE, OAuth2Properties.CATALOG_SCOPE);
+    oauth2ServerUri =
+        mergedProps.getOrDefault(OAuth2Properties.OAUTH2_SERVER_URI, ResourcePaths.tokens());

Review Comment:
   It's a good idea to get certain configs from server. Thanks for doing this. Should we add these into config endpoint spec if we are doing this? Without something in the spec, we are relying on the convention. There is a minor concern that the config response from the server side may happen to have the same name with different meaning. For example, a property named `scope` in the config response may mean different things. cc @danielcweeks 



-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org