marin-ma commented on code in PR #9183:
URL: https://github.com/apache/incubator-gluten/pull/9183#discussion_r2024584340


##########
cpp/velox/utils/ConfigExtractor.cc:
##########
@@ -72,6 +83,8 @@ std::shared_ptr<facebook::velox::config::ConfigBase> 
getHiveConfig(
       {S3Config::Keys::kUseInstanceCredentials, 
std::make_pair("instance.credentials", "false")},
       {S3Config::Keys::kIamRole, std::make_pair("iam.role", std::nullopt)},
       {S3Config::Keys::kIamRoleSessionName, 
std::make_pair("iam.role.session.name", "gluten-session")},
+      {S3Config::Keys::kEndpointRegion, std::make_pair("endpoint.region", 
std::nullopt)},
+      {S3Config::Keys::kCredentialsProvider, 
std::make_pair("aws.credentials.provider", std::nullopt)},

Review Comment:
   I wonder if it's safe to add this mapping in Gluten. If this configuration 
is not set, Velox uses [its own 
logic](https://github.com/facebookincubator/velox/blob/3a0a69178f1f20f890c2f3800ef9be0c322679b3/velox/connectors/hive/storage_adapters/s3fs/S3FileSystem.cpp#L729-L771)
 to create credential providers that are supported in aws-sdk-cpp. 
   
   Say if a workload is configured with 
`spark.hadoop.fs.s3a.aws.credentials.provider=org.apache.hadoop.fs.s3a.SimpleAWSCredentialsProvider`
 and access/secret keys are set, it should go to [(code 
link)](https://github.com/facebookincubator/velox/blob/3a0a69178f1f20f890c2f3800ef9be0c322679b3/velox/connectors/hive/storage_adapters/s3fs/S3FileSystem.cpp#L756-L759).
 But adding this mapping may break that part of logic and go to [(code 
link)](https://github.com/facebookincubator/velox/blob/3a0a69178f1f20f890c2f3800ef9be0c322679b3/velox/connectors/hive/storage_adapters/s3fs/S3FileSystem.cpp#L731-L736).
 Then it will get the error 'CredentialsProviderFactory for 
'org.apache.hadoop.fs.s3a.SimpleAWSCredentialsProvider' not registered'
   
   @dcoliversun @zhouyuan Do you have any thoughts or suggestions?



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