marin-ma commented on code in PR #9183:
URL: https://github.com/apache/incubator-gluten/pull/9183#discussion_r2026468637
##########
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:
Can we use the registration to add mappings from AWSCredentialsProvider to
the implementation in aws-sdk-cpp?
e.g.
```
registerAWSCredentialsProvider("org.apache.hadoop.fs.s3a.SimpleAWSCredentialsProvider",
[](const S3Config& config) {
GLUTEN_CHECK(
config.accessKey().has_value() &&
!config.accessKey().value().empty(),
"Access key cannot be empty for SimpleAWSCredentialsProvider");
GLUTEN_CHECK(
config.secretKey().has_value() &&
!config.secretKey().value().empty(),
"Secret key cannot be empty for SimpleAWSCredentialsProvider");
return std::make_shared<Aws::Auth::SimpleAWSCredentialsProvider>(
config.accessKey().value(), config.secretKey().value());
});
```
Then we need to support all valid mappings in Gluten. Perhaps we can remove
the configuration from this patch and add it back in another one together with
adding support for all valid mappings. cc: @FelixYBW
--
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]
