From df7fbe4c76cf17dcc50964ad374232a6abab9715 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Fri, 8 Dec 2023 10:16:52 -0800 Subject: [PATCH 1/4] Adding Pre-Kill --- .../_azureappconfigurationprovider.py | 22 ++++++++++++++++++- .../_azureappconfigurationproviderasync.py | 16 ++++++++++++-- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py index e773b1f042f8..6e16147a091a 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py @@ -7,6 +7,7 @@ import json import random import time +import datetime from threading import Lock import logging from typing import ( @@ -51,6 +52,8 @@ logger = logging.getLogger(__name__) +min_uptime = 5 + @overload def load( @@ -151,6 +154,7 @@ def load(*args, **kwargs) -> "AzureAppConfigurationProvider": credential: Optional["TokenCredential"] = kwargs.pop("credential", None) connection_string: Optional[str] = kwargs.pop("connection_string", None) key_vault_options: Optional[AzureAppConfigurationKeyVaultOptions] = kwargs.pop("key_vault_options", None) + start_time = datetime.datetime.now() # Update endpoint and credential if specified positionally. if len(args) > 2: @@ -186,7 +190,11 @@ def load(*args, **kwargs) -> "AzureAppConfigurationProvider": provider = _buildprovider( connection_string, endpoint, credential, uses_key_vault="UsesKeyVault" in headers, **kwargs ) - provider._load_all(headers=headers) + try: + provider._load_all(headers=headers) + except Exception as e: + _prekill(start_time) + raise e # Refresh-All sentinels are not updated on load_all, as they are not necessarily included in the provider. for (key, label), etag in provider._refresh_on.items(): @@ -203,9 +211,21 @@ def load(*args, **kwargs) -> "AzureAppConfigurationProvider": label, ) else: + _prekill(start_time) raise e + except Exception as e: + _prekill(start_time) + raise e return provider +def _prekill(start_time: datetime.datetime) -> None: + # We want to make sure we are up a minimum amount of time before we kill the process. Otherwise, we could get stuck + # in a quick restart loop. + min_time = datetime.timedelta(seconds=min_uptime) + current_time = datetime.datetime.now() + if current_time - start_time < min_time: + time.sleep(min_time - (current_time - start_time)) + def _get_headers(request_type, **kwargs) -> str: headers = kwargs.pop("headers", {}) diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_azureappconfigurationproviderasync.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_azureappconfigurationproviderasync.py index b6f7f4f0dae9..c19c3ef3a161 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_azureappconfigurationproviderasync.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_azureappconfigurationproviderasync.py @@ -4,6 +4,7 @@ # license information. # ------------------------------------------------------------------------- import json +import datetime from asyncio.locks import Lock import logging from typing import ( @@ -42,6 +43,7 @@ _get_headers, _RefreshTimer, _build_sentinel, + _prekill, ) from .._user_agent import USER_AGENT @@ -153,6 +155,7 @@ async def load(*args, **kwargs) -> "AzureAppConfigurationProvider": credential: Optional["AsyncTokenCredential"] = kwargs.pop("credential", None) connection_string: Optional[str] = kwargs.pop("connection_string", None) key_vault_options: Optional[AzureAppConfigurationKeyVaultOptions] = kwargs.pop("key_vault_options", None) + start_time = datetime.datetime.now() # Update endpoint and credential if specified positionally. if len(args) > 2: @@ -186,7 +189,13 @@ async def load(*args, **kwargs) -> "AzureAppConfigurationProvider": headers = _get_headers("Startup", **kwargs) provider = _buildprovider(connection_string, endpoint, credential, **kwargs) - await provider._load_all(headers=headers) + + try: + await provider._load_all(headers=headers) + except Exception as e: + _prekill(start_time) + raise e + # Refresh-All sentinels are not updated on load_all, as they are not necessarily included in the provider. for (key, label), etag in provider._refresh_on.items(): @@ -204,10 +213,13 @@ async def load(*args, **kwargs) -> "AzureAppConfigurationProvider": ) provider._refresh_on[(key, label)] = None else: + _prekill(start_time) raise e + except Exception as e: + _prekill(start_time) + raise e return provider - def _buildprovider( connection_string: Optional[str], endpoint: Optional[str], credential: Optional["AsyncTokenCredential"], **kwargs ) -> "AzureAppConfigurationProvider": From f6120c5da708b8e505f7e420a53bdd17fa3a2f70 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Fri, 8 Dec 2023 10:49:45 -0800 Subject: [PATCH 2/4] Adding Tests --- .../provider/_azureappconfigurationprovider.py | 1 + .../aio/_azureappconfigurationproviderasync.py | 4 ++-- .../tests/test_provider.py | 16 ++++++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py index 6e16147a091a..c6c3f928e646 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py @@ -218,6 +218,7 @@ def load(*args, **kwargs) -> "AzureAppConfigurationProvider": raise e return provider + def _prekill(start_time: datetime.datetime) -> None: # We want to make sure we are up a minimum amount of time before we kill the process. Otherwise, we could get stuck # in a quick restart loop. diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_azureappconfigurationproviderasync.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_azureappconfigurationproviderasync.py index c19c3ef3a161..463a77136519 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_azureappconfigurationproviderasync.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_azureappconfigurationproviderasync.py @@ -189,14 +189,13 @@ async def load(*args, **kwargs) -> "AzureAppConfigurationProvider": headers = _get_headers("Startup", **kwargs) provider = _buildprovider(connection_string, endpoint, credential, **kwargs) - + try: await provider._load_all(headers=headers) except Exception as e: _prekill(start_time) raise e - # Refresh-All sentinels are not updated on load_all, as they are not necessarily included in the provider. for (key, label), etag in provider._refresh_on.items(): if not etag: @@ -220,6 +219,7 @@ async def load(*args, **kwargs) -> "AzureAppConfigurationProvider": raise e return provider + def _buildprovider( connection_string: Optional[str], endpoint: Optional[str], credential: Optional["AsyncTokenCredential"], **kwargs ) -> "AzureAppConfigurationProvider": diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_provider.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_provider.py index 12d993a4ed70..5a2aebf144df 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_provider.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_provider.py @@ -7,6 +7,10 @@ from devtools_testutils import recorded_by_proxy from preparers import app_config_decorator from testcase import AppConfigTestCase +import datetime +from unittest.mock import patch + +from azure.appconfiguration.provider._azureappconfigurationprovider import _prekill class TestAppConfigurationProvider(AppConfigTestCase): @@ -104,6 +108,18 @@ def test_provider_secret_resolver_options(self, appconfiguration_connection_stri ) assert client["secret"] == "Reslover Value" + # method: _prekill + @patch("time.sleep") + def test_prekill(self, mock_sleep, **kwargs): + start_time = datetime.datetime.now() + _prekill(start_time) + mock_sleep.assert_called_with(datetime.timedelta(seconds=5)) + + mock_sleep.reset_mock() + start_time = datetime.datetime.now() - datetime.timedelta(seconds=10) + _prekill(start_time) + mock_sleep.assert_not_called() + def secret_resolver(secret_id): return "Reslover Value" From cf6759bd6ccc1ae5cd7182c4c52a0fa4066e8403 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Fri, 8 Dec 2023 10:54:02 -0800 Subject: [PATCH 3/4] Update CHANGELOG.md --- .../azure-appconfiguration-provider/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md b/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md index 928b6e03a80b..b9f6967dca69 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md +++ b/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md @@ -5,6 +5,7 @@ ### Features Added - Added on_refresh_success callback to load method. This callback is called when the refresh method successfully refreshes the configuration. +- Added minimum up time. This is the minimum amount of time the proivder will try to be up before throwing an error. This is to prevent quick restart loops. ### Breaking Changes From 736a1cd10b83ffcd73044f905a8b1fc6e18400ec Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Fri, 8 Dec 2023 11:44:27 -0800 Subject: [PATCH 4/4] Fixes live test issue. SP. --- .../azure-appconfiguration-provider/CHANGELOG.md | 2 +- .../azure-appconfiguration-provider/tests/test_provider.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md b/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md index b9f6967dca69..2a59bc83aa30 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md +++ b/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md @@ -5,7 +5,7 @@ ### Features Added - Added on_refresh_success callback to load method. This callback is called when the refresh method successfully refreshes the configuration. -- Added minimum up time. This is the minimum amount of time the proivder will try to be up before throwing an error. This is to prevent quick restart loops. +- Added minimum up time. This is the minimum amount of time the provider will try to be up before throwing an error. This is to prevent quick restart loops. ### Breaking Changes diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_provider.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_provider.py index 5a2aebf144df..e2a0a4a2282a 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_provider.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_provider.py @@ -113,7 +113,7 @@ def test_provider_secret_resolver_options(self, appconfiguration_connection_stri def test_prekill(self, mock_sleep, **kwargs): start_time = datetime.datetime.now() _prekill(start_time) - mock_sleep.assert_called_with(datetime.timedelta(seconds=5)) + assert mock_sleep.call_count == 1 mock_sleep.reset_mock() start_time = datetime.datetime.now() - datetime.timedelta(seconds=10)