pitrou commented on a change in pull request #8197:
URL: https://github.com/apache/arrow/pull/8197#discussion_r490889667
##########
File path: r/src/filesystem.cpp
##########
@@ -255,10 +255,71 @@ void fs___CopyFiles(const
std::shared_ptr<fs::FileSystem>& src_fs,
// [[s3::export]]
void fs___EnsureS3Initialized() { StopIfNotOk(fs::EnsureS3Initialized()); }
+std::string get_optional_string(SEXP x) {
+ std::string out = "";
+ if (!Rf_isNull(x)) {
+ out = cpp11::as_cpp<std::string>(x);
+ }
+ return out;
+}
+
// [[s3::export]]
-std::shared_ptr<fs::S3FileSystem> fs___S3FileSystem__create() {
- auto opts = fs::S3Options::Defaults();
- return ValueOrStop(fs::S3FileSystem::Make(opts));
+std::shared_ptr<fs::S3FileSystem> fs___S3FileSystem__create(bool anonymous,
+ cpp11::list
options) {
+ fs::S3Options s3_opts;
+ // Handle auth (anonymous, keys, default)
+ // (validation/internal coherence handled in R)
+ if (anonymous) {
+ s3_opts = fs::S3Options::Anonymous();
+ } else {
+ SEXP access_key = options["access_key"];
+ SEXP secret_key = options["secret_key"];
+ if (!Rf_isNull(access_key) && !Rf_isNull(secret_key)) {
+ s3_opts = fs::S3Options::FromAccessKey(
+ cpp11::as_cpp<std::string>(access_key),
cpp11::as_cpp<std::string>(secret_key),
+ get_optional_string(options["session_token"]));
+ } else {
+ SEXP role_arn = options["role_arn"];
+ if (!Rf_isNull(role_arn)) {
+ int load_frequency = 900;
+ SEXP freq = options["load_frequency"];
+ if (!Rf_isNull(freq)) {
+ load_frequency = cpp11::as_cpp<int>(freq);
+ }
+ s3_opts =
+ fs::S3Options::FromAssumeRole(cpp11::as_cpp<std::string>(role_arn),
+
get_optional_string(options["session_name"]),
+
get_optional_string(options["external_id"]));
+ } else {
+ s3_opts = fs::S3Options::Defaults();
+ }
+ }
+ }
+
+ // Now handle the rest of the options
+ /// AWS region to connect to (default "us-east-1")
+ SEXP region = options["region"];
+ if (!Rf_isNull(region)) {
+ s3_opts.region = cpp11::as_cpp<std::string>(region);
+ }
+ /// If non-empty, override region with a connect string such as
"localhost:9000"
+ SEXP endpoint_override = options["endpoint_override"];
+ if (!Rf_isNull(endpoint_override)) {
+ s3_opts.endpoint_override = cpp11::as_cpp<std::string>(endpoint_override);
Review comment:
Why not reuse `get_optional_string` here and below?
##########
File path: r/src/filesystem.cpp
##########
@@ -255,10 +255,71 @@ void fs___CopyFiles(const
std::shared_ptr<fs::FileSystem>& src_fs,
// [[s3::export]]
void fs___EnsureS3Initialized() { StopIfNotOk(fs::EnsureS3Initialized()); }
+std::string get_optional_string(SEXP x) {
+ std::string out = "";
+ if (!Rf_isNull(x)) {
+ out = cpp11::as_cpp<std::string>(x);
+ }
+ return out;
+}
+
// [[s3::export]]
-std::shared_ptr<fs::S3FileSystem> fs___S3FileSystem__create() {
- auto opts = fs::S3Options::Defaults();
- return ValueOrStop(fs::S3FileSystem::Make(opts));
+std::shared_ptr<fs::S3FileSystem> fs___S3FileSystem__create(bool anonymous,
+ cpp11::list
options) {
+ fs::S3Options s3_opts;
+ // Handle auth (anonymous, keys, default)
+ // (validation/internal coherence handled in R)
+ if (anonymous) {
+ s3_opts = fs::S3Options::Anonymous();
+ } else {
+ SEXP access_key = options["access_key"];
+ SEXP secret_key = options["secret_key"];
+ if (!Rf_isNull(access_key) && !Rf_isNull(secret_key)) {
+ s3_opts = fs::S3Options::FromAccessKey(
+ cpp11::as_cpp<std::string>(access_key),
cpp11::as_cpp<std::string>(secret_key),
+ get_optional_string(options["session_token"]));
+ } else {
+ SEXP role_arn = options["role_arn"];
+ if (!Rf_isNull(role_arn)) {
+ int load_frequency = 900;
+ SEXP freq = options["load_frequency"];
+ if (!Rf_isNull(freq)) {
+ load_frequency = cpp11::as_cpp<int>(freq);
+ }
+ s3_opts =
+ fs::S3Options::FromAssumeRole(cpp11::as_cpp<std::string>(role_arn),
+
get_optional_string(options["session_name"]),
+
get_optional_string(options["external_id"]));
+ } else {
+ s3_opts = fs::S3Options::Defaults();
+ }
+ }
+ }
+
+ // Now handle the rest of the options
+ /// AWS region to connect to (default "us-east-1")
+ SEXP region = options["region"];
+ if (!Rf_isNull(region)) {
+ s3_opts.region = cpp11::as_cpp<std::string>(region);
+ }
+ /// If non-empty, override region with a connect string such as
"localhost:9000"
+ SEXP endpoint_override = options["endpoint_override"];
+ if (!Rf_isNull(endpoint_override)) {
+ s3_opts.endpoint_override = cpp11::as_cpp<std::string>(endpoint_override);
Review comment:
Why not reuse `get_optional_string` here?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
