kou commented on code in PR #39263:
URL: https://github.com/apache/arrow/pull/39263#discussion_r1429277257


##########
cpp/src/arrow/filesystem/azurefs.h:
##########
@@ -70,16 +70,23 @@ struct ARROW_EXPORT AzureOptions {
 
   enum class CredentialKind {
     kAnonymous,
+    kTokenCredential,
     kStorageSharedKeyCredential,
   } credential_kind_ = CredentialKind::kAnonymous;
 
   std::shared_ptr<Azure::Storage::StorageSharedKeyCredential>
       storage_shared_key_credential_;
 
+  std::shared_ptr<Azure::Core::Credentials::TokenCredential> token_credential_;
+
  public:
   AzureOptions();
   ~AzureOptions();
 
+  void SetUrlsForAccountName(const std::string& account_name);

Review Comment:
   Do we need to make this `public`?



##########
cpp/src/arrow/filesystem/azurefs_test.cc:
##########
@@ -266,15 +263,12 @@ class AzureHierarchicalNSEnv : public 
AzureEnvImpl<AzureHierarchicalNSEnv> {
   bool WithHierarchicalNamespace() const final { return true; }
 };
 
-// Placeholder tests
-// TODO: GH-18014 Remove once a proper test is added
-TEST(AzureFileSystem, InitializeCredentials) {
-  auto default_credential = 
std::make_shared<Azure::Identity::DefaultAzureCredential>();
-  auto managed_identity_credential =
-      std::make_shared<Azure::Identity::ManagedIdentityCredential>();
-  auto service_principal_credential =
-      std::make_shared<Azure::Identity::ClientSecretCredential>("tenant_id", 
"client_id",
-                                                                
"client_secret");
+TEST(AzureFileSystem, InitialiseFilesystemWithDefaultCredential) {
+  AzureOptions options;
+  options.backend = AzureBackend::kAzurite;  // Irrelevant for this test 
because it
+                                             // doesn't connect to the server.
+  ARROW_EXPECT_OK(options.ConfigureDefaultCredential("dummy-account-name"));
+  EXPECT_OK_AND_ASSIGN(auto default_credential_fs, 
AzureFileSystem::Make(options));

Review Comment:
   It seems that we can use Azurite with the `DefaultAzureCredential`: 
https://learn.microsoft.com/en-us/azure/storage/common/storage-use-azurite?tabs=visual-studio%2Cblob-storage#azure-sdks
   
   Can we do an operation (`CreateDir()`?) and check the result to verify 
whether this filesystem is valid or not?



##########
cpp/src/arrow/filesystem/azurefs_test.cc:
##########
@@ -266,15 +263,12 @@ class AzureHierarchicalNSEnv : public 
AzureEnvImpl<AzureHierarchicalNSEnv> {
   bool WithHierarchicalNamespace() const final { return true; }
 };
 
-// Placeholder tests
-// TODO: GH-18014 Remove once a proper test is added
-TEST(AzureFileSystem, InitializeCredentials) {
-  auto default_credential = 
std::make_shared<Azure::Identity::DefaultAzureCredential>();
-  auto managed_identity_credential =
-      std::make_shared<Azure::Identity::ManagedIdentityCredential>();
-  auto service_principal_credential =
-      std::make_shared<Azure::Identity::ClientSecretCredential>("tenant_id", 
"client_id",
-                                                                
"client_secret");
+TEST(AzureFileSystem, InitialiseFilesystemWithDefaultCredential) {

Review Comment:
   ```suggestion
   TEST(AzureFileSystem, InitializeFilesystemWithDefaultCredential) {
   ```



-- 
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]

Reply via email to