NathanCYee commented on issue #16667:
URL: https://github.com/apache/iceberg/issues/16667#issuecomment-4859578509

   Hi @GabrielBBaldez apologies for the lack of communication,
   
   Thank you for your efforts in the implementation. I created a [PR to your 
branch](https://github.com/GabrielBBaldez/iceberg/pull/2) to merge some of my 
unification changes and doc updates. 
   
   I made a change to the behavior of `clientAssumeRoleSessionName`. Before, if 
it was not provided it would be set to null, the client factory would generate 
a random one on class initialization using `genSessionName` with the format 
`iceberg-aws-UUID`. The REST implementation would **generate a new random 
session name on every function call which was differing behavior from the 
client factory and meant the session id would not be stable across the entire 
interaction**. I updated it to getOrDefault to default to `iceberg-aws-UUID` if 
it was not provided during the `AwsProperties` initialization; this would mean 
that the **generated session name would persist for the lifetime of 
`AwsProperties` instead of the lifetime of the client factory**, @CTTY is this 
change in behavior appropriate?
   
   There was overlap between the existing code and your contributions in the 
assume role provider creation, so instead of creating a shared utility class I 
updated your new function to be protected and made the client factory use your 
implementation of `assumeRoleCredentialsProvider`. The other functions (`sts`, 
`createAssumeRoleRequest`) were helpers that were only used by 
`assumeRoleCredentialsProvider` so they were removed as well.
   
   Please take a look at the 
[PR](https://github.com/GabrielBBaldez/iceberg/pull/2) and let me know if it 
could be merged into your branch and picked up on the combined PR to iceberg 
main.


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

Reply via email to