Skip to content

Commit

Permalink
Change name to process_timeout
Browse files Browse the repository at this point in the history
Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
  • Loading branch information
pvaneck committed Jan 12, 2023
1 parent 2131348 commit e5c4ed5
Show file tree
Hide file tree
Showing 11 changed files with 40 additions and 43 deletions.
5 changes: 1 addition & 4 deletions sdk/identity/azure-identity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@

### Features Added

- Credentials that are implemented via launching a subprocess to acquire tokens now have configurable timeouts. This addresses scenarios where these proceses can take longer than the current default timeout values. The affected credentials and their associated options are:
- `AzureCliCredential` and `cli_process_timeout`
- `AzurePowerShellCredential` and `powershell_process_timeout`
- `DefaultAzureCredential` and `developer_credential_timeout` (Note: this option applies to both `AzureCliCredential` and `AzurePowerShellCredential` in the authentication chain.) ([#28290](https://github.com/Azure/azure-sdk-for-python/pull/28290))
- Credentials that are implemented via launching a subprocess to acquire tokens now have configurable timeouts using the `process_timeout` keyword argument. This addresses scenarios where these proceses can take longer than the current default timeout values. The affected credentials are `AzureCliCredential`, `AzurePowerShellCredential` and `DefaultAzureCredential` (Note: For `DefaultAzureCredential`, this option only applies to `AzureCliCredential` and `AzurePowerShellCredential` in the authentication chain.) ([#28290](https://github.com/Azure/azure-sdk-for-python/pull/28290))

### Breaking Changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,19 @@ class AzureCliCredential:
:keyword List[str] additionally_allowed_tenants: Specifies tenants in addition to the specified "tenant_id"
for which the credential may acquire tokens. Add the wildcard value "*" to allow the credential to
acquire tokens for any tenant the application can access.
:keyword int cli_process_timeout: Seconds to wait for the Azure CLI to respond. Defaults to 10.
:keyword int process_timeout: Seconds to wait for the Azure CLI process to respond. Defaults to 10.
"""
def __init__(
self,
*,
tenant_id: str = "",
additionally_allowed_tenants: Optional[List[str]] = None,
cli_process_timeout: int = 10
process_timeout: int = 10
) -> None:

self.tenant_id = tenant_id
self._additionally_allowed_tenants = additionally_allowed_tenants or []
self._cli_process_timeout = cli_process_timeout
self._process_timeout = process_timeout

def __enter__(self):
return self
Expand Down Expand Up @@ -87,7 +87,7 @@ def get_token(self, *scopes: str, **kwargs: Any) -> AccessToken:
)
if tenant:
command += " --tenant " + tenant
output = _run_command(command, self._cli_process_timeout)
output = _run_command(command, self._process_timeout)

token = parse_token(output)
if not token:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,19 @@ class AzurePowerShellCredential:
:keyword List[str] additionally_allowed_tenants: Specifies tenants in addition to the specified "tenant_id"
for which the credential may acquire tokens. Add the wildcard value "*" to allow the credential to
acquire tokens for any tenant the application can access.
:keyword int powershell_process_timeout: Seconds to wait for the Azure PowerShell to respond. Defaults to 10.
:keyword int process_timeout: Seconds to wait for the Azure PowerShell process to respond. Defaults to 10.
"""
def __init__(
self,
*,
tenant_id: str = "",
additionally_allowed_tenants: Optional[List[str]] = None,
powershell_process_timeout: int = 10
process_timeout: int = 10
) -> None:

self.tenant_id = tenant_id
self._additionally_allowed_tenants = additionally_allowed_tenants or []
self._powershell_process_timeout = powershell_process_timeout
self._process_timeout = process_timeout

def __enter__(self):
return self
Expand Down Expand Up @@ -98,7 +98,7 @@ def get_token(self, *scopes: str, **kwargs: Any) -> AccessToken:
**kwargs
)
command_line = get_command_line(scopes, tenant_id)
output = run_command_line(command_line, self._powershell_process_timeout)
output = run_command_line(command_line, self._process_timeout)
token = parse_token(output)
return token

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class DefaultAzureCredential(ChainedTokenCredential):
:class:`~azure.identity.VisualStudioCodeCredential`. Defaults to the "Azure: Tenant" setting in VS Code's user
settings or, when that setting has no value, the "organizations" tenant, which supports only Azure Active
Directory work or school accounts.
:keyword int developer_credential_timeout: The timeout in seconds to use for developer credentials
:keyword int process_timeout: The timeout in seconds to use for developer credentials that run subprocesses
(i.e. AzureCliCredential and AzurePowerShellCredential). Defaults to **10**.
"""

Expand Down Expand Up @@ -106,7 +106,7 @@ def __init__(self, **kwargs: Any) -> None:
"shared_cache_tenant_id", os.environ.get(EnvironmentVariables.AZURE_TENANT_ID)
)

developer_credential_timeout = kwargs.pop("developer_credential_timeout", 10)
process_timeout = kwargs.pop("process_timeout", 10)

exclude_environment_credential = kwargs.pop("exclude_environment_credential", False)
exclude_managed_identity_credential = kwargs.pop("exclude_managed_identity_credential", False)
Expand All @@ -133,9 +133,9 @@ def __init__(self, **kwargs: Any) -> None:
if not exclude_visual_studio_code_credential:
credentials.append(VisualStudioCodeCredential(**vscode_args))
if not exclude_cli_credential:
credentials.append(AzureCliCredential(cli_process_timeout=developer_credential_timeout))
credentials.append(AzureCliCredential(process_timeout=process_timeout))
if not exclude_powershell_credential:
credentials.append(AzurePowerShellCredential(powershell_process_timeout=developer_credential_timeout))
credentials.append(AzurePowerShellCredential(process_timeout=process_timeout))
if not exclude_interactive_browser_credential:
if interactive_browser_client_id:
credentials.append(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,19 @@ class AzureCliCredential(AsyncContextManager):
:keyword List[str] additionally_allowed_tenants: Specifies tenants in addition to the specified "tenant_id"
for which the credential may acquire tokens. Add the wildcard value "*" to allow the credential to
acquire tokens for any tenant the application can access.
:keyword int cli_process_timeout: Seconds to wait for the Azure CLI to respond. Defaults to 10.
:keyword int process_timeout: Seconds to wait for the Azure CLI process to respond. Defaults to 10.
"""
def __init__(
self,
*,
tenant_id: str = "",
additionally_allowed_tenants: Optional[List[str]] = None,
cli_process_timeout: int = 10
process_timeout: int = 10
) -> None:

self.tenant_id = tenant_id
self._additionally_allowed_tenants = additionally_allowed_tenants or []
self._cli_process_timeout = cli_process_timeout
self._process_timeout = process_timeout

@log_get_token_async
async def get_token(self, *scopes: str, **kwargs: Any) -> AccessToken:
Expand Down Expand Up @@ -79,7 +79,7 @@ async def get_token(self, *scopes: str, **kwargs: Any) -> AccessToken:

if tenant:
command += " --tenant " + tenant
output = await _run_command(command, self._cli_process_timeout)
output = await _run_command(command, self._process_timeout)

token = parse_token(output)
if not token:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,20 @@ class AzurePowerShellCredential(AsyncContextManager):
:keyword List[str] additionally_allowed_tenants: Specifies tenants in addition to the specified "tenant_id"
for which the credential may acquire tokens. Add the wildcard value "*" to allow the credential to
acquire tokens for any tenant the application can access.
:keyword int powershell_process_timeout: Seconds to wait for the Azure PowerShell to respond. Defaults to 10.
:keyword int process_timeout: Seconds to wait for the Azure PowerShell process to respond. Defaults to 10.
"""

def __init__(
self,
*,
tenant_id: str = "",
additionally_allowed_tenants: List[str] = None,
powershell_process_timeout: int = 10
process_timeout: int = 10
) -> None:

self.tenant_id = tenant_id
self._additionally_allowed_tenants = additionally_allowed_tenants or []
self._powershell_process_timeout = powershell_process_timeout
self._process_timeout = process_timeout

@log_get_token_async
async def get_token(
Expand Down Expand Up @@ -73,7 +73,7 @@ async def get_token(
**kwargs
)
command_line = get_command_line(scopes, tenant_id)
output = await run_command_line(command_line, self._powershell_process_timeout)
output = await run_command_line(command_line, self._process_timeout)
token = parse_token(output)
return token

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class DefaultAzureCredential(ChainedTokenCredential):
:class:`~azure.identity.aio.VisualStudioCodeCredential`. Defaults to the "Azure: Tenant" setting in VS Code's
user settings or, when that setting has no value, the "organizations" tenant, which supports only Azure Active
Directory work or school accounts.
:keyword int developer_credential_timeout: The timeout in seconds to use for developer credentials
:keyword int process_timeout: The timeout in seconds to use for developer credentials that run subprocesses
(i.e. AzureCliCredential and AzurePowerShellCredential). Defaults to **10**.
"""

Expand Down Expand Up @@ -97,7 +97,7 @@ def __init__(self, **kwargs: Any) -> None:
"visual_studio_code_tenant_id", os.environ.get(EnvironmentVariables.AZURE_TENANT_ID)
)

developer_credential_timeout = kwargs.pop("developer_credential_timeout", 10)
process_timeout = kwargs.pop("process_timeout", 10)

exclude_visual_studio_code_credential = kwargs.pop("exclude_visual_studio_code_credential", True)
exclude_cli_credential = kwargs.pop("exclude_cli_credential", False)
Expand All @@ -123,9 +123,9 @@ def __init__(self, **kwargs: Any) -> None:
if not exclude_visual_studio_code_credential:
credentials.append(VisualStudioCodeCredential(**vscode_args))
if not exclude_cli_credential:
credentials.append(AzureCliCredential(cli_process_timeout=developer_credential_timeout))
credentials.append(AzureCliCredential(process_timeout=process_timeout))
if not exclude_powershell_credential:
credentials.append(AzurePowerShellCredential(powershell_process_timeout=developer_credential_timeout))
credentials.append(AzurePowerShellCredential(process_timeout=process_timeout))

super().__init__(*credentials)

Expand Down
4 changes: 2 additions & 2 deletions sdk/identity/azure-identity/tests/test_cli_credential.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,10 @@ def test_timeout():
with mock.patch("shutil.which", return_value="az"):
with mock.patch(CHECK_OUTPUT, mock.Mock(side_effect=TimeoutExpired("", 42))) as check_output_mock:
with pytest.raises(CredentialUnavailableError):
AzureCliCredential(cli_process_timeout=42).get_token("scope")
AzureCliCredential(process_timeout=42).get_token("scope")

# Ensure custom timeout is passed to subprocess
kwargs = check_output_mock.call_args.kwargs
_, kwargs = check_output_mock.call_args
assert "timeout" in kwargs
assert kwargs["timeout"] == 42

Expand Down
12 changes: 6 additions & 6 deletions sdk/identity/azure-identity/tests/test_default.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,19 +362,19 @@ def validate_client_id(credential):
validate_client_id(mock_credential)


def test_developer_credential_timeout():
def test_process_timeout():
"""the credential should allow configuring a process timeout for Azure CLI and PowerShell by kwarg"""

timeout = 42

with patch(DefaultAzureCredential.__module__ + ".AzureCliCredential") as mock_cli_credential:
with patch(DefaultAzureCredential.__module__ + ".AzurePowerShellCredential") as mock_pwsh_credential:
DefaultAzureCredential(developer_credential_timeout=timeout)
DefaultAzureCredential(process_timeout=timeout)

assert "cli_process_timeout" in mock_cli_credential.call_args.kwargs
assert "powershell_process_timeout" in mock_pwsh_credential.call_args.kwargs
assert mock_cli_credential.call_args.kwargs["cli_process_timeout"] == timeout
assert mock_pwsh_credential.call_args.kwargs["powershell_process_timeout"] == timeout
for credential in (mock_cli_credential, mock_pwsh_credential):
_, kwargs = credential.call_args
assert "process_timeout" in kwargs
assert kwargs["process_timeout"] == timeout


def test_unexpected_kwarg():
Expand Down
10 changes: 5 additions & 5 deletions sdk/identity/azure-identity/tests/test_default_async.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,12 +279,12 @@ def test_developer_credential_timeout():

with patch(DefaultAzureCredential.__module__ + ".AzureCliCredential") as mock_cli_credential:
with patch(DefaultAzureCredential.__module__ + ".AzurePowerShellCredential") as mock_pwsh_credential:
DefaultAzureCredential(developer_credential_timeout=timeout)
DefaultAzureCredential(process_timeout=timeout)

assert "cli_process_timeout" in mock_cli_credential.call_args.kwargs
assert "powershell_process_timeout" in mock_pwsh_credential.call_args.kwargs
assert mock_cli_credential.call_args.kwargs["cli_process_timeout"] == timeout
assert mock_pwsh_credential.call_args.kwargs["powershell_process_timeout"] == timeout
for credential in (mock_cli_credential, mock_pwsh_credential):
_, kwargs = credential.call_args
assert "process_timeout" in kwargs
assert kwargs["process_timeout"] == timeout


def test_unexpected_kwarg():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,11 @@ def test_timeout():
proc = Mock(communicate=Mock(side_effect=TimeoutExpired("", 42)), returncode=None)
with patch(POPEN, Mock(return_value=proc)):
with pytest.raises(CredentialUnavailableError):
AzurePowerShellCredential(powershell_process_timeout=42).get_token("scope")
AzurePowerShellCredential(process_timeout=42).get_token("scope")

assert proc.communicate.call_count == 1
# Ensure custom timeout is passed to subprocess
kwargs = proc.communicate.call_args.kwargs
_, kwargs = proc.communicate.call_args
assert "timeout" in kwargs
assert kwargs["timeout"] == 42

Expand Down

0 comments on commit e5c4ed5

Please sign in to comment.