Skip to content

Commit

Permalink
C.41 compliance: GCS factory constructor. (#5393)
Browse files Browse the repository at this point in the history
Remove `GCS::init` in favor of a C.41-compliant factory constructor. Use
`struct GCSParameters` to store GCS-specific configuration parameters.
Remove `Status_GCSError`, and replace call sites with throws of
`GCSException`.

[sc-60072]

---
TYPE: IMPROVEMENT
DESC: C.41 constructor: `GCS`
  • Loading branch information
bekadavis9 authored Jan 9, 2025
1 parent 3eda3c1 commit c36bce6
Show file tree
Hide file tree
Showing 8 changed files with 286 additions and 256 deletions.
21 changes: 4 additions & 17 deletions test/src/unit-vfs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,6 @@ TEST_CASE(
"[gcs][credentials][impersonation]") {
ThreadPool thread_pool(2);
Config cfg = set_config_params(true);
GCS gcs;
std::string impersonate_service_account, target_service_account;
std::vector<std::string> delegates;

Expand All @@ -847,9 +846,7 @@ TEST_CASE(

require_tiledb_ok(cfg.set(
"vfs.gcs.impersonate_service_account", impersonate_service_account));

require_tiledb_ok(gcs.init(cfg, &thread_pool));

GCS gcs(&thread_pool, cfg);
auto credentials = gcs.make_credentials({});

// We are using an internal class only for inspection purposes.
Expand All @@ -869,16 +866,13 @@ TEST_CASE(
"[gcs][credentials][service-account]") {
ThreadPool thread_pool(2);
Config cfg = set_config_params(true);
GCS gcs;
// The content of the credentials does not matter; it does not get parsed
// until it is used in an API request, which we are not doing.
std::string service_account_key = "{\"foo\": \"bar\"}";

require_tiledb_ok(
cfg.set("vfs.gcs.service_account_key", service_account_key));

require_tiledb_ok(gcs.init(cfg, &thread_pool));

GCS gcs(&thread_pool, cfg);
auto credentials = gcs.make_credentials({});

// We are using an internal class only for inspection purposes.
Expand All @@ -895,7 +889,6 @@ TEST_CASE(
"[gcs][credentials][service-account-and-impersonation]") {
ThreadPool thread_pool(2);
Config cfg = set_config_params(true);
GCS gcs;
// The content of the credentials does not matter; it does not get parsed
// until it is used in an API request, which we are not doing.
std::string service_account_key = "{\"foo\": \"bar\"}";
Expand All @@ -905,9 +898,7 @@ TEST_CASE(
cfg.set("vfs.gcs.service_account_key", service_account_key));
require_tiledb_ok(cfg.set(
"vfs.gcs.impersonate_service_account", impersonate_service_account));

require_tiledb_ok(gcs.init(cfg, &thread_pool));

GCS gcs(&thread_pool, cfg);
auto credentials = gcs.make_credentials({});

// We are using an internal class only for inspection purposes.
Expand All @@ -933,17 +924,13 @@ TEST_CASE(
"[gcs][credentials][external-account]") {
ThreadPool thread_pool(2);
Config cfg = set_config_params(true);
GCS gcs;
// The content of the credentials does not matter; it does not get parsed
// until it is used in an API request, which we are not doing.
std::string workload_identity_configuration = "{\"foo\": \"bar\"}";

require_tiledb_ok(cfg.set(
"vfs.gcs.workload_identity_configuration",
workload_identity_configuration));

require_tiledb_ok(gcs.init(cfg, &thread_pool));

GCS gcs(&thread_pool, cfg);
auto credentials = gcs.make_credentials({});

// We are using an internal class only for inspection purposes.
Expand Down
5 changes: 4 additions & 1 deletion tiledb/api/c_api/config/config_api_external.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*
* The MIT License
*
* @copyright Copyright (c) 2023 TileDB, Inc.
* @copyright Copyright (c) 2023-2024 TileDB, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -455,6 +455,9 @@ TILEDB_EXPORT void tiledb_config_free(tiledb_config_t** config) TILEDB_NOEXCEPT;
* The maximum permissible delay between Azure netwwork request retry
* attempts, in milliseconds.
* **Default**: 60000
* - `vfs.gcs.endpoint` <br>
* The GCS endpoint. <br>
* **Default**: ""
* - `vfs.gcs.project_id` <br>
* Set the GCS project ID to create new buckets to. Not required unless you
* are going to use the VFS to create buckets. <br>
Expand Down
4 changes: 0 additions & 4 deletions tiledb/common/exception/status.h
Original file line number Diff line number Diff line change
Expand Up @@ -310,10 +310,6 @@ inline Status Status_UtilsError(const std::string& msg) {
inline Status Status_S3Error(const std::string& msg) {
return {"[TileDB::S3] Error", msg};
}
/** Return a FS_GCS error class Status with a given message **/
inline Status Status_GCSError(const std::string& msg) {
return {"[TileDB::GCS] Error", msg};
}
/** Return a FS_HDFS error class Status with a given message **/
inline Status Status_HDFSError(const std::string& msg) {
return {"[TileDB::HDFS] Error", msg};
Expand Down
7 changes: 4 additions & 3 deletions tiledb/sm/cpp_api/config.h
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
/**
* @file config.h
*
* @author Ravi Gaddipati
*
* @section LICENSE
*
* The MIT License
*
* @copyright Copyright (c) 2017-2023 TileDB, Inc.
* @copyright Copyright (c) 2017-2024 TileDB, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -627,6 +625,9 @@ class Config {
* The maximum permissible delay between Azure netwwork request retry
* attempts, in milliseconds.
* **Default**: 60000
* - `vfs.gcs.endpoint` <br>
* The GCS endpoint. <br>
* **Default**: ""
* - `vfs.gcs.project_id` <br>
* Set the GCS project ID to create new buckets to. Not required unless you
* are going to use the VFS to create buckets. <br>
Expand Down
Loading

0 comments on commit c36bce6

Please sign in to comment.