From de0a869193322e7f0b2c05bf2c28a82fc454abaa Mon Sep 17 00:00:00 2001 From: Thomas Newton Date: Fri, 22 Dec 2023 19:27:26 +0000 Subject: [PATCH] Move account_name to AzureOptions constructor --- cpp/src/arrow/filesystem/azurefs.cc | 25 +++++++------------- cpp/src/arrow/filesystem/azurefs.h | 15 +++++------- cpp/src/arrow/filesystem/azurefs_test.cc | 30 +++++++++++------------- 3 files changed, 29 insertions(+), 41 deletions(-) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index ff58d253b0030..dce2773c53f1f 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -47,7 +47,9 @@ using HNSSupport = internal::HierarchicalNamespaceSupport; // ----------------------------------------------------------------------- // AzureOptions Implementation -AzureOptions::AzureOptions() = default; +// AzureOptions::AzureOptions(const std::string& account_name) = default; +AzureOptions::AzureOptions(const std::string& account_name) + : account_name_(account_name) {} AzureOptions::~AzureOptions() = default; @@ -104,45 +106,36 @@ std::string AzureOptions::AccountDfsUrl(const std::string& account_name) const { return BuildBaseUrl(dfs_storage_scheme, dfs_storage_authority, account_name); } -Status AzureOptions::ConfigureAccountKeyCredential(const std::string& account_name, - const std::string& account_key) { +Status AzureOptions::ConfigureAccountKeyCredential(const std::string& account_key) { credential_kind_ = CredentialKind::kStorageSharedKeyCredential; - account_name_ = account_name; storage_shared_key_credential_ = - std::make_shared(account_name, account_key); + std::make_shared(account_name_, account_key); return Status::OK(); } -Status AzureOptions::ConfigureClientSecretCredential(const std::string& account_name, - const std::string& tenant_id, +Status AzureOptions::ConfigureClientSecretCredential(const std::string& tenant_id, const std::string& client_id, const std::string& client_secret) { - account_name_ = account_name; credential_kind_ = CredentialKind::kTokenCredential; token_credential_ = std::make_shared( tenant_id, client_id, client_secret); return Status::OK(); } -Status AzureOptions::ConfigureDefaultCredential(const std::string& account_name) { - account_name_ = account_name; +Status AzureOptions::ConfigureDefaultCredential() { credential_kind_ = CredentialKind::kTokenCredential; token_credential_ = std::make_shared(); return Status::OK(); } -Status AzureOptions::ConfigureManagedIdentityCredential(const std::string& account_name, - const std::string& client_id) { - account_name_ = account_name; +Status AzureOptions::ConfigureManagedIdentityCredential(const std::string& client_id) { credential_kind_ = CredentialKind::kTokenCredential; token_credential_ = std::make_shared(client_id); return Status::OK(); } -Status AzureOptions::ConfigureWorkloadIdentityCredential( - const std::string& account_name) { - account_name_ = account_name; +Status AzureOptions::ConfigureWorkloadIdentityCredential() { credential_kind_ = CredentialKind::kTokenCredential; token_credential_ = std::make_shared(); return Status::OK(); diff --git a/cpp/src/arrow/filesystem/azurefs.h b/cpp/src/arrow/filesystem/azurefs.h index 99f06e3421f6d..2e635984d4e1d 100644 --- a/cpp/src/arrow/filesystem/azurefs.h +++ b/cpp/src/arrow/filesystem/azurefs.h @@ -100,21 +100,18 @@ struct ARROW_EXPORT AzureOptions { storage_shared_key_credential_; public: - AzureOptions(); + AzureOptions(const std::string& account_name); ~AzureOptions(); - Status ConfigureDefaultCredential(const std::string& account_name); + Status ConfigureDefaultCredential(); - Status ConfigureManagedIdentityCredential(const std::string& account_name, - const std::string& client_id = std::string()); + Status ConfigureManagedIdentityCredential(const std::string& client_id = std::string()); - Status ConfigureWorkloadIdentityCredential(const std::string& account_name); + Status ConfigureWorkloadIdentityCredential(); - Status ConfigureAccountKeyCredential(const std::string& account_name, - const std::string& account_key); + Status ConfigureAccountKeyCredential(const std::string& account_key); - Status ConfigureClientSecretCredential(const std::string& account_name, - const std::string& tenant_id, + Status ConfigureClientSecretCredential(const std::string& tenant_id, const std::string& client_id, const std::string& client_secret); diff --git a/cpp/src/arrow/filesystem/azurefs_test.cc b/cpp/src/arrow/filesystem/azurefs_test.cc index 5d1b2e5ece26c..ab23b0b3fc70f 100644 --- a/cpp/src/arrow/filesystem/azurefs_test.cc +++ b/cpp/src/arrow/filesystem/azurefs_test.cc @@ -272,36 +272,35 @@ class AzureHierarchicalNSEnv : public AzureEnvImpl { }; TEST(AzureFileSystem, InitializeFilesystemWithClientSecretCredential) { - AzureOptions options; - ARROW_EXPECT_OK(options.ConfigureClientSecretCredential( - "dummy-account-name", "tenant_id", "client_id", "client_secret")); + AzureOptions options("dummy-account-name"); + ARROW_EXPECT_OK( + options.ConfigureClientSecretCredential("tenant_id", "client_id", "client_secret")); EXPECT_OK_AND_ASSIGN(auto fs, AzureFileSystem::Make(options)); } TEST(AzureFileSystem, InitializeFilesystemWithDefaultCredential) { - AzureOptions options; - ARROW_EXPECT_OK(options.ConfigureDefaultCredential("dummy-account-name")); + AzureOptions options("dummy-account-name"); + ARROW_EXPECT_OK(options.ConfigureDefaultCredential()); EXPECT_OK_AND_ASSIGN(auto fs, AzureFileSystem::Make(options)); } TEST(AzureFileSystem, InitializeFilesystemWithManagedIdentityCredential) { - AzureOptions options; - ARROW_EXPECT_OK(options.ConfigureManagedIdentityCredential("dummy-account-name")); + AzureOptions options("dummy-account-name"); + ARROW_EXPECT_OK(options.ConfigureManagedIdentityCredential()); EXPECT_OK_AND_ASSIGN(auto fs, AzureFileSystem::Make(options)); - ARROW_EXPECT_OK(options.ConfigureManagedIdentityCredential("dummy-account-name", - "specific-client-id")); + ARROW_EXPECT_OK(options.ConfigureManagedIdentityCredential("specific-client-id")); EXPECT_OK_AND_ASSIGN(fs, AzureFileSystem::Make(options)); } TEST(AzureFileSystem, InitializeFilesystemWithWorkloadIdentityCredential) { - AzureOptions options; - ARROW_EXPECT_OK(options.ConfigureWorkloadIdentityCredential("dummy-account-name")); + AzureOptions options("dummy-account-name"); + ARROW_EXPECT_OK(options.ConfigureWorkloadIdentityCredential()); EXPECT_OK_AND_ASSIGN(auto fs, AzureFileSystem::Make(options)); } TEST(AzureFileSystem, OptionsCompare) { - AzureOptions options; + AzureOptions options("account-name"); EXPECT_TRUE(options.Equals(options)); } @@ -375,7 +374,7 @@ class TestAzureFileSystem : public ::testing::Test { std::unique_ptr datalake_service_client_; public: - TestAzureFileSystem() : rng_(std::random_device()()) {} + TestAzureFileSystem() : rng_(std::random_device()()), options_("temp") {} virtual Result GetAzureEnv() const = 0; virtual HNSSupport CachedHNSSupport(const BaseAzureEnv& env) const = 0; @@ -392,7 +391,7 @@ class TestAzureFileSystem : public ::testing::Test { } static Result MakeOptions(BaseAzureEnv* env) { - AzureOptions options; + AzureOptions options(env->account_name()); switch (env->backend()) { case AzureBackend::kAzurite: options.blob_storage_authority = "127.0.0.1:10000"; @@ -404,8 +403,7 @@ class TestAzureFileSystem : public ::testing::Test { // Use the default values break; } - ARROW_EXPECT_OK( - options.ConfigureAccountKeyCredential(env->account_name(), env->account_key())); + ARROW_EXPECT_OK(options.ConfigureAccountKeyCredential(env->account_key())); return options; }