Samrat002 commented on code in PR #27788:
URL: https://github.com/apache/flink/pull/27788#discussion_r3121277495
##########
flink-filesystems/flink-s3-fs-native/src/main/java/org/apache/flink/fs/s3native/NativeS3FileSystemFactory.java:
##########
@@ -285,13 +301,54 @@ public FileSystem create(URI fsUri) throws IOException {
String region = config.get(REGION);
String endpoint = config.get(ENDPOINT);
boolean pathStyleAccess = config.get(PATH_STYLE_ACCESS);
-
+ String sseType = config.get(SSE_TYPE);
+ String sseKmsKeyId = config.get(SSE_KMS_KEY_ID);
+ String assumeRoleArn = config.get(ASSUME_ROLE_ARN);
+ String assumeRoleExternalId = config.get(ASSUME_ROLE_EXTERNAL_ID);
+ String assumeRoleSessionName = config.get(ASSUME_ROLE_SESSION_NAME);
+ int assumeRoleSessionDuration =
config.get(ASSUME_ROLE_SESSION_DURATION_SECONDS);
+ String credentialsProviderClasses =
config.get(AWS_CREDENTIALS_PROVIDER);
+
+ // Apply bucket-specific overrides
+ String bucketName = fsUri.getHost();
+ if (bucketName == null || bucketName.isEmpty()) {
+ throw new IOException("Invalid S3 URI: missing or empty bucket
name in URI: " + fsUri);
+ }
+ if (bucketConfigProvider != null) {
+ S3BucketConfig overrides =
bucketConfigProvider.getBucketConfig(bucketName);
+ if (overrides != null) {
+ LOG.debug(
+ "Applying bucket-specific configuration for bucket
'{}': {}",
+ bucketName,
+ overrides);
+ accessKey = firstNonNull(overrides.getAccessKey(), accessKey);
+ secretKey = firstNonNull(overrides.getSecretKey(), secretKey);
+ region = firstNonNull(overrides.getRegion(), region);
+ endpoint = firstNonNull(overrides.getEndpoint(), endpoint);
+ sseType = firstNonNull(overrides.getSseType(), sseType);
+ sseKmsKeyId = firstNonNull(overrides.getSseKmsKeyId(),
sseKmsKeyId);
+ assumeRoleArn = firstNonNull(overrides.getAssumeRoleArn(),
assumeRoleArn);
+ assumeRoleExternalId =
+ firstNonNull(overrides.getAssumeRoleExternalId(),
assumeRoleExternalId);
+ assumeRoleSessionName =
+ firstNonNull(overrides.getAssumeRoleSessionName(),
assumeRoleSessionName);
+ credentialsProviderClasses =
+ firstNonNull(
+ overrides.getCredentialsProvider(),
credentialsProviderClasses);
+ if (overrides.getPathStyleAccess() != null) {
+ pathStyleAccess = overrides.getPathStyleAccess();
+ }
+ if (overrides.getAssumeRoleSessionDurationSeconds() != null) {
+ assumeRoleSessionDuration =
overrides.getAssumeRoleSessionDurationSeconds();
+ }
Review Comment:
Java language constraint.
these two are primitives (boolean and int), not String.
firstNonNull(overrides.getPathStyleAccess(), pathStyleAccess) silently breaks
because the primitive auto-boxes to a non-null Boolean.FALSE/Integer(3600),
making firstNonNull always return the base regardless of the override value.
The explicit null-check on the @Nullable Boolean/@Nullable Integer getter is
the only correct way to conditionally override a primitive.
Alternatively, I could change these locals to boxed types (Boolean/Integer)
for uniformity, but they're guaranteed non-null from config.get() (both
ConfigOptions have defaults), so boxing them just to use firstNonNull feels
like the bigger code smell. Happy to go either way.
--
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]
