JonasJ-ap commented on code in PR #5684:
URL: https://github.com/apache/iceberg/pull/5684#discussion_r963067612
##########
aws/src/main/java/org/apache/iceberg/aws/AssumeRoleAwsClientFactory.java:
##########
@@ -91,37 +93,26 @@ public void initialize(Map<String, String> properties) {
Preconditions.checkNotNull(
region, "Cannot initialize AssumeRoleClientConfigFactory with null
region");
- this.s3Endpoint = properties.get(AwsProperties.S3FILEIO_ENDPOINT);
this.tags = toTags(properties);
- this.s3UseArnRegionEnabled =
- PropertyUtil.propertyAsBoolean(
- properties,
- AwsProperties.S3_USE_ARN_REGION_ENABLED,
- AwsProperties.S3_USE_ARN_REGION_ENABLED_DEFAULT);
- this.dynamoDbEndpoint = properties.get(AwsProperties.DYNAMODB_ENDPOINT);
- this.httpClientType =
- PropertyUtil.propertyAsString(
- properties, AwsProperties.HTTP_CLIENT_TYPE,
AwsProperties.HTTP_CLIENT_TYPE_DEFAULT);
- }
-
- protected <T extends AwsClientBuilder & AwsSyncClientBuilder> T configure(T
clientBuilder) {
- AssumeRoleRequest request =
+ this.assumeRoleRequest =
AssumeRoleRequest.builder()
.roleArn(roleArn)
.roleSessionName(genSessionName())
Review Comment:
I did some search and here is what I found:
The `roleSessionName` is used to differentiate between different programs
which assume the same role. It seems that a single program can make requests
with the same `roleSessionName` to establish different types of connections.
Hence, I think we can use the same session name here.
It seems that we can instead treat this change as a new feature and add some
functionality such as optional user-defined `roleSessionName`. Hence, I plan to
leave this unchanged in this PR and do it in another.
The doc related to `roleSessionName`:
https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/STS.html#assumeRole-property
--
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]
