nastra commented on code in PR #9523:
URL: https://github.com/apache/iceberg/pull/9523#discussion_r1461677965
##########
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:
##########
@@ -1022,11 +1022,17 @@ public void commitTransaction(SessionContext context,
List<TableCommit> commits)
UpdateTableRequest.create(commit.identifier(),
commit.requirements(), commit.updates()));
}
+ Map<String, String> props = properties();
+ if (null != context.credentials()) {
+ props = RESTUtil.merge(properties(), context.credentials());
+ }
+
+ AuthSession authSession = tableSession(props, session(context));
Review Comment:
I think the `table` part in the method name makes this a bit confusing, but
what `tableSession()` does internally is really just create a new `AuthSession`
from a given token/credential that is being passed via a Map.
What we're doing here is passing the `properties()` that contain the
token/credential from the very first `config` response.
The only other element worth discussing here is whether
`commitTransaction()` should be using **catalog vs user credentials**. If it
should be using user credentials, the code would stay as-is.
If it should be using catalog credentials, then we'd have `AuthSession
authSession = tableSession(properties(), session(context));` and then also the
below diff:
```
- if ("v1/config".equals(path)) {
+ if ("v1/config".equals(path) ||
"v1/transactions/commit".equals(path)) {
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
