Use DefaultAzureCredential by default (#497) by janjagusch · Pull Request #550 · drivendataorg/cloudpathlib

janjagusch · 2026-02-27T21:27:28Z

Summary

When account_url is provided without credential, automatically uses DefaultAzureCredential from azure-identity if installed (falls back to credential=None if not installed)
Adds support for AZURE_STORAGE_ACCOUNT_URL environment variable as a new fallback before raising MissingCredentialsError, mirroring the existing AZURE_STORAGE_CONNECTION_STRING pattern
Fixes minor docstring typos (""AZURE_STORAGE_CONNECTION_STRING" and raised raised)

Closes #497

Test plan

Existing test_azureblobpath_nocreds updated to also clear AZURE_STORAGE_ACCOUNT_URL env var
New test: DefaultAzureCredential used when account_url provided without credential
New test: explicit credential takes precedence over DefaultAzureCredential
New test: fallback to credential=None when azure-identity not installed
New test: AZURE_STORAGE_ACCOUNT_URL env var with .blob. URL
New test: AZURE_STORAGE_ACCOUNT_URL env var with .dfs. URL
New test: MissingCredentialsError still raised with no config
All 13 tests in test_azure_specific.py pass
All 16 tests in test_local.py pass

🤖 Generated with Claude Code

…provided When `account_url` is provided without `credential`, automatically use `DefaultAzureCredential` from `azure-identity` if installed, bringing Azure auth in line with how `GSClient` uses `google.auth.default()`. Also adds support for `AZURE_STORAGE_ACCOUNT_URL` env var as a fallback. Closes drivendataorg#497 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

janjagusch · 2026-02-27T21:43:19Z

@pjbull, hope you don't mind this agent-written PR. I reviewed the changes myself, making sure the diff remains minimal and plausible, and the changes look good to me. :)

janjagusch · 2026-03-09T07:34:23Z

Friendly ping, @pjbull. Let me know what you think of this PR. :)

janjagusch · 2026-03-16T11:27:43Z

@pjbull, friendly reminder. :)

Is there anything I can do to move this PR forward?

pjbull

This is not sufficient review of an AI generated PR. Please don't submit AI PRs that don't follow the conventions of the repository, it creates an unnecessary burden for maintainers. We have extensive contributing docs.

Did you actually test this against a real Azure backend configured to use this path? A mock is not enough proof this works.

Also, please follow existing test patterns, not the MagicMock patterns in this PR. You should add azure mocks if you need them to the mock we already have and actually ensure the AzureClient object gets properties set on it correctly.

pjbull · 2026-03-16T14:38:39Z

cloudpathlib/azure/azblobclient.py

+try:
+    from azure.identity import DefaultAzureCredential
+except ImportError:
+    DefaultAzureCredential = None


Please follow existing convention and add to import block of azure dependencies above.

The azure.identity import was intentionally left out of the big import block above, as azure-identity is an optional azure dependency, meaning you can use the azure blob client without it (you will just not be able to use identity-based authentication).

I've left a comment to make this clearer. Let me know if you prefer a different approach.

Please put it in with the other dependencies, we don't want multiple dependency fallback paths. Either a user has all the backend dependencies, or they don't, which is how you implemented it in the pyproject.toml.

Sounds good to me! I've addressed this comment. Feel free to resolve.

Replace unittest.mock.MagicMock/patch patterns with the project's existing MockBlobServiceClient and MockedDataLakeServiceClient, following the same monkeypatch approach used in conftest.py. Tests now verify actual client properties instead of asserting on mock calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

pyproject.toml

pjbull · 2026-03-16T18:31:35Z

cloudpathlib/azure/azblobclient.py

+try:
+    from azure.identity import DefaultAzureCredential
+except ImportError:
+    DefaultAzureCredential = None


Please put it in with the other dependencies, we don't want multiple dependency fallback paths. Either a user has all the backend dependencies, or they don't, which is how you implemented it in the pyproject.toml.

pyproject.toml

pjbull · 2026-03-16T18:33:29Z

tests/test_azure_specific.py

+def test_fallback_when_azure_identity_not_installed(monkeypatch):
+    """When azure-identity is not installed, credential=None is passed through."""
+    monkeypatch.delenv("AZURE_STORAGE_CONNECTION_STRING", raising=False)
+    monkeypatch.delenv("AZURE_STORAGE_ACCOUNT_URL", raising=False)
+    monkeypatch.setattr(
+        cloudpathlib.azure.azblobclient, "DefaultAzureCredential", None
+    )
+    _mock_azure_clients(monkeypatch)
+
+    client = AzureBlobClient(account_url="https://myaccount.blob.core.windows.net")
+
+    assert client.service_client._credential is None


You can remove per comments above

I've addressed this comment. Feel free to resolve.

Per reviewer feedback, azure-identity is no longer treated as a separate optional dependency. It's imported alongside the other Azure SDK packages and falls under the same dependencies_loaded check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

janjagusch · 2026-03-17T13:42:21Z

.gitignore

+# pixi environments
+.pixi/*
+!.pixi/config.toml
+pixi.toml
+pixi.lock


I set up my development environment with Pixi. Also happy to remove the diff in the .gitignore, if that's not wanted.

janjagusch force-pushed the feat/default-azure-credential branch 2 times, most recently from 03ea50f to e23d7e5 Compare February 27, 2026 21:32

janjagusch force-pushed the feat/default-azure-credential branch from e23d7e5 to fafa0b7 Compare February 27, 2026 21:34

janjagusch marked this pull request as ready for review February 27, 2026 21:38

janjagusch mentioned this pull request Feb 27, 2026

Use DefaultAzureCredential by default for Azure paths #497

Open

pjbull requested changes Mar 16, 2026

View reviewed changes

janjagusch marked this pull request as draft March 16, 2026 15:52

janjagusch and others added 3 commits March 16, 2026 16:58

Add azure identity as optional dependency

88fd3f9

Add comment why azure.identity import is not in big try-except block

15e3a5e

janjagusch commented Mar 16, 2026

View reviewed changes

pyproject.toml Show resolved Hide resolved

pjbull requested changes Mar 16, 2026

View reviewed changes

janjagusch and others added 3 commits March 17, 2026 08:50

Ignore pixi config and lock file

741dd25

Add check_access test for azure rigs with default azure credentials

c320d95

janjagusch commented Mar 17, 2026

View reviewed changes

Conversation

janjagusch commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

janjagusch commented Feb 27, 2026

Uh oh!

janjagusch commented Mar 9, 2026

Uh oh!

janjagusch commented Mar 16, 2026

Uh oh!

pjbull left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

janjagusch commented Feb 27, 2026 •

edited

Loading