From e5c4ed5805f0387d40e040125956a93b1c30b75a Mon Sep 17 00:00:00 2001 From: Paul Van Eck Date: Wed, 11 Jan 2023 19:47:18 -0800 Subject: [PATCH] Change name to process_timeout Signed-off-by: Paul Van Eck --- sdk/identity/azure-identity/CHANGELOG.md | 5 +---- .../azure/identity/_credentials/azure_cli.py | 8 ++++---- .../azure/identity/_credentials/azure_powershell.py | 8 ++++---- .../azure/identity/_credentials/default.py | 8 ++++---- .../azure/identity/aio/_credentials/azure_cli.py | 8 ++++---- .../identity/aio/_credentials/azure_powershell.py | 8 ++++---- .../azure/identity/aio/_credentials/default.py | 8 ++++---- .../azure-identity/tests/test_cli_credential.py | 4 ++-- sdk/identity/azure-identity/tests/test_default.py | 12 ++++++------ .../azure-identity/tests/test_default_async.py | 10 +++++----- .../tests/test_powershell_credential.py | 4 ++-- 11 files changed, 40 insertions(+), 43 deletions(-) diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md index 222f0ce07ec0..e56e4748fb8f 100644 --- a/sdk/identity/azure-identity/CHANGELOG.md +++ b/sdk/identity/azure-identity/CHANGELOG.md @@ -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 diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py b/sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py index 154d7b9659f1..250f06fef037 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py @@ -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 @@ -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: diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/azure_powershell.py b/sdk/identity/azure-identity/azure/identity/_credentials/azure_powershell.py index 8dfe748a63a3..88c96a395669 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/azure_powershell.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/azure_powershell.py @@ -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 @@ -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 diff --git a/sdk/identity/azure-identity/azure/identity/_credentials/default.py b/sdk/identity/azure-identity/azure/identity/_credentials/default.py index 51f0b2ebb9fc..c803f86652af 100644 --- a/sdk/identity/azure-identity/azure/identity/_credentials/default.py +++ b/sdk/identity/azure-identity/azure/identity/_credentials/default.py @@ -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**. """ @@ -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) @@ -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( diff --git a/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_cli.py b/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_cli.py index 4ddc5e5d86bc..9743cfa3d5d0 100644 --- a/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_cli.py +++ b/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_cli.py @@ -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: @@ -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: diff --git a/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_powershell.py b/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_powershell.py index fdb076a1b356..3459a5a5e30e 100644 --- a/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_powershell.py +++ b/sdk/identity/azure-identity/azure/identity/aio/_credentials/azure_powershell.py @@ -29,7 +29,7 @@ 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__( @@ -37,12 +37,12 @@ def __init__( *, 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( @@ -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 diff --git a/sdk/identity/azure-identity/azure/identity/aio/_credentials/default.py b/sdk/identity/azure-identity/azure/identity/aio/_credentials/default.py index 39754dee4dfb..05697d5c5121 100644 --- a/sdk/identity/azure-identity/azure/identity/aio/_credentials/default.py +++ b/sdk/identity/azure-identity/azure/identity/aio/_credentials/default.py @@ -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**. """ @@ -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) @@ -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) diff --git a/sdk/identity/azure-identity/tests/test_cli_credential.py b/sdk/identity/azure-identity/tests/test_cli_credential.py index 056dd728e866..baa1e744610e 100644 --- a/sdk/identity/azure-identity/tests/test_cli_credential.py +++ b/sdk/identity/azure-identity/tests/test_cli_credential.py @@ -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 diff --git a/sdk/identity/azure-identity/tests/test_default.py b/sdk/identity/azure-identity/tests/test_default.py index 9d53aa754104..5121f61633e8 100644 --- a/sdk/identity/azure-identity/tests/test_default.py +++ b/sdk/identity/azure-identity/tests/test_default.py @@ -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(): diff --git a/sdk/identity/azure-identity/tests/test_default_async.py b/sdk/identity/azure-identity/tests/test_default_async.py index d0c792473c08..f4e785a812f9 100644 --- a/sdk/identity/azure-identity/tests/test_default_async.py +++ b/sdk/identity/azure-identity/tests/test_default_async.py @@ -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(): diff --git a/sdk/identity/azure-identity/tests/test_powershell_credential.py b/sdk/identity/azure-identity/tests/test_powershell_credential.py index ef42fa98f019..7c4b11715760 100644 --- a/sdk/identity/azure-identity/tests/test_powershell_credential.py +++ b/sdk/identity/azure-identity/tests/test_powershell_credential.py @@ -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