adutra commented on code in PR #10256:
URL: https://github.com/apache/iceberg/pull/10256#discussion_r1590651661
##########
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);
Review Comment:
I'd argue that the client can already get a `token` from the server's config
endpoint, so why not a `credential`? :-)
Taking a step back, I'd like this PR to focus on what the _client_ is
supposed to do with configuration received from the server, not what the
_server_ is supposed to send. For that, there is an ongoing discussion
[here](https://docs.google.com/document/d/1JUtFpdEoa6IAKt1EzJi_re0PUbh56XnfUtRe5WAfl0s/edit?usp=sharing)
where @jbonofre and others suggest many changes, e.g. the introduction of a
"handshake" or "pre-flight" endpoint to facilitate client auto-configuration,
including auth properties. But server-side changes will take a lot more time.
And back to the client side of things: I think it makes sense for the client
to apply _all_ configuration received from the server. It's then up to the
server to decide which options make sense to send, and which don't.
One important property I'd like to see properly handled is
`oauth2-server-uri`. I'd also like to see the map `optionalOAuthParams`
correctly handled, since it may contain more important config properties in the
future.
--
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
