Skip to content

Commit da260d3

Browse files
Shira SassoonCopilot
andcommitted
refactor: move azure-identity import to module level
azure-identity is a required dependency (pyproject.toml), so the try/except ImportError guard is unnecessary. Move import to module level and remove the missing-package error message and test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent cb334eb commit da260d3

3 files changed

Lines changed: 16 additions & 51 deletions

File tree

src/fabric_cli/core/fab_auth.py

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from binascii import hexlify
1010
from typing import Any, NamedTuple, Optional
1111

12+
from azure.identity import AzureCliCredential, CredentialUnavailableError
1213
import jwt
1314
import msal
1415
import requests
@@ -484,14 +485,6 @@ def _get_azure_cli_tenant(self, force_refresh: bool = False) -> Optional[str]:
484485

485486
def _acquire_token_from_azure_cli(self, scope: list[str]) -> dict:
486487
"""Acquire a token using Azure CLI's AzureCliCredential."""
487-
try:
488-
from azure.identity import AzureCliCredential, CredentialUnavailableError
489-
except ImportError:
490-
raise FabricCLIError(
491-
ErrorMessages.Auth.azure_cli_missing_azure_identity(),
492-
status_code=con.ERROR_AUTHENTICATION_FAILED,
493-
)
494-
495488
# Tenant drift check: compare stored tenant against current az session
496489
stored_tenant = self.get_tenant_id()
497490
if stored_tenant:

src/fabric_cli/errors/auth.py

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -120,13 +120,6 @@ def cert_read_failed(error: str) -> str:
120120
def only_supported_with_user_authentication() -> str:
121121
return "This operation is only supported with user authentication"
122122

123-
@staticmethod
124-
def azure_cli_missing_azure_identity() -> str:
125-
return (
126-
"Azure CLI auth requires the 'azure-identity' package. "
127-
"Install it with: pip install azure-identity"
128-
)
129-
130123
@staticmethod
131124
def azure_cli_tenant_mismatch(stored_tenant: str, current_tenant: str) -> str:
132125
return (

tests/test_core/test_fab_auth_azure_cli.py

Lines changed: 15 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ def test_set_azure_cli_explicit_tenant_overrides_auto(
119119
class TestAzureCliTokenAcquisition:
120120
"""Test token acquisition via AzureCliCredential."""
121121

122-
@patch("azure.identity.AzureCliCredential")
122+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
123123
def test_acquire_token_dispatches_to_azure_cli(
124124
self, mock_credential_class, temp_dir_fixture
125125
):
@@ -144,7 +144,7 @@ def test_acquire_token_dispatches_to_azure_cli(
144144
"https://api.fabric.microsoft.com/.default"
145145
)
146146

147-
@patch("azure.identity.AzureCliCredential")
147+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
148148
def test_acquire_token_from_azure_cli_success(
149149
self, mock_credential_class, temp_dir_fixture
150150
):
@@ -168,7 +168,7 @@ def test_acquire_token_from_azure_cli_success(
168168
"https://api.fabric.microsoft.com/.default"
169169
)
170170

171-
@patch("azure.identity.AzureCliCredential")
171+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
172172
def test_acquire_token_from_azure_cli_with_tenant(
173173
self, mock_credential_class, temp_dir_fixture
174174
):
@@ -190,12 +190,12 @@ def test_acquire_token_from_azure_cli_with_tenant(
190190

191191
mock_credential_class.assert_called_once_with(tenant_id="my-tenant-id")
192192

193-
@patch("azure.identity.AzureCliCredential")
193+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
194194
def test_acquire_token_from_azure_cli_credential_unavailable(
195195
self, mock_credential_class, temp_dir_fixture
196196
):
197197
"""Should raise FabricCLIError when Azure CLI is not logged in."""
198-
from azure.identity import CredentialUnavailableError
198+
from fabric_cli.core.fab_auth import CredentialUnavailableError
199199

200200
mock_credential = MagicMock()
201201
mock_credential.get_token.side_effect = CredentialUnavailableError(
@@ -212,28 +212,7 @@ def test_acquire_token_from_azure_cli_credential_unavailable(
212212

213213
assert "not installed or not logged in" in str(exc_info.value)
214214

215-
@patch(
216-
"azure.identity.AzureCliCredential",
217-
side_effect=ImportError("No module named 'azure.identity'"),
218-
)
219-
def test_acquire_token_from_azure_cli_missing_package(
220-
self, mock_import, temp_dir_fixture
221-
):
222-
"""Should raise FabricCLIError when azure-identity is not installed."""
223-
auth = FabAuth()
224-
auth.set_access_mode("azure_cli")
225-
226-
# Need to actually test the import failure path
227-
with patch.dict("sys.modules", {"azure.identity": None}):
228-
with patch(
229-
"builtins.__import__", side_effect=ImportError("no azure.identity")
230-
):
231-
with pytest.raises(FabricCLIError) as exc_info:
232-
auth._acquire_token_from_azure_cli(con.SCOPE_FABRIC_DEFAULT)
233-
234-
assert "azure-identity" in str(exc_info.value)
235-
236-
@patch("azure.identity.AzureCliCredential")
215+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
237216
def test_sdk_exception_surfaces_message(
238217
self, mock_credential_class, temp_dir_fixture
239218
):
@@ -252,7 +231,7 @@ def test_sdk_exception_surfaces_message(
252231

253232
assert "Tenant not found" in str(exc_info.value)
254233

255-
@patch("azure.identity.AzureCliCredential")
234+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
256235
def test_unknown_exception_returns_safe_message(
257236
self, mock_credential_class, temp_dir_fixture
258237
):
@@ -277,7 +256,7 @@ def test_unknown_exception_returns_safe_message(
277256
class TestAzureCliTenantDrift:
278257
"""Test tenant drift detection during token acquisition."""
279258

280-
@patch("azure.identity.AzureCliCredential")
259+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
281260
@patch("subprocess.run")
282261
def test_tenant_drift_blocks_token_acquisition(
283262
self, mock_run, mock_credential_class, temp_dir_fixture
@@ -298,7 +277,7 @@ def test_tenant_drift_blocks_token_acquisition(
298277
assert "Tenant mismatch" in str(exc_info.value)
299278
assert "original-tenant" in str(exc_info.value)
300279

301-
@patch("azure.identity.AzureCliCredential")
280+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
302281
@patch("subprocess.run")
303282
def test_tenant_match_allows_token_acquisition(
304283
self, mock_run, mock_credential_class, temp_dir_fixture
@@ -327,7 +306,7 @@ def test_tenant_match_allows_token_acquisition(
327306
class TestAzureCliTokenCache:
328307
"""Test in-memory token caching for Azure CLI tokens."""
329308

330-
@patch("azure.identity.AzureCliCredential")
309+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
331310
def test_cached_token_avoids_subprocess(
332311
self, mock_credential_class, temp_dir_fixture
333312
):
@@ -352,7 +331,7 @@ def test_cached_token_avoids_subprocess(
352331
# get_token should only be called once (second call uses cache)
353332
mock_credential.get_token.assert_called_once()
354333

355-
@patch("azure.identity.AzureCliCredential")
334+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
356335
def test_expired_cache_triggers_refresh(
357336
self, mock_credential_class, temp_dir_fixture
358337
):
@@ -378,7 +357,7 @@ def test_expired_cache_triggers_refresh(
378357
assert result["access_token"] == "fresh-token"
379358
mock_credential.get_token.assert_called_once()
380359

381-
@patch("azure.identity.AzureCliCredential")
360+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
382361
def test_different_scopes_cached_separately(
383362
self, mock_credential_class, temp_dir_fixture
384363
):
@@ -412,7 +391,7 @@ def make_token(*args):
412391
class TestAzureCliScopeHandling:
413392
"""Test that different scopes are correctly passed to Azure CLI."""
414393

415-
@patch("azure.identity.AzureCliCredential")
394+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
416395
def test_onelake_scope(self, mock_credential_class, temp_dir_fixture):
417396
"""OneLake scope should be passed correctly."""
418397
mock_token = MagicMock()
@@ -433,7 +412,7 @@ def test_onelake_scope(self, mock_credential_class, temp_dir_fixture):
433412
"https://storage.azure.com/.default"
434413
)
435414

436-
@patch("azure.identity.AzureCliCredential")
415+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
437416
def test_azure_management_scope(self, mock_credential_class, temp_dir_fixture):
438417
"""Azure management scope should be passed correctly."""
439418
mock_token = MagicMock()
@@ -495,7 +474,7 @@ def test_login_forces_fresh_tenant_query(self, mock_run, temp_dir_fixture):
495474
auth.set_azure_cli() # Should force refresh, get tenant-B
496475
assert auth.get_tenant_id() == "tenant-B"
497476

498-
@patch("azure.identity.AzureCliCredential")
477+
@patch("fabric_cli.core.fab_auth.AzureCliCredential")
499478
@patch("subprocess.run")
500479
def test_single_subprocess_across_three_login_scopes(
501480
self, mock_run, mock_credential_class, temp_dir_fixture

0 commit comments

Comments
 (0)