From d38ec718f880ffc84ed9ff3074c381a94f75a54b Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Mon, 23 Sep 2024 14:50:01 +0300 Subject: [PATCH] Let the AWS SDK determine the default region. --- test/src/unit-capi-config.cc | 19 +++++++++---------- test/src/unit-s3.cc | 3 +-- tiledb/api/c_api/config/config_api_external.h | 5 ++++- tiledb/sm/config/config.cc | 2 +- tiledb/sm/cpp_api/config.h | 7 +++++-- 5 files changed, 20 insertions(+), 16 deletions(-) diff --git a/test/src/unit-capi-config.cc b/test/src/unit-capi-config.cc index 1f539e7a828..d27cb5e76ee 100644 --- a/test/src/unit-capi-config.cc +++ b/test/src/unit-capi-config.cc @@ -337,7 +337,6 @@ void check_save_to_file() { ss << "vfs.s3.object_canned_acl NOT_SET\n"; ss << "vfs.s3.proxy_port 0\n"; ss << "vfs.s3.proxy_scheme http\n"; - ss << "vfs.s3.region us-east-1\n"; ss << "vfs.s3.request_timeout_ms 3000\n"; ss << "vfs.s3.requester_pays false\n"; ss << "vfs.s3.scheme https\n"; @@ -465,22 +464,22 @@ TEST_CASE("C API: Test config", "[capi][config]") { CHECK(!strcmp(value, "5368709120")); // Set valid, defaulting parameter - rc = tiledb_config_set(config, "vfs.s3.region", "pluto", &error); + rc = tiledb_config_set(config, "sm.consolidation.mode", "commits", &error); CHECK(rc == TILEDB_OK); CHECK(error == nullptr); - rc = tiledb_config_get(config, "vfs.s3.region", &value, &error); + rc = tiledb_config_get(config, "sm.consolidation.mode", &value, &error); CHECK(rc == TILEDB_OK); CHECK(error == nullptr); - CHECK(!strcmp(value, "pluto")); + CHECK(!strcmp(value, "commits")); // Unset valid, defaulting parameter - rc = tiledb_config_unset(config, "vfs.s3.region", &error); + rc = tiledb_config_unset(config, "sm.consolidation.mode", &error); CHECK(rc == TILEDB_OK); CHECK(error == nullptr); - rc = tiledb_config_get(config, "vfs.s3.region", &value, &error); + rc = tiledb_config_get(config, "sm.consolidation.mode", &value, &error); CHECK(rc == TILEDB_OK); CHECK(error == nullptr); - CHECK(!strcmp(value, "us-east-1")); + CHECK(!strcmp(value, "fragments")); // Set valid, non-defaulting parameter rc = tiledb_config_set(config, "foo", "123", &error); @@ -723,7 +722,7 @@ TEST_CASE("C API: Test config iter", "[capi][config]") { all_param_values["vfs.file.posix_file_permissions"] = "644"; all_param_values["vfs.file.posix_directory_permissions"] = "755"; all_param_values["vfs.s3.scheme"] = "https"; - all_param_values["vfs.s3.region"] = "us-east-1"; + all_param_values["vfs.s3.region"] = ""; all_param_values["vfs.s3.aws_access_key_id"] = ""; all_param_values["vfs.s3.aws_secret_access_key"] = ""; all_param_values["vfs.s3.aws_session_token"] = ""; @@ -798,7 +797,7 @@ TEST_CASE("C API: Test config iter", "[capi][config]") { vfs_param_values["file.posix_file_permissions"] = "644"; vfs_param_values["file.posix_directory_permissions"] = "755"; vfs_param_values["s3.scheme"] = "https"; - vfs_param_values["s3.region"] = "us-east-1"; + vfs_param_values["s3.region"] = ""; vfs_param_values["s3.aws_access_key_id"] = ""; vfs_param_values["s3.aws_secret_access_key"] = ""; vfs_param_values["s3.aws_session_token"] = ""; @@ -867,7 +866,7 @@ TEST_CASE("C API: Test config iter", "[capi][config]") { std::map s3_param_values; s3_param_values["scheme"] = "https"; - s3_param_values["region"] = "us-east-1"; + s3_param_values["region"] = ""; s3_param_values["aws_access_key_id"] = ""; s3_param_values["aws_secret_access_key"] = ""; s3_param_values["aws_session_token"] = ""; diff --git a/test/src/unit-s3.cc b/test/src/unit-s3.cc index 922507ced52..d7833c03f58 100644 --- a/test/src/unit-s3.cc +++ b/test/src/unit-s3.cc @@ -132,8 +132,7 @@ TEST_CASE_METHOD(S3Fx, "Test S3 multiupload abort path", "[s3]") { } } -TEST_CASE_METHOD( - S3Fx, "Test S3 setting bucket/object canned acls", "[s3][config]") { +TEST_CASE("Test S3 setting bucket/object canned acls", "[s3][config]") { Config config; REQUIRE(config.set("vfs.s3.bucket_canned_acl", "private_").ok()); REQUIRE(config.set("vfs.s3.bucket_canned_acl", "public_read").ok()); diff --git a/tiledb/api/c_api/config/config_api_external.h b/tiledb/api/c_api/config/config_api_external.h index 4a4551bebbb..2e4f3095b7e 100644 --- a/tiledb/api/c_api/config/config_api_external.h +++ b/tiledb/api/c_api/config/config_api_external.h @@ -499,7 +499,10 @@ TILEDB_EXPORT void tiledb_config_free(tiledb_config_t** config) TILEDB_NOEXCEPT; * **Default**: "10737418240" * - `vfs.s3.region`
* The S3 region, if S3 is enabled.
- * **Default**: us-east-1 + * If empty, the region will be determined by the AWS SDK using sources such + * as environment variables, profile configuration, or instance metadata. + *
+ * **Default**: "" * - `vfs.s3.aws_access_key_id`
* Set the AWS_ACCESS_KEY_ID
* **Default**: "" diff --git a/tiledb/sm/config/config.cc b/tiledb/sm/config/config.cc index e4cf8bb667c..a258710da6a 100644 --- a/tiledb/sm/config/config.cc +++ b/tiledb/sm/config/config.cc @@ -199,7 +199,7 @@ const std::string Config::VFS_GCS_MULTI_PART_SIZE = "5242880"; const std::string Config::VFS_GCS_USE_MULTI_PART_UPLOAD = "true"; const std::string Config::VFS_GCS_REQUEST_TIMEOUT_MS = "3000"; const std::string Config::VFS_GCS_MAX_DIRECT_UPLOAD_SIZE = "10737418240"; -const std::string Config::VFS_S3_REGION = "us-east-1"; +const std::string Config::VFS_S3_REGION = ""; const std::string Config::VFS_S3_AWS_ACCESS_KEY_ID = ""; const std::string Config::VFS_S3_AWS_SECRET_ACCESS_KEY = ""; const std::string Config::VFS_S3_AWS_SESSION_TOKEN = ""; diff --git a/tiledb/sm/cpp_api/config.h b/tiledb/sm/cpp_api/config.h index 08c410658ea..e1af024fc9e 100644 --- a/tiledb/sm/cpp_api/config.h +++ b/tiledb/sm/cpp_api/config.h @@ -158,7 +158,7 @@ struct ConfigProxy { * conf["vfs.s3.region"] = "us-east-1a"; * conf["vfs.s3.use_virtual_addressing"] = "true"; * Context ctx(conf); - * // array/kv operations with ctx + * // array operations with ctx * @endcode * */ class Config { @@ -671,7 +671,10 @@ class Config { * **Default**: "10737418240" * - `vfs.s3.region`
* The S3 region, if S3 is enabled.
- * **Default**: us-east-1 + * If empty, the region will be determined by the AWS SDK using sources + * such as environment variables, profile configuration, or instance + * metadata.
+ * **Default**: "" * - `vfs.s3.aws_access_key_id`
* Set the AWS_ACCESS_KEY_ID
* **Default**: ""