From 6c77ae6a203ceb8f8dedbad3e91d6b66fbde3c3c Mon Sep 17 00:00:00 2001 From: Alexandra Szanto Date: Thu, 29 Oct 2026 14:00:29 +0100 Subject: [PATCH 1/7] feat: use UnleashEngine.get_spec_version --- UnleashClient/constants.py | 3 ++- pyproject.toml | 2 +- requirements.txt | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/UnleashClient/constants.py b/UnleashClient/constants.py index f9059e81..bf61f6ed 100644 --- a/UnleashClient/constants.py +++ b/UnleashClient/constants.py @@ -1,4 +1,5 @@ from importlib_metadata import version +from yggdrasil_engine.engine import UnleashEngine # Library SDK_NAME = "unleash-python-sdk" @@ -6,7 +7,7 @@ REQUEST_TIMEOUT = 30 REQUEST_RETRIES = 3 METRIC_LAST_SENT_TIME = "mlst" -CLIENT_SPEC_VERSION = "6.1.0" +CLIENT_SPEC_VERSION = UnleashEngine.get_spec_version() # =Unleash= APPLICATION_HEADERS = { diff --git a/pyproject.toml b/pyproject.toml index f0a26c95..64782bfd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,7 +29,7 @@ dependencies=[ "importlib_metadata", "python-dateutil", "semver < 4.0.0", - "yggdrasil-engine >= 3.0.0a1, < 4.0.0", + "yggdrasil-engine >= 3.0.0a2, < 4.0.0", "launchdarkly-eventsource", ] diff --git a/requirements.txt b/requirements.txt index 84384e59..a3f0d69c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,7 +6,7 @@ mmhash3 python-dateutil requests semver -yggdrasil-engine==3.0.0a1 +yggdrasil-engine==3.0.0a2 launchdarkly-eventsource aiohttp From 3cc303744d982934b3ad144b0ac6ac2dd3ff7ab7 Mon Sep 17 00:00:00 2001 From: Alexandra Szanto Date: Thu, 29 Oct 2026 14:11:41 +0100 Subject: [PATCH 2/7] chore: remove unused function and dep --- UnleashClient/utils.py | 12 ------------ pyproject.toml | 1 - requirements.txt | 1 - 3 files changed, 14 deletions(-) diff --git a/UnleashClient/utils.py b/UnleashClient/utils.py index 6514eb36..46ad5dff 100644 --- a/UnleashClient/utils.py +++ b/UnleashClient/utils.py @@ -2,8 +2,6 @@ from enum import Enum from typing import Any, Optional -import mmh3 # pylint: disable=import-error - LOGGER = logging.getLogger("UnleashClient") @@ -13,16 +11,6 @@ class InstanceAllowType(Enum): SILENTLY_ALLOW = 3 -def normalized_hash( - identifier: str, activation_group: str, normalizer: int = 100, seed: int = 0 -) -> int: - return ( - mmh3.hash(f"{activation_group}:{identifier}", signed=False, seed=seed) - % normalizer - + 1 - ) - - def get_identifier(context_key_name: str, context: dict) -> Any: if context_key_name in context.keys(): value = context[context_key_name] diff --git a/pyproject.toml b/pyproject.toml index 64782bfd..ba9a95f8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,7 +24,6 @@ classifiers=[ dependencies=[ "requests", "fcache", - "mmh3", "apscheduler < 4.0.0", "importlib_metadata", "python-dateutil", diff --git a/requirements.txt b/requirements.txt index a3f0d69c..961c2ac1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,7 +2,6 @@ APScheduler fcache importlib_metadata -mmhash3 python-dateutil requests semver From d9fb8e5ea40c2f710fc5f846714e5a6261916797 Mon Sep 17 00:00:00 2001 From: Alexandra Szanto Date: Mon, 2 Nov 2026 11:26:47 +0100 Subject: [PATCH 3/7] refactor: add url validation in init --- CHANGELOG.md | 1 + UnleashClient/clients/async_unleash_client.py | 10 +++-- UnleashClient/clients/unleash_client.py | 6 ++- UnleashClient/utils.py | 27 ++++++++++++ .../clients/test_async_unleash_client.py | 18 ++++++++ .../unit_tests/clients/test_unleash_client.py | 41 +++++++++++++++++++ tests/unit_tests/test_utils.py | 38 ++++++++++++++++- v7_MIGRATION_GUIDE.md | 17 ++++++++ 8 files changed, 153 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9d61359..ea49cc42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * (Major): Settings can no longer be changed on an existing client. The `unleash_*` settings properties and the `strategy_mapping` and `connector_mode` properties have been removed, together with their setters. * (Major): The API token is passed as `api_key=""`. An `Authorization` header in `HttpOptions.headers` raises `ValueError`. * (Major): The deprecated `environment` setting has been removed. The `environment` field of the evaluation context and the impact metrics `environment` label come from the API token, or are `"default"` when the token doesn't name one. +* (Major): `initialize_client()` raises `ValueError` if `url` is not an absolute http(s) URL with a host, for every client, including ones that never contact the server. * (Major): Upgraded to yggdrasil-engine 3, new engine built with PyO3 for better performance. * (Minor): New `FETCH_COMPLETED` event, emitted after every feature fetch with how it ended: `CHANGED`, `NOT_CHANGED`, `UNAUTHORIZED`, `RATE_LIMITED`, `UNAVAILABLE` or `INVALID_RESPONSE`. diff --git a/UnleashClient/clients/async_unleash_client.py b/UnleashClient/clients/async_unleash_client.py index 14fef239..d9751c9e 100644 --- a/UnleashClient/clients/async_unleash_client.py +++ b/UnleashClient/clients/async_unleash_client.py @@ -37,7 +37,7 @@ from UnleashClient.constants import ETAG, METRIC_LAST_SENT_TIME from UnleashClient.events import BaseEvent from UnleashClient.impact_metrics import ImpactMetrics -from UnleashClient.utils import LOGGER, InstanceAllowType +from UnleashClient.utils import LOGGER, InstanceAllowType, _validate_url class AsyncUnleashClient: @@ -269,8 +269,10 @@ async def initialize_client(self, fetch_toggles: bool = True) -> None: :param fetch_toggles: Accepted for parity with :class:`UnleashClient`. It has no effect: the client always polls. - :raises aiohttp.InvalidURL: If registration is enabled and the URL is invalid. - :raises ValueError: If a custom strategy is invalid. + :raises ValueError: If the URL is not an absolute http(s) URL with a host, + or a custom strategy is invalid. + :raises aiohttp.InvalidURL: If registration is enabled and aiohttp + rejects a URL that passed the check above. """ if self._closed or self._starting or self._run_state > _RunState.UNINITIALIZED: warnings.warn( @@ -280,6 +282,8 @@ async def initialize_client(self, fetch_toggles: bool = True) -> None: self._starting = True try: + _validate_url(self._config.url) + self._cache.mset( {METRIC_LAST_SENT_TIME: datetime.now(timezone.utc), ETAG: ""} ) diff --git a/UnleashClient/clients/unleash_client.py b/UnleashClient/clients/unleash_client.py index 8f77e451..83fe990e 100644 --- a/UnleashClient/clients/unleash_client.py +++ b/UnleashClient/clients/unleash_client.py @@ -49,6 +49,7 @@ from UnleashClient.utils import ( LOGGER, InstanceAllowType, + _validate_url, ) INSTANCES = _get_instance_registry() @@ -272,7 +273,8 @@ def initialize_client(self, fetch_toggles: bool = True) -> None: usually used to cater the multi-process setups, e.g. Django, Celery, etc. - This will raise an exception on registration if the URL is invalid. It is done automatically if called inside a context manager as in: + This raises a ``ValueError`` if the URL is not an absolute http(s) URL with a + host, even for a client that never talks to the server. It is done automatically if called inside a context manager as in: .. code-block:: python @@ -288,6 +290,8 @@ def initialize_client(self, fetch_toggles: bool = True) -> None: return try: start_scheduler = False + + _validate_url(self._config.url) if self._custom_strategies: self._engine.register_custom_strategies(self._custom_strategies) diff --git a/UnleashClient/utils.py b/UnleashClient/utils.py index 46ad5dff..ed33c099 100644 --- a/UnleashClient/utils.py +++ b/UnleashClient/utils.py @@ -1,6 +1,7 @@ import logging from enum import Enum from typing import Any, Optional +from urllib.parse import urlsplit LOGGER = logging.getLogger("UnleashClient") @@ -35,3 +36,29 @@ def extract_environment_from_api_key(api_key: Optional[str]) -> Optional[str]: environment, _, _ = after_colon.partition(".") return environment or None + + +def _validate_url(url: str) -> None: + """ + Checks that ``url`` has the shape of an Unleash API url. + + Only the shape is checked, not whether the host exists or answers: that is a + network failure, which the client reports. + + :raises ValueError: if ``url`` is not an absolute http(s) url with a host. + """ + if any(c.isspace() for c in url): + raise ValueError(f"Unleash URL contains whitespace: {url!r}") + + try: + parts = urlsplit(url) + _ = parts.port + except ValueError as exc: + raise ValueError(f"Invalid Unleash URL {url!r}: {exc}") from exc + + if parts.scheme not in ("http", "https"): + raise ValueError( + f"Unleash URL must start with http:// or https://, got {url!r}" + ) + if not parts.hostname: + raise ValueError(f"Unleash URL has no host: {url!r}") diff --git a/tests/unit_tests/clients/test_async_unleash_client.py b/tests/unit_tests/clients/test_async_unleash_client.py index 6ac7395d..4bda4e10 100644 --- a/tests/unit_tests/clients/test_async_unleash_client.py +++ b/tests/unit_tests/clients/test_async_unleash_client.py @@ -627,6 +627,24 @@ async def test_initializing_twice_warns_and_registers_once( assert len(server.calls("POST", REGISTER_PATH)) == 1 +@pytest.mark.asyncio +@pytest.mark.parametrize( + "metrics", + [MetricsOptions(), MetricsOptions(disabled=True, disable_registration=True)], +) +async def test_initializing_with_an_invalid_url_raises(tmpdir, metrics): + client = build_async_client( + tmpdir, url="thisisnotavalidurl", app_name=APP_NAME, metrics=metrics + ) + try: + with pytest.raises(ValueError, match="must start with http:// or https://"): + await client.initialize_client() + + assert not client.is_initialized + finally: + await client.destroy() + + @pytest.mark.asyncio async def test_initializing_a_destroyed_client_warns_and_does_nothing( server, build_running_client diff --git a/tests/unit_tests/clients/test_unleash_client.py b/tests/unit_tests/clients/test_unleash_client.py index 100adf72..e582aa3b 100644 --- a/tests/unit_tests/clients/test_unleash_client.py +++ b/tests/unit_tests/clients/test_unleash_client.py @@ -849,6 +849,47 @@ def test_uc_with_invalid_url(tmp_path): unleash_client.destroy() +def test_uc_with_invalid_url_raises_when_registration_is_disabled(tmp_path): + unleash_client = UnleashClient( + "thisisnotavalidurl", + APP_NAME, + metrics=MetricsOptions(disabled=True, disable_registration=True), + cache_directory=str(tmp_path), + ) + + with pytest.raises(ValueError, match="must start with http:// or https://"): + unleash_client.initialize_client() + unleash_client.destroy() + + +def test_uc_offline_starts_with_a_well_formed_url(tmp_path): + unleash_client = UnleashClient( + "http://localhost", + APP_NAME, + metrics=MetricsOptions(disabled=True, disable_registration=True), + cache_directory=str(tmp_path), + ) + try: + unleash_client.initialize_client(fetch_toggles=False) + + assert isinstance(unleash_client.connector, OfflineConnector) + finally: + unleash_client.destroy() + + +def test_uc_offline_still_validates_the_url(tmp_path): + unleash_client = UnleashClient( + "placeholder", + APP_NAME, + metrics=MetricsOptions(disabled=True, disable_registration=True), + cache_directory=str(tmp_path), + ) + + with pytest.raises(ValueError, match="must start with http:// or https://"): + unleash_client.initialize_client(fetch_toggles=False) + unleash_client.destroy() + + def test_uc_with_network_error(tmp_path): unleash_client = UnleashClient( "https://this-will-never-try-to-dns-resolve.invalid/", diff --git a/tests/unit_tests/test_utils.py b/tests/unit_tests/test_utils.py index afb76b84..ddf4eaa3 100644 --- a/tests/unit_tests/test_utils.py +++ b/tests/unit_tests/test_utils.py @@ -1,4 +1,6 @@ -from UnleashClient.utils import extract_environment_from_api_key +import pytest + +from UnleashClient.utils import _validate_url, extract_environment_from_api_key def test_valid_api_key(): @@ -14,3 +16,37 @@ def test_api_key_not_present(): def test_environment_part_is_empty(): result = extract_environment_from_api_key("project:.hash") assert result is None + + +@pytest.mark.parametrize( + "url", + [ + "http://localhost:4242/api", + "https://unleash.example.com/api", + "https://unleash.example.com:8443/api", + "http://127.0.0.1/api", + "http://[::1]:4242/api", + ], +) +def test_validate_url_accepts_http_urls_with_a_host(url): + _validate_url(url) + + +@pytest.mark.parametrize( + "url, message", + [ + ("thisisnotavalidurl", "must start with http:// or https://"), + ("localhost:4242/api", "must start with http:// or https://"), + ("ftp://unleash.example.com", "must start with http:// or https://"), + ("", "must start with http:// or https://"), + ("https:", "has no host"), + ("https:///api", "has no host"), + ("https://unleash.example.com:abc/api", "Invalid Unleash URL"), + ("https://unleash.example.com:99999/api", "Invalid Unleash URL"), + ("https://unleash.example.com/my api", "contains whitespace"), + (" https://unleash.example.com/api", "contains whitespace"), + ], +) +def test_validate_url_rejects_malformed_urls(url, message): + with pytest.raises(ValueError, match=message): + _validate_url(url) diff --git a/v7_MIGRATION_GUIDE.md b/v7_MIGRATION_GUIDE.md index 832fcf30..0d86554b 100644 --- a/v7_MIGRATION_GUIDE.md +++ b/v7_MIGRATION_GUIDE.md @@ -81,6 +81,23 @@ client = UnleashClient(url, app_name, api_key="") An `Authorization` header in `HttpOptions.headers` raises a `ValueError`, in any letter case. It is not silently ignored, because a client without a key still starts: its requests fail with 401 and it keeps serving the flags in its cache, so the mistake could go unnoticed. All other custom headers work as before. +## The URL is validated + +`initialize_client()` raises a `ValueError` if `url` is not an absolute `http://` or `https://` URL with a host. Only the shape is checked, not whether the server can be reached. + +This applies to every client, including one that never talks to the server (`fetch_toggles=False` with registration and metrics disabled). In v6 such a client accepted any string as `url`; pass a well-formed one instead: + +``` python +client = UnleashClient( + "http://localhost", + app_name, + metrics=MetricsOptions(disabled=True, disable_registration=True), +) +client.initialize_client(fetch_toggles=False) +``` + +In v6 a malformed URL only raised when the client registered. With registration disabled, every fetch and metrics send failed and was only logged. + ## `environment` has been removed The `environment` argument has been removed, with no replacement. Your API token decides which environment the client gets flags for. From b4a2d6dca4fd5498e07ff5a7ea19b6e1770f600f Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 2 Nov 2026 10:34:47 +0000 Subject: [PATCH 4/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- UnleashClient/clients/unleash_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UnleashClient/clients/unleash_client.py b/UnleashClient/clients/unleash_client.py index 83fe990e..0ee18a5b 100644 --- a/UnleashClient/clients/unleash_client.py +++ b/UnleashClient/clients/unleash_client.py @@ -290,7 +290,7 @@ def initialize_client(self, fetch_toggles: bool = True) -> None: return try: start_scheduler = False - + _validate_url(self._config.url) if self._custom_strategies: From 9d392240994642fee41612d53fdd6a3ecc5e2b8d Mon Sep 17 00:00:00 2001 From: Alexandra Szanto Date: Mon, 2 Nov 2026 11:26:47 +0100 Subject: [PATCH 5/7] refactor: add url validation in init --- CHANGELOG.md | 2 +- UnleashClient/clients/async_unleash_client.py | 8 +-- UnleashClient/clients/unleash_client.py | 3 +- UnleashClient/config.py | 13 ++++- .../clients/test_async_unleash_client.py | 17 ++---- .../unit_tests/clients/test_unleash_client.py | 55 ++++++++----------- tests/unit_tests/test_async_transport.py | 11 ++++ tests/unit_tests/test_config.py | 10 ++++ tests/unit_tests/test_transport.py | 16 +++++- v7_MIGRATION_GUIDE.md | 2 +- 10 files changed, 82 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea49cc42..05e94865 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ * (Major): Settings can no longer be changed on an existing client. The `unleash_*` settings properties and the `strategy_mapping` and `connector_mode` properties have been removed, together with their setters. * (Major): The API token is passed as `api_key=""`. An `Authorization` header in `HttpOptions.headers` raises `ValueError`. * (Major): The deprecated `environment` setting has been removed. The `environment` field of the evaluation context and the impact metrics `environment` label come from the API token, or are `"default"` when the token doesn't name one. -* (Major): `initialize_client()` raises `ValueError` if `url` is not an absolute http(s) URL with a host, for every client, including ones that never contact the server. +* (Major): The `UnleashClient` and `AsyncUnleashClient` constructors raise `ValueError` if `url` is not an absolute http(s) URL with a host, for every client. * (Major): Upgraded to yggdrasil-engine 3, new engine built with PyO3 for better performance. * (Minor): New `FETCH_COMPLETED` event, emitted after every feature fetch with how it ended: `CHANGED`, `NOT_CHANGED`, `UNAUTHORIZED`, `RATE_LIMITED`, `UNAVAILABLE` or `INVALID_RESPONSE`. diff --git a/UnleashClient/clients/async_unleash_client.py b/UnleashClient/clients/async_unleash_client.py index d9751c9e..6d31a366 100644 --- a/UnleashClient/clients/async_unleash_client.py +++ b/UnleashClient/clients/async_unleash_client.py @@ -65,7 +65,7 @@ class AsyncUnleashClient: client.impact_metrics.define_counter("purchases", "Number of purchases") client.impact_metrics.increment_counter("purchases") - :param url: URL of the Unleash server, required. + :param url: URL of the Unleash server, required. Must be an absolute http(s) URL with a host. :param app_name: Name of the application using the client, required. :param api_key: Client API token. Optional, because a client that only loads from a backup never talks to the server. :param identity: How the client identifies itself. @@ -269,10 +269,10 @@ async def initialize_client(self, fetch_toggles: bool = True) -> None: :param fetch_toggles: Accepted for parity with :class:`UnleashClient`. It has no effect: the client always polls. - :raises ValueError: If the URL is not an absolute http(s) URL with a host, - or a custom strategy is invalid. + :raises ValueError: If a custom strategy is invalid. :raises aiohttp.InvalidURL: If registration is enabled and aiohttp - rejects a URL that passed the check above. + rejects a URL that passed the constructor's + check. """ if self._closed or self._starting or self._run_state > _RunState.UNINITIALIZED: warnings.warn( diff --git a/UnleashClient/clients/unleash_client.py b/UnleashClient/clients/unleash_client.py index 0ee18a5b..f9c9e4a0 100644 --- a/UnleashClient/clients/unleash_client.py +++ b/UnleashClient/clients/unleash_client.py @@ -273,8 +273,7 @@ def initialize_client(self, fetch_toggles: bool = True) -> None: usually used to cater the multi-process setups, e.g. Django, Celery, etc. - This raises a ``ValueError`` if the URL is not an absolute http(s) URL with a - host, even for a client that never talks to the server. It is done automatically if called inside a context manager as in: + It is done automatically if called inside a context manager as in: .. code-block:: python diff --git a/UnleashClient/config.py b/UnleashClient/config.py index 3b6167e8..e62f0817 100644 --- a/UnleashClient/config.py +++ b/UnleashClient/config.py @@ -6,7 +6,11 @@ from typing import Any, Mapping, Optional, Union from UnleashClient.constants import REQUEST_RETRIES, REQUEST_TIMEOUT -from UnleashClient.utils import LOGGER, extract_environment_from_api_key +from UnleashClient.utils import ( + LOGGER, + _validate_url, + extract_environment_from_api_key, +) def redact_to_print_safely(value: Optional[str]) -> Optional[str]: @@ -120,6 +124,12 @@ class UnleashConfig: Types are not validated or coerced beyond ``url.rstrip("/")`` and ``int()`` on the two jitters. Option groups left as None get their defaults. + ``url`` must be an absolute http(s) URL with a host, even for a client that + never talks to the server: deciding which settings make requests would need + updating with every new server call. + + :raises ValueError: if ``url`` is not an absolute http(s) URL with a host. + ``api_key`` is optional because a client that only loads from a backup never talks to the server. """ @@ -140,6 +150,7 @@ class UnleashConfig: def __post_init__(self) -> None: object.__setattr__(self, "url", self.url.rstrip("/")) + _validate_url(self.url) if self.identity is None: object.__setattr__(self, "identity", IdentityOptions()) if self.http is None: diff --git a/tests/unit_tests/clients/test_async_unleash_client.py b/tests/unit_tests/clients/test_async_unleash_client.py index 4bda4e10..765bb9d9 100644 --- a/tests/unit_tests/clients/test_async_unleash_client.py +++ b/tests/unit_tests/clients/test_async_unleash_client.py @@ -627,22 +627,15 @@ async def test_initializing_twice_warns_and_registers_once( assert len(server.calls("POST", REGISTER_PATH)) == 1 -@pytest.mark.asyncio @pytest.mark.parametrize( "metrics", [MetricsOptions(), MetricsOptions(disabled=True, disable_registration=True)], ) -async def test_initializing_with_an_invalid_url_raises(tmpdir, metrics): - client = build_async_client( - tmpdir, url="thisisnotavalidurl", app_name=APP_NAME, metrics=metrics - ) - try: - with pytest.raises(ValueError, match="must start with http:// or https://"): - await client.initialize_client() - - assert not client.is_initialized - finally: - await client.destroy() +def test_async_client_with_an_invalid_url_raises_on_construction(tmpdir, metrics): + with pytest.raises(ValueError, match="must start with http:// or https://"): + build_async_client( + tmpdir, url="thisisnotavalidurl", app_name=APP_NAME, metrics=metrics + ) @pytest.mark.asyncio diff --git a/tests/unit_tests/clients/test_unleash_client.py b/tests/unit_tests/clients/test_unleash_client.py index e582aa3b..30361045 100644 --- a/tests/unit_tests/clients/test_unleash_client.py +++ b/tests/unit_tests/clients/test_unleash_client.py @@ -839,27 +839,33 @@ def test_uc_server_error(readyable_unleash_client): assert unleash_client.is_enabled("testFlag") -def test_uc_with_invalid_url(tmp_path): - unleash_client = UnleashClient( - "thisisnotavalidurl", APP_NAME, cache_directory=str(tmp_path) - ) - - with pytest.raises(ValueError): - unleash_client.initialize_client() - unleash_client.destroy() +@pytest.mark.parametrize( + "metrics", + [ + MetricsOptions(), + MetricsOptions(disable_registration=True), + MetricsOptions(disabled=True, disable_registration=True), + ], +) +def test_uc_with_invalid_url_raises_on_construction(tmp_path, metrics): + with pytest.raises(ValueError, match="must start with http:// or https://"): + UnleashClient( + "thisisnotavalidurl", + APP_NAME, + metrics=metrics, + cache_directory=str(tmp_path), + ) -def test_uc_with_invalid_url_raises_when_registration_is_disabled(tmp_path): - unleash_client = UnleashClient( - "thisisnotavalidurl", - APP_NAME, - metrics=MetricsOptions(disabled=True, disable_registration=True), - cache_directory=str(tmp_path), - ) +def test_uc_with_invalid_url_is_not_counted_as_an_instance(tmp_path, caplog): + with pytest.raises(ValueError): + UnleashClient("thisisnotavalidurl", APP_NAME, cache_directory=str(tmp_path)) - with pytest.raises(ValueError, match="must start with http:// or https://"): - unleash_client.initialize_client() - unleash_client.destroy() + unleash_client = UnleashClient(URL, APP_NAME, cache_directory=str(tmp_path)) + try: + assert "You already have" not in caplog.text + finally: + unleash_client.destroy() def test_uc_offline_starts_with_a_well_formed_url(tmp_path): @@ -877,19 +883,6 @@ def test_uc_offline_starts_with_a_well_formed_url(tmp_path): unleash_client.destroy() -def test_uc_offline_still_validates_the_url(tmp_path): - unleash_client = UnleashClient( - "placeholder", - APP_NAME, - metrics=MetricsOptions(disabled=True, disable_registration=True), - cache_directory=str(tmp_path), - ) - - with pytest.raises(ValueError, match="must start with http:// or https://"): - unleash_client.initialize_client(fetch_toggles=False) - unleash_client.destroy() - - def test_uc_with_network_error(tmp_path): unleash_client = UnleashClient( "https://this-will-never-try-to-dns-resolve.invalid/", diff --git a/tests/unit_tests/test_async_transport.py b/tests/unit_tests/test_async_transport.py index b6ca00f9..692d1f85 100644 --- a/tests/unit_tests/test_async_transport.py +++ b/tests/unit_tests/test_async_transport.py @@ -264,7 +264,17 @@ async def test_register_sends_the_payload_it_is_given(server, transport): assert json.loads(body) == {"appName": APP_NAME, "strategies": []} +@pytest.fixture +def skip_url_validation(monkeypatch): + """ + UnleashConfig rejects these urls on construction, so register() only sees + one that slipped past that check. Skip it to reach the re-raise. + """ + monkeypatch.setattr("UnleashClient.config._validate_url", lambda url: None) + + @mark.asyncio +@mark.usefixtures("skip_url_validation") @mark.parametrize( "url", ("thisisnotavalidurl", "localhost:4242/api"), ids=("no-scheme", "bad-scheme") ) @@ -278,6 +288,7 @@ async def test_register_reraises_an_unusable_url(server, build_transport, url): @mark.asyncio +@mark.usefixtures("skip_url_validation") async def test_register_reraises_a_missing_scheme_as_a_value_error(build_transport): transport = build_transport(url="thisisnotavalidurl") diff --git a/tests/unit_tests/test_config.py b/tests/unit_tests/test_config.py index 84cb0ef3..3357eb99 100644 --- a/tests/unit_tests/test_config.py +++ b/tests/unit_tests/test_config.py @@ -78,6 +78,16 @@ def test_url_is_rstripped(): assert UnleashConfig(URL, APP_NAME).url == URL +def test_url_is_validated(): + with pytest.raises(ValueError, match="must start with http:// or https://"): + UnleashConfig("thisisnotavalidurl", APP_NAME) + + +def test_url_is_validated_after_rstripping(): + with pytest.raises(ValueError, match="has no host"): + UnleashConfig("https://", APP_NAME) + + def test_jitters_are_int_coerced(): config = UnleashConfig( URL, diff --git a/tests/unit_tests/test_transport.py b/tests/unit_tests/test_transport.py index de84a1a3..df636aa3 100644 --- a/tests/unit_tests/test_transport.py +++ b/tests/unit_tests/test_transport.py @@ -233,18 +233,28 @@ def test_register_sends_the_payload_it_is_given(transport): } +@pytest.fixture +def skip_url_validation(monkeypatch): + """ + UnleashConfig rejects these urls on construction, so register() only sees + one that slipped past that check. Skip it to reach the re-raise. + """ + monkeypatch.setattr("UnleashClient.config._validate_url", lambda url: None) + + @responses.activate +@pytest.mark.usefixtures("skip_url_validation") def test_register_reraises_a_missing_schema(build_transport): transport = build_transport(url="thisisnotavalidurl") - # initialize_client() fails loudly on a malformed URL because these four - # are re-raised rather than reported as False. See test_uc_with_invalid_url, - # which only sees the ValueError that MissingSchema also subclasses. + # These four are re-raised rather than reported as False, so a url + # requests rejects still fails initialize_client() loudly. with pytest.raises(MissingSchema): transport.register({}) @responses.activate +@pytest.mark.usefixtures("skip_url_validation") def test_register_reraises_an_invalid_schema(build_transport): transport = build_transport(url="localhost:4242/api") diff --git a/v7_MIGRATION_GUIDE.md b/v7_MIGRATION_GUIDE.md index 0d86554b..7d75b2fd 100644 --- a/v7_MIGRATION_GUIDE.md +++ b/v7_MIGRATION_GUIDE.md @@ -83,7 +83,7 @@ An `Authorization` header in `HttpOptions.headers` raises a `ValueError`, in any ## The URL is validated -`initialize_client()` raises a `ValueError` if `url` is not an absolute `http://` or `https://` URL with a host. Only the shape is checked, not whether the server can be reached. +The client constructor raises a `ValueError` if `url` is not an absolute `http://` or `https://` URL with a host. Only the shape is checked, not whether the server can be reached. This applies to every client, including one that never talks to the server (`fetch_toggles=False` with registration and metrics disabled). In v6 such a client accepted any string as `url`; pass a well-formed one instead: From de6e361b624ac6cd5f24fc6425453a224c6018c0 Mon Sep 17 00:00:00 2001 From: Alexandra Szanto Date: Mon, 2 Nov 2026 14:53:55 +0100 Subject: [PATCH 6/7] refactor: rename transport to apiclient --- UnleashClient/_async_metrics.py | 10 +- ...nsport.py => _async_unleash_api_client.py} | 20 +-- UnleashClient/_feature_store.py | 2 +- UnleashClient/_metrics.py | 10 +- .../{_transport.py => _unleash_api_client.py} | 2 +- UnleashClient/clients/async_unleash_client.py | 12 +- UnleashClient/clients/unleash_client.py | 10 +- UnleashClient/connectors/_async_connector.py | 12 +- UnleashClient/connectors/polling_connector.py | 10 +- UnleashClient/errors.py | 6 +- .../clients/test_async_unleash_client.py | 8 +- .../connectors/test_async_connector.py | 12 +- .../connectors/test_polling_connector.py | 18 +-- tests/unit_tests/test_async_metrics.py | 18 +-- ...rt.py => test_async_unleash_api_client.py} | 146 +++++++++--------- tests/unit_tests/test_metrics.py | 4 +- ...ransport.py => test_unleash_api_client.py} | 98 ++++++------ 17 files changed, 200 insertions(+), 198 deletions(-) rename UnleashClient/{_async_transport.py => _async_unleash_api_client.py} (94%) rename UnleashClient/{_transport.py => _unleash_api_client.py} (99%) rename tests/unit_tests/{test_async_transport.py => test_async_unleash_api_client.py} (74%) rename tests/unit_tests/{test_transport.py => test_unleash_api_client.py} (78%) diff --git a/UnleashClient/_async_metrics.py b/UnleashClient/_async_metrics.py index 5e4d7eaf..6eb8cf20 100644 --- a/UnleashClient/_async_metrics.py +++ b/UnleashClient/_async_metrics.py @@ -5,7 +5,7 @@ from yggdrasil_engine.engine import UnleashEngine from UnleashClient._async_scheduler import _AsyncJob, _AsyncScheduler -from UnleashClient._async_transport import _AsyncTransport +from UnleashClient._async_unleash_api_client import _AsyncUnleashApiClient from UnleashClient._payloads import _build_metrics_payload from UnleashClient.config import UnleashConfig from UnleashClient.impact_metrics import ImpactMetrics @@ -23,7 +23,7 @@ class _AsyncMetricsReporter: reporter = _AsyncMetricsReporter( config=config, - transport=transport, + api_client=api_client, scheduler=scheduler, engine=engine, impact_metrics=impact_metrics, @@ -38,13 +38,13 @@ class _AsyncMetricsReporter: def __init__( self, config: UnleashConfig, - transport: _AsyncTransport, + api_client: _AsyncUnleashApiClient, scheduler: _AsyncScheduler, engine: UnleashEngine, impact_metrics: ImpactMetrics, ) -> None: self._config: UnleashConfig = config - self._transport: _AsyncTransport = transport + self._api: _AsyncUnleashApiClient = api_client self._scheduler: _AsyncScheduler = scheduler self._engine: UnleashEngine = engine self._impact_metrics: ImpactMetrics = impact_metrics @@ -75,7 +75,7 @@ async def flush(self) -> None: payload = _build_metrics_payload(self._config, bucket, impact_metrics) sent = False try: - sent = await self._transport.send_metrics(payload) + sent = await self._api.send_metrics(payload) finally: if not sent and impact_metrics: self._impact_metrics.restore(impact_metrics) diff --git a/UnleashClient/_async_transport.py b/UnleashClient/_async_unleash_api_client.py similarity index 94% rename from UnleashClient/_async_transport.py rename to UnleashClient/_async_unleash_api_client.py index 2ffec88d..c2885253 100644 --- a/UnleashClient/_async_transport.py +++ b/UnleashClient/_async_unleash_api_client.py @@ -1,4 +1,4 @@ -"""Asynchronous HTTP transport, for the async Unleash client.""" +"""Unleash API client over aiohttp, for the async Unleash client.""" import asyncio import json @@ -7,7 +7,7 @@ from UnleashClient._event_dispatcher import _EventDispatcher from UnleashClient._headers import _HeaderFactory -from UnleashClient._transport import ( +from UnleashClient._unleash_api_client import ( FetchResult, _emit_fetch_completed, _failure_status, @@ -42,8 +42,8 @@ async def _log_resp_info(resp: "aiohttp.ClientResponse") -> None: LOGGER.debug("HTTP content: %s", await resp.text()) -class _AsyncTransport: - """The asyncio twin of :class:`UnleashClient._transport._Transport`.""" +class _AsyncUnleashApiClient: + """The asyncio twin of :class:`UnleashClient._unleash_api_client._UnleashApiClient`.""" def __init__( self, @@ -82,10 +82,10 @@ def _timeout(self) -> "aiohttp.ClientTimeout": async def aclose(self) -> None: """ - Close the pooled session and put the transport out of service. + Close the pooled session and put the API client out of service. - Every later request raises :class:`AlreadyClosedError`. Closing a - transport that is already closed does nothing. + Every later request raises :class:`AlreadyClosedError`. Closing an already + closed API client does nothing. """ with self._close_lock: if self._is_closed: @@ -112,7 +112,7 @@ async def fetch_features( :param etag: the cached etag, or "" to fetch unconditionally. :param options: the polling settings - :raises AlreadyClosedError: if the transport has been closed. + :raises AlreadyClosedError: if the API client has been closed. """ self._raise_if_closed() @@ -202,7 +202,7 @@ async def register(self, payload: Dict[str, Any]) -> bool: :param payload: as built by :func:`UnleashClient._payloads._build_register_payload`. - :raises AlreadyClosedError: if the transport has been closed. + :raises AlreadyClosedError: if the API client has been closed. """ self._raise_if_closed() @@ -249,7 +249,7 @@ async def send_metrics(self, payload: Dict[str, Any]) -> bool: timeout all return False. :param payload: the metrics request body. - :raises AlreadyClosedError: if the transport has been closed. + :raises AlreadyClosedError: if the API client has been closed. :raises asyncio.CancelledError: if the send is cancelled. """ self._raise_if_closed() diff --git a/UnleashClient/_feature_store.py b/UnleashClient/_feature_store.py index efedb31e..2955efa7 100644 --- a/UnleashClient/_feature_store.py +++ b/UnleashClient/_feature_store.py @@ -31,7 +31,7 @@ class _FeatureStore: store = _FeatureStore(engine=UnleashEngine(), cache=cache, events=dispatcher) store.load_from_cache() - result = transport.fetch_features(etag=store.cached_etag, options=polling) + result = api_client.fetch_features(etag=store.cached_etag, options=polling) store.apply_fetched(raw_state=result.raw_state, etag=result.etag) """ diff --git a/UnleashClient/_metrics.py b/UnleashClient/_metrics.py index bf3c1694..aa5c5f12 100644 --- a/UnleashClient/_metrics.py +++ b/UnleashClient/_metrics.py @@ -4,7 +4,7 @@ from UnleashClient._payloads import _build_metrics_payload from UnleashClient._scheduler import _ScheduledJob, _Scheduler -from UnleashClient._transport import _Transport +from UnleashClient._unleash_api_client import _UnleashApiClient from UnleashClient.config import UnleashConfig from UnleashClient.impact_metrics import ImpactMetrics from UnleashClient.utils import LOGGER @@ -19,7 +19,7 @@ class _MetricsReporter: reporter = _MetricsReporter( config=config, - transport=transport, + api_client=api_client, scheduler=scheduler, engine=engine, impact_metrics=impact_metrics, @@ -34,13 +34,13 @@ class _MetricsReporter: def __init__( self, config: UnleashConfig, - transport: _Transport, + api_client: _UnleashApiClient, scheduler: _Scheduler, engine: UnleashEngine, impact_metrics: ImpactMetrics, ) -> None: self._config: UnleashConfig = config - self._transport: _Transport = transport + self._api: _UnleashApiClient = api_client self._scheduler: _Scheduler = scheduler self._engine: UnleashEngine = engine self._impact_metrics: ImpactMetrics = impact_metrics @@ -78,7 +78,7 @@ def flush(self) -> None: return payload = _build_metrics_payload(self._config, bucket, impact_metrics) - if not self._transport.send_metrics(payload) and impact_metrics: + if not self._api.send_metrics(payload) and impact_metrics: self._impact_metrics.restore(impact_metrics) def stop(self) -> None: diff --git a/UnleashClient/_transport.py b/UnleashClient/_unleash_api_client.py similarity index 99% rename from UnleashClient/_transport.py rename to UnleashClient/_unleash_api_client.py index 0bdda58b..d95f27c7 100644 --- a/UnleashClient/_transport.py +++ b/UnleashClient/_unleash_api_client.py @@ -84,7 +84,7 @@ def _log_resp_info(resp: Response) -> None: LOGGER.debug("HTTP content: %s", resp.text) -class _Transport: +class _UnleashApiClient: """ Sends the SDK's requests to the Unleash server: feature fetches, client registration and metrics submission. diff --git a/UnleashClient/clients/async_unleash_client.py b/UnleashClient/clients/async_unleash_client.py index 6d31a366..d07d33bd 100644 --- a/UnleashClient/clients/async_unleash_client.py +++ b/UnleashClient/clients/async_unleash_client.py @@ -12,7 +12,7 @@ from UnleashClient._async_metrics import _AsyncMetricsReporter from UnleashClient._async_scheduler import _AsyncScheduler -from UnleashClient._async_transport import _AsyncTransport +from UnleashClient._async_unleash_api_client import _AsyncUnleashApiClient from UnleashClient._context import _ContextEnricher from UnleashClient._evaluator import _Evaluator from UnleashClient._event_dispatcher import _EventDispatcher @@ -142,13 +142,13 @@ def __init__( config=self._config, events=self._event_dispatcher, ) - self._transport: _AsyncTransport = _AsyncTransport( + self._api: _AsyncUnleashApiClient = _AsyncUnleashApiClient( self._config, self._headers, self._event_dispatcher ) self._metrics_scheduler: _AsyncScheduler = _AsyncScheduler() self._metrics: _AsyncMetricsReporter = _AsyncMetricsReporter( config=self._config, - transport=self._transport, + api_client=self._api, scheduler=self._metrics_scheduler, engine=self._engine, impact_metrics=self.impact_metrics, @@ -292,7 +292,7 @@ async def initialize_client(self, fetch_toggles: bool = True) -> None: self._engine.register_custom_strategies(self._custom_strategies) if not self._config.metrics.disable_registration: - await self._transport.register( + await self._api.register( _build_register_payload(self._config, self._custom_strategies) ) @@ -301,7 +301,7 @@ async def initialize_client(self, fetch_toggles: bool = True) -> None: self._connector = _AsyncPollingConnector( store=self._store, - transport=self._transport, + api_client=self._api, options=cast(PollingOptions, self._config.mode), ) await self._connector.start() @@ -346,7 +346,7 @@ async def destroy(self) -> None: except Exception as exc: LOGGER.warning("Exception during scheduler teardown: %s", exc) - await self._transport.aclose() + await self._api.aclose() if not isinstance(self._cache, FileCache): try: diff --git a/UnleashClient/clients/unleash_client.py b/UnleashClient/clients/unleash_client.py index f9c9e4a0..2d1411d6 100644 --- a/UnleashClient/clients/unleash_client.py +++ b/UnleashClient/clients/unleash_client.py @@ -21,7 +21,7 @@ from UnleashClient._metrics import _MetricsReporter from UnleashClient._payloads import _build_register_payload from UnleashClient._scheduler import _ScheduledJob, _Scheduler -from UnleashClient._transport import _Transport +from UnleashClient._unleash_api_client import _UnleashApiClient from UnleashClient.cache import BaseCache, FileCache from UnleashClient.config import ( HttpOptions, @@ -199,13 +199,13 @@ def __init__( events=self.__events, ) - self._transport = _Transport(self._config, self._headers, self.__events) + self._api = _UnleashApiClient(self._config, self._headers, self.__events) self._scheduler = _Scheduler(scheduler, scheduler_executor) self._metrics = _MetricsReporter( config=self._config, - transport=self._transport, + api_client=self._api, scheduler=self._scheduler, engine=self._engine, impact_metrics=self.impact_metrics, @@ -297,7 +297,7 @@ def initialize_client(self, fetch_toggles: bool = True) -> None: # Register app if not self._config.metrics.disable_registration: - self._transport.register( + self._api.register( _build_register_payload(self._config, self._custom_strategies) ) mode = self._config.mode @@ -315,7 +315,7 @@ def initialize_client(self, fetch_toggles: bool = True) -> None: self.connector = PollingConnector( store=self._store, scheduler=self._scheduler, - transport=self._transport, + api_client=self._api, options=mode, ) else: diff --git a/UnleashClient/connectors/_async_connector.py b/UnleashClient/connectors/_async_connector.py index a293de2c..592e9c59 100644 --- a/UnleashClient/connectors/_async_connector.py +++ b/UnleashClient/connectors/_async_connector.py @@ -1,7 +1,7 @@ from abc import ABC, abstractmethod from UnleashClient._async_scheduler import _AsyncScheduler -from UnleashClient._async_transport import _AsyncTransport +from UnleashClient._async_unleash_api_client import _AsyncUnleashApiClient from UnleashClient._feature_store import _FeatureStore from UnleashClient.config import PollingOptions @@ -32,7 +32,7 @@ class _AsyncPollingConnector(_AsyncBaseConnector): connector = _AsyncPollingConnector( store=store, - transport=transport, + api_client=api_client, options=PollingOptions(interval=15), ) await connector.start() @@ -43,21 +43,21 @@ class _AsyncPollingConnector(_AsyncBaseConnector): def __init__( self, store: _FeatureStore, - transport: _AsyncTransport, + api_client: _AsyncUnleashApiClient, options: PollingOptions, ) -> None: """ :param store: Applies feature state to the engine and the cache. - :param transport: Performs the fetch against the Unleash server. + :param api_client: Performs the fetch against the Unleash server. :param options: How often to fetch, and which project to fetch. """ super().__init__(store) - self._transport: _AsyncTransport = transport + self._api: _AsyncUnleashApiClient = api_client self._options: PollingOptions = options self._scheduler: _AsyncScheduler = _AsyncScheduler() async def _fetch_and_load(self) -> None: - result = await self._transport.fetch_features( + result = await self._api.fetch_features( etag=self._store.cached_etag, options=self._options ) diff --git a/UnleashClient/connectors/polling_connector.py b/UnleashClient/connectors/polling_connector.py index 1bffaa66..3e7372de 100644 --- a/UnleashClient/connectors/polling_connector.py +++ b/UnleashClient/connectors/polling_connector.py @@ -1,6 +1,6 @@ from UnleashClient._feature_store import _FeatureStore from UnleashClient._scheduler import _ScheduledJob, _Scheduler -from UnleashClient._transport import _Transport +from UnleashClient._unleash_api_client import _UnleashApiClient from UnleashClient.config import PollingOptions from .base_connector import BaseConnector @@ -13,21 +13,21 @@ def __init__( self, store: _FeatureStore, scheduler: _Scheduler, - transport: _Transport, + api_client: _UnleashApiClient, options: PollingOptions, ): """ - :param transport: Performs the fetch against the Unleash server. + :param api_client: Performs the fetch against the Unleash server. :param options: How often to fetch, and which project to fetch. """ super().__init__(store) self.scheduler: _Scheduler = scheduler - self.transport: _Transport = transport + self.api_client: _UnleashApiClient = api_client self.options: PollingOptions = options self.job: _ScheduledJob = None def _fetch_and_load(self) -> None: - result = self.transport.fetch_features( + result = self.api_client.fetch_features( self._store.cached_etag, options=self.options ) diff --git a/UnleashClient/errors.py b/UnleashClient/errors.py index ad23ce67..9e3b957b 100644 --- a/UnleashClient/errors.py +++ b/UnleashClient/errors.py @@ -94,7 +94,7 @@ class TransportError(UnleashClientError): Example:: try: - result = await transport.fetch_features(options=polling) + result = await api_client.fetch_features(options=polling) except TransportError as error: LOGGER.error("Could not reach the Unleash server: %s", error) """ @@ -108,10 +108,10 @@ class AlreadyClosedError(TransportError): Example:: - await transport.aclose() + await api_client.aclose() try: - await transport.fetch_features(options=polling) + await api_client.fetch_features(options=polling) except AlreadyClosedError: ... # build a new client instead of reusing the closed one """ diff --git a/tests/unit_tests/clients/test_async_unleash_client.py b/tests/unit_tests/clients/test_async_unleash_client.py index 765bb9d9..97a7d2a2 100644 --- a/tests/unit_tests/clients/test_async_unleash_client.py +++ b/tests/unit_tests/clients/test_async_unleash_client.py @@ -376,7 +376,7 @@ def test_async_client_builds_a_metrics_reporter_over_its_own_collaborators(tmpdi client = build_async_client(tmpdir, url=URL, app_name=APP_NAME) assert client._metrics._config is client._config - assert client._metrics._transport is client._transport + assert client._metrics._api is client._api assert client._metrics._engine is client._engine assert client._metrics._impact_metrics is client.impact_metrics @@ -539,14 +539,14 @@ async def test_initializing_awaits_the_first_fetch_while_yielding_to_the_event_l client = build_running_client(mode=PollingOptions(interval=3600)) started = asyncio.Event() release = asyncio.Event() - fetch_features = client._transport.fetch_features + fetch_features = client._api.fetch_features async def delayed_fetch(**kwargs): started.set() await release.wait() return await fetch_features(**kwargs) - monkeypatch.setattr(client._transport, "fetch_features", delayed_fetch) + monkeypatch.setattr(client._api, "fetch_features", delayed_fetch) initializing = asyncio.create_task(client.initialize_client()) try: await asyncio.wait_for(started.wait(), WAIT_TIMEOUT) @@ -706,7 +706,7 @@ async def test_destroy_during_registration_leaves_nothing_polling( ): server.on("GET", FEATURES_PATH, payload=MOCK_FEATURE_RESPONSE, repeat=True) client = build_running_client(mode=PollingOptions(interval=0.01)) - client._transport.register = lambda payload: asyncio.sleep(0.05) + client._api.register = lambda payload: asyncio.sleep(0.05) initializing = asyncio.ensure_future(client.initialize_client()) await asyncio.sleep(0) diff --git a/tests/unit_tests/connectors/test_async_connector.py b/tests/unit_tests/connectors/test_async_connector.py index b97b2723..1a94edf5 100644 --- a/tests/unit_tests/connectors/test_async_connector.py +++ b/tests/unit_tests/connectors/test_async_connector.py @@ -10,7 +10,7 @@ from tests.utilities.fake_unleash_server import FakeUnleash from tests.utilities.mocks.mock_features import MOCK_FEATURE_RESPONSE from tests.utilities.testing_constants import APP_NAME, ETAG_VALUE -from UnleashClient._async_transport import _AsyncTransport +from UnleashClient._async_unleash_api_client import _AsyncUnleashApiClient from UnleashClient._event_dispatcher import _EventDispatcher from UnleashClient._feature_store import _FeatureStore from UnleashClient._headers import _HeaderFactory @@ -46,21 +46,21 @@ def _build_connector( refresh_jitter: float = None, ) -> _AsyncPollingConnector: config = UnleashConfig(server.base_url, APP_NAME, http=HttpOptions(retries=0)) - transport = _AsyncTransport(config, _HeaderFactory(config)) + api_client = _AsyncUnleashApiClient(config, _HeaderFactory(config)) connector = _AsyncPollingConnector( store=store, - transport=transport, + api_client=api_client, options=PollingOptions(interval=refresh_interval, jitter=refresh_jitter), ) - built.append((connector, transport)) + built.append((connector, api_client)) return connector try: yield _build_connector finally: - for connector, transport in built: + for connector, api_client in built: await connector.stop() - await transport.aclose() + await api_client.aclose() async def until(predicate: Callable[[], bool]) -> None: diff --git a/tests/unit_tests/connectors/test_polling_connector.py b/tests/unit_tests/connectors/test_polling_connector.py index 87eac380..6ecc3bf5 100644 --- a/tests/unit_tests/connectors/test_polling_connector.py +++ b/tests/unit_tests/connectors/test_polling_connector.py @@ -22,7 +22,7 @@ from UnleashClient._feature_store import _FeatureStore from UnleashClient._headers import _HeaderFactory from UnleashClient._scheduler import _Scheduler -from UnleashClient._transport import _Transport +from UnleashClient._unleash_api_client import _UnleashApiClient from UnleashClient.config import ( HttpOptions, IdentityOptions, @@ -36,7 +36,7 @@ FULL_FEATURE_URL = URL + FEATURES_URL -def build_transport(**kwargs) -> _Transport: +def build_api_client(**kwargs) -> _UnleashApiClient: defaults = { "identity": IdentityOptions(instance_id=INSTANCE_ID), "http": HttpOptions( @@ -48,7 +48,7 @@ def build_transport(**kwargs) -> _Transport: } defaults.update(kwargs) config = UnleashConfig(URL, APP_NAME, **defaults) - return _Transport(config, _HeaderFactory(config)) + return _UnleashApiClient(config, _HeaderFactory(config)) @responses.activate @@ -67,7 +67,7 @@ def test_polling_connector_fetch_and_load(cache_empty): connector = PollingConnector( store=_FeatureStore(engine=engine, cache=temp_cache), scheduler=scheduler, - transport=build_transport(), + api_client=build_api_client(), options=PollingOptions(), ) @@ -89,7 +89,7 @@ def test_polling_connector_fetch_and_load_project(cache_empty): connector = PollingConnector( store=_FeatureStore(engine=engine, cache=temp_cache), scheduler=scheduler, - transport=build_transport(), + api_client=build_api_client(), options=PollingOptions(project_name=PROJECT_NAME), ) @@ -110,7 +110,7 @@ def test_polling_connector_fetch_and_load_failure(cache_empty): connector = PollingConnector( store=_FeatureStore(engine=engine, cache=temp_cache), scheduler=scheduler, - transport=build_transport(), + api_client=build_api_client(), options=PollingOptions(), ) @@ -141,7 +141,7 @@ def test_polling_connector_emits_fetched_and_ready( connector = PollingConnector( store=_FeatureStore(engine=engine, cache=cache_empty, events=dispatcher), scheduler=scheduler, - transport=build_transport(), + api_client=build_api_client(), # Huge refresh interval to avoid any polling during the test. That # way we avoid having to call _fetch_and_load() directly and can test the start/stop behavior. options=PollingOptions(interval=10), @@ -168,7 +168,7 @@ def test_polling_connector_emits_ready_once_across_polls( connector = PollingConnector( store=_FeatureStore(engine=engine, cache=cache_empty, events=dispatcher), scheduler=scheduler, - transport=build_transport(), + api_client=build_api_client(), options=PollingOptions(), ) @@ -201,7 +201,7 @@ def test_polling_connector_start_stop(cache_empty): connector = PollingConnector( store=_FeatureStore(engine=engine, cache=temp_cache), scheduler=scheduler, - transport=build_transport(), + api_client=build_api_client(), options=PollingOptions(interval=1), ) diff --git a/tests/unit_tests/test_async_metrics.py b/tests/unit_tests/test_async_metrics.py index 8b40b5f1..9b05823f 100644 --- a/tests/unit_tests/test_async_metrics.py +++ b/tests/unit_tests/test_async_metrics.py @@ -10,7 +10,7 @@ from tests.utilities.fake_unleash_server import FakeUnleash from UnleashClient._async_metrics import _AsyncMetricsReporter from UnleashClient._async_scheduler import _AsyncJob, _AsyncJobFn, _AsyncScheduler -from UnleashClient._async_transport import _AsyncTransport +from UnleashClient._async_unleash_api_client import _AsyncUnleashApiClient from UnleashClient._headers import _HeaderFactory from UnleashClient.config import IdentityOptions, MetricsOptions, UnleashConfig from UnleashClient.constants import CLIENT_SPEC_VERSION, METRICS_URL @@ -18,7 +18,7 @@ APP_NAME = "pytest" -# The fake server mounts Unleash under /api, so the async transport builds the URL +# The fake server mounts Unleash under /api, so the async API client builds the URL # shape it builds against a deployment. API_PREFIX = "/api" METRICS_PATH = API_PREFIX + METRICS_URL @@ -121,7 +121,7 @@ def _build_reporter(impact_metrics=None, **kwargs) -> _AsyncMetricsReporter: engine = UnleashEngine() reporter = _AsyncMetricsReporter( config=config, - transport=_AsyncTransport(config, _HeaderFactory(config)), + api_client=_AsyncUnleashApiClient(config, _HeaderFactory(config)), scheduler=RecordingAsyncScheduler(), engine=engine, impact_metrics=( @@ -138,7 +138,7 @@ def _build_reporter(impact_metrics=None, **kwargs) -> _AsyncMetricsReporter: finally: for reporter in built: await reporter._scheduler.shutdown() - await reporter._transport.aclose() + await reporter._api.aclose() @pytest_asyncio.fixture async def reporter( @@ -225,11 +225,11 @@ async def test_flush_omits_sdk_flavor_when_unset(self, server, reporter): assert "sdkFlavorVersion" not in body @mark.asyncio - async def test_the_flush_goes_through_the_async_transport(self, reporter): - # The flush runs on the client's loop, so a blocking transport would hold it up for + async def test_the_flush_goes_through_the_async_api_client(self, reporter): + # The flush runs on the client's loop, so a blocking API client would hold it up for # the length of every POST. - assert isinstance(reporter._transport, _AsyncTransport) - assert asyncio.iscoroutinefunction(reporter._transport.send_metrics) + assert isinstance(reporter._api, _AsyncUnleashApiClient) + assert asyncio.iscoroutinefunction(reporter._api.send_metrics) assert asyncio.iscoroutinefunction(reporter.flush) # start and stop @@ -309,7 +309,7 @@ async def test_constructing_the_reporter_registers_no_job_and_opens_no_session( reporter = build_reporter() assert reporter._scheduler.registered == [] - assert reporter._transport._session is None + assert reporter._api._session is None # impact metrics diff --git a/tests/unit_tests/test_async_transport.py b/tests/unit_tests/test_async_unleash_api_client.py similarity index 74% rename from tests/unit_tests/test_async_transport.py rename to tests/unit_tests/test_async_unleash_api_client.py index 692d1f85..6c9a873a 100644 --- a/tests/unit_tests/test_async_transport.py +++ b/tests/unit_tests/test_async_unleash_api_client.py @@ -22,7 +22,7 @@ REQUEST_RETRIES, REQUEST_TIMEOUT, ) -from UnleashClient._async_transport import _AsyncTransport +from UnleashClient._async_unleash_api_client import _AsyncUnleashApiClient from UnleashClient._headers import _HeaderFactory from UnleashClient.config import ( HttpOptions, @@ -58,14 +58,14 @@ async def server(): @pytest_asyncio.fixture -async def build_transport(server: FakeUnleash): +async def build_api_client(server: FakeUnleash): """ Factory pointed at the fake server. Keyword arguments override the defaults - on the config. Every transport it builds is closed on teardown. + on the config. Every API client it builds is closed on teardown. """ built = [] - def _build_transport(events=None, **kwargs) -> _AsyncTransport: + def _build_api_client(events=None, **kwargs) -> _AsyncUnleashApiClient: defaults = { "url": server.base_url, "app_name": APP_NAME, @@ -78,21 +78,23 @@ def _build_transport(events=None, **kwargs) -> _AsyncTransport: } defaults.update(kwargs) config = UnleashConfig(**defaults) - transport = _AsyncTransport(config, _HeaderFactory(config), events) - built.append(transport) - return transport + api_client = _AsyncUnleashApiClient(config, _HeaderFactory(config), events) + built.append(api_client) + return api_client try: - yield _build_transport + yield _build_api_client finally: - for transport in built: - await transport.aclose() + for api_client in built: + await api_client.aclose() @pytest_asyncio.fixture -async def transport(build_transport: Callable[..., _AsyncTransport]) -> _AsyncTransport: - """The transport the tests that need no config override share.""" - return build_transport() +async def api_client( + build_api_client: Callable[..., _AsyncUnleashApiClient], +) -> _AsyncUnleashApiClient: + """The API client the tests that need no config override share.""" + return build_api_client() @mark.asyncio @@ -111,7 +113,7 @@ async def transport(build_transport: Callable[..., _AsyncTransport]) -> _AsyncTr ), ) async def test_fetch_features( - server, transport, response, status, call_count, expected + server, api_client, response, status, call_count, expected ): server.on( "GET", @@ -122,7 +124,7 @@ async def test_fetch_features( repeat=True, ) - result = await transport.fetch_features(options=PollingOptions()) + result = await api_client.fetch_features(options=PollingOptions()) assert len(server.calls("GET", FEATURES_PATH)) == call_count assert expected(result.raw_state) @@ -130,9 +132,9 @@ async def test_fetch_features( @mark.asyncio async def test_fetch_features_sends_the_project_as_a_query_parameter( - server, build_transport + server, build_api_client ): - transport = build_transport() + api_client = build_api_client() server.on( "GET", FEATURES_PATH, @@ -141,7 +143,7 @@ async def test_fetch_features_sends_the_project_as_a_query_parameter( headers={"etag": ETAG_VALUE}, ) - result = await transport.fetch_features( + result = await api_client.fetch_features( options=PollingOptions(project_name=PROJECT_NAME) ) @@ -153,8 +155,8 @@ async def test_fetch_features_sends_the_project_as_a_query_parameter( @mark.asyncio -async def test_fetch_features_drops_the_etag_on_failure(server, build_transport): - transport = build_transport() +async def test_fetch_features_drops_the_etag_on_failure(server, build_api_client): + api_client = build_api_client() server.on( "GET", FEATURES_PATH, @@ -164,7 +166,7 @@ async def test_fetch_features_drops_the_etag_on_failure(server, build_transport) repeat=True, ) - result = await transport.fetch_features( + result = await api_client.fetch_features( options=PollingOptions(project_name=PROJECT_NAME) ) @@ -175,12 +177,12 @@ async def test_fetch_features_drops_the_etag_on_failure(server, build_transport) @mark.asyncio async def test_fetch_features_sends_if_none_match_when_an_etag_is_known( - server, build_transport + server, build_api_client ): - transport = build_transport() + api_client = build_api_client() server.on("GET", FEATURES_PATH, status=304, headers={"etag": ETAG_VALUE}) - result = await transport.fetch_features( + result = await api_client.fetch_features( ETAG_VALUE, options=PollingOptions(project_name=PROJECT_NAME) ) @@ -193,8 +195,8 @@ async def test_fetch_features_sends_if_none_match_when_an_etag_is_known( @mark.asyncio -async def test_fetch_features_retries_and_recovers(server, build_transport): - transport = build_transport() +async def test_fetch_features_retries_and_recovers(server, build_api_client): + api_client = build_api_client() server.on("GET", FEATURES_PATH, status=500, payload={}) server.on( "GET", @@ -204,7 +206,7 @@ async def test_fetch_features_retries_and_recovers(server, build_transport): headers={"etag": ETAG_VALUE}, ) - result = await transport.fetch_features( + result = await api_client.fetch_features( ETAG_VALUE, options=PollingOptions(project_name=PROJECT_NAME) ) @@ -215,9 +217,9 @@ async def test_fetch_features_retries_and_recovers(server, build_transport): @mark.asyncio async def test_fetch_features_retries_and_recovers_from_a_connection_error( - server, build_transport + server, build_api_client ): - transport = build_transport() + api_client = build_api_client() server.on("GET", FEATURES_PATH, disconnect=True) server.on( "GET", @@ -227,7 +229,7 @@ async def test_fetch_features_retries_and_recovers_from_a_connection_error( headers={"etag": ETAG_VALUE}, ) - result = await transport.fetch_features( + result = await api_client.fetch_features( ETAG_VALUE, options=PollingOptions(project_name=PROJECT_NAME) ) @@ -245,20 +247,20 @@ async def test_fetch_features_retries_and_recovers_from_a_connection_error( param({"disconnect": True}, False, id="connection-error"), ), ) -async def test_register(server, transport, reply, expected): +async def test_register(server, api_client, reply, expected): server.on("POST", REGISTER_PATH, **reply) - result = await transport.register({"appName": APP_NAME}) + result = await api_client.register({"appName": APP_NAME}) assert len(server.calls("POST", REGISTER_PATH)) == 1 assert result is expected @mark.asyncio -async def test_register_sends_the_payload_it_is_given(server, transport): +async def test_register_sends_the_payload_it_is_given(server, api_client): server.on("POST", REGISTER_PATH, status=202, payload={}) - await transport.register({"appName": APP_NAME, "strategies": []}) + await api_client.register({"appName": APP_NAME, "strategies": []}) body = server.calls("POST", REGISTER_PATH)[0].body assert json.loads(body) == {"appName": APP_NAME, "strategies": []} @@ -278,22 +280,22 @@ def skip_url_validation(monkeypatch): @mark.parametrize( "url", ("thisisnotavalidurl", "localhost:4242/api"), ids=("no-scheme", "bad-scheme") ) -async def test_register_reraises_an_unusable_url(server, build_transport, url): - transport = build_transport(url=url) +async def test_register_reraises_an_unusable_url(server, build_api_client, url): + api_client = build_api_client(url=url) with pytest.raises(aiohttp.ClientError): - await transport.register({}) + await api_client.register({}) assert server.requests == [] @mark.asyncio @mark.usefixtures("skip_url_validation") -async def test_register_reraises_a_missing_scheme_as_a_value_error(build_transport): - transport = build_transport(url="thisisnotavalidurl") +async def test_register_reraises_a_missing_scheme_as_a_value_error(build_api_client): + api_client = build_api_client(url="thisisnotavalidurl") with pytest.raises(ValueError): - await transport.register({}) + await api_client.register({}) @mark.asyncio @@ -306,10 +308,10 @@ async def test_register_reraises_a_missing_scheme_as_a_value_error(build_transpo param({"disconnect": True}, False, id="connection-error"), ), ) -async def test_send_metrics(server, transport, reply, expected): +async def test_send_metrics(server, api_client, reply, expected): server.on("POST", METRICS_PATH, **reply) - result = await transport.send_metrics(MOCK_METRICS_REQUEST) + result = await api_client.send_metrics(MOCK_METRICS_REQUEST) calls = server.calls("POST", METRICS_PATH) assert len(calls) == 1 @@ -321,21 +323,21 @@ async def test_send_metrics(server, transport, reply, expected): @mark.asyncio async def test_send_metrics_returns_false_when_the_request_times_out( - server, build_transport + server, build_api_client ): - transport = build_transport(http=HttpOptions(timeout=0.1)) + api_client = build_api_client(http=HttpOptions(timeout=0.1)) server.on("POST", METRICS_PATH, hang=True) - result = await transport.send_metrics(MOCK_METRICS_REQUEST) + result = await api_client.send_metrics(MOCK_METRICS_REQUEST) assert len(server.calls("POST", METRICS_PATH)) == 1 assert result is False @mark.asyncio -async def test_send_metrics_raises_when_cancelled(server, transport): +async def test_send_metrics_raises_when_cancelled(server, api_client): server.on("POST", METRICS_PATH, hang=True) - send = asyncio.create_task(transport.send_metrics(MOCK_METRICS_REQUEST)) + send = asyncio.create_task(api_client.send_metrics(MOCK_METRICS_REQUEST)) while not server.calls("POST", METRICS_PATH): await asyncio.sleep(0.01) @@ -347,16 +349,16 @@ async def test_send_metrics_raises_when_cancelled(server, transport): @mark.asyncio async def test_every_endpoint_gets_its_headers_from_the_factory( - server, build_transport + server, build_api_client ): - transport = build_transport(metrics=MetricsOptions(interval=2)) + api_client = build_api_client(metrics=MetricsOptions(interval=2)) server.on("GET", FEATURES_PATH, status=200, payload=MOCK_FEATURE_RESPONSE) server.on("POST", REGISTER_PATH, status=202, payload={}) server.on("POST", METRICS_PATH, status=202, payload={}) - await transport.fetch_features(options=PollingOptions(interval=1)) - await transport.register({}) - await transport.send_metrics(MOCK_METRICS_REQUEST) + await api_client.fetch_features(options=PollingOptions(interval=1)) + await api_client.register({}) + await api_client.send_metrics(MOCK_METRICS_REQUEST) fetch = server.calls("GET", FEATURES_PATH)[0].headers register = server.calls("POST", REGISTER_PATH)[0].headers @@ -371,15 +373,15 @@ async def test_every_endpoint_gets_its_headers_from_the_factory( @mark.asyncio -async def test_the_session_is_reused_across_requests(server, transport): +async def test_the_session_is_reused_across_requests(server, api_client): server.on("POST", METRICS_PATH, status=202, payload={}, repeat=True) - await transport.send_metrics(MOCK_METRICS_REQUEST) - first = transport._session - await transport.send_metrics(MOCK_METRICS_REQUEST) + await api_client.send_metrics(MOCK_METRICS_REQUEST) + first = api_client._session + await api_client.send_metrics(MOCK_METRICS_REQUEST) assert first is not None - assert transport._session is first + assert api_client._session is first @mark.asyncio @@ -387,29 +389,29 @@ async def test_the_session_is_reused_across_requests(server, transport): "call", ( param( - lambda transport: transport.fetch_features(options=PollingOptions()), + lambda api_client: api_client.fetch_features(options=PollingOptions()), id="fetch_features", ), - param(lambda transport: transport.register({}), id="register"), + param(lambda api_client: api_client.register({}), id="register"), param( - lambda transport: transport.send_metrics(MOCK_METRICS_REQUEST), + lambda api_client: api_client.send_metrics(MOCK_METRICS_REQUEST), id="send_metrics", ), ), ) -async def test_a_closed_transport_refuses_every_request(server, transport, call): - await transport.aclose() +async def test_a_closed_api_client_refuses_every_request(server, api_client, call): + await api_client.aclose() with pytest.raises(AlreadyClosedError): - await call(transport) + await call(api_client) assert server.requests == [] @mark.asyncio -async def test_closing_a_closed_transport_does_nothing(transport): - await transport.aclose() - await transport.aclose() +async def test_closing_a_closed_api_client_does_nothing(api_client): + await api_client.aclose() + await api_client.aclose() class RecordingEvents: @@ -448,31 +450,31 @@ def fetch_outcomes(events: RecordingEvents): ), ) async def test_fetch_features_reports_how_the_fetch_ended( - server, build_transport, status, payload, expected + server, build_api_client, status, payload, expected ): server.on("GET", FEATURES_PATH, status=status, payload=payload, repeat=True) events = RecordingEvents() - await build_transport(events=events).fetch_features(options=PollingOptions()) + await build_api_client(events=events).fetch_features(options=PollingOptions()) assert fetch_outcomes(events) == [expected] @mark.asyncio async def test_fetch_features_reports_unavailable_when_no_response_arrives( - server, build_transport + server, build_api_client ): server.on("GET", FEATURES_PATH, disconnect=True, repeat=True) events = RecordingEvents() - await build_transport(events=events).fetch_features(options=PollingOptions()) + await build_api_client(events=events).fetch_features(options=PollingOptions()) assert fetch_outcomes(events) == [FetchStatus.UNAVAILABLE] @mark.asyncio async def test_fetch_features_reports_unavailable_when_the_body_times_out( - server, build_transport, monkeypatch + server, build_api_client, monkeypatch ): # A 200 status line arrives but its body never does. That is the server # being unreachable, not a bad response. @@ -484,6 +486,6 @@ async def body_times_out(_response): monkeypatch.setattr(aiohttp.ClientResponse, "text", body_times_out) events = RecordingEvents() - await build_transport(events=events).fetch_features(options=PollingOptions()) + await build_api_client(events=events).fetch_features(options=PollingOptions()) assert fetch_outcomes(events) == [FetchStatus.UNAVAILABLE] diff --git a/tests/unit_tests/test_metrics.py b/tests/unit_tests/test_metrics.py index c6d710ac..5dcc800e 100644 --- a/tests/unit_tests/test_metrics.py +++ b/tests/unit_tests/test_metrics.py @@ -7,7 +7,7 @@ from UnleashClient._headers import _HeaderFactory from UnleashClient._metrics import _MetricsReporter from UnleashClient._scheduler import _Scheduler -from UnleashClient._transport import _Transport +from UnleashClient._unleash_api_client import _UnleashApiClient from UnleashClient.config import IdentityOptions, MetricsOptions, UnleashConfig from UnleashClient.constants import CLIENT_SPEC_VERSION, METRICS_URL from UnleashClient.impact_metrics import ImpactMetrics @@ -72,7 +72,7 @@ def build_sync_reporter( engine = UnleashEngine() return _MetricsReporter( config=config, - transport=_Transport(config, _HeaderFactory(config)), + api_client=_UnleashApiClient(config, _HeaderFactory(config)), scheduler=scheduler if scheduler is not None else _Scheduler(), engine=engine, impact_metrics=( diff --git a/tests/unit_tests/test_transport.py b/tests/unit_tests/test_unleash_api_client.py similarity index 78% rename from tests/unit_tests/test_transport.py rename to tests/unit_tests/test_unleash_api_client.py index df636aa3..c556b005 100644 --- a/tests/unit_tests/test_transport.py +++ b/tests/unit_tests/test_unleash_api_client.py @@ -24,7 +24,7 @@ URL, ) from UnleashClient._headers import _HeaderFactory -from UnleashClient._transport import _Transport +from UnleashClient._unleash_api_client import _UnleashApiClient from UnleashClient.config import ( HttpOptions, IdentityOptions, @@ -46,10 +46,10 @@ @pytest.fixture -def build_transport(): +def build_api_client(): """Factory. Keyword arguments override the defaults on the config.""" - def _build_transport(events=None, **kwargs) -> _Transport: + def _build_api_client(events=None, **kwargs) -> _UnleashApiClient: defaults = { "url": URL, "app_name": APP_NAME, @@ -63,14 +63,14 @@ def _build_transport(events=None, **kwargs) -> _Transport: } defaults.update(kwargs) config = UnleashConfig(**defaults) - return _Transport(config, _HeaderFactory(config), events) + return _UnleashApiClient(config, _HeaderFactory(config), events) - return _build_transport + return _build_api_client @pytest.fixture -def transport(build_transport) -> _Transport: - return build_transport() +def api_client(build_api_client) -> _UnleashApiClient: + return build_api_client() # fetch_features @@ -91,7 +91,7 @@ def transport(build_transport) -> _Transport: param({}, 500, 4, lambda result: not result, id="failure"), ), ) -def test_fetch_features(transport, response, status, calls, expected): +def test_fetch_features(api_client, response, status, calls, expected): responses.add( responses.GET, FULL_FEATURE_URL, @@ -100,7 +100,7 @@ def test_fetch_features(transport, response, status, calls, expected): headers={"etag": ETAG_VALUE}, ) - result = transport.fetch_features(options=PollingOptions()) + result = api_client.fetch_features(options=PollingOptions()) # 4 calls on a 500 is 1 + REQUEST_RETRIES: the mounted HTTPAdapter/Retry # has to survive every move of this code. @@ -109,7 +109,7 @@ def test_fetch_features(transport, response, status, calls, expected): @responses.activate -def test_fetch_features_sends_the_project_as_a_query_parameter(build_transport): +def test_fetch_features_sends_the_project_as_a_query_parameter(build_api_client): responses.add( responses.GET, PROJECT_URL, @@ -118,7 +118,7 @@ def test_fetch_features_sends_the_project_as_a_query_parameter(build_transport): headers={"etag": ETAG_VALUE}, ) - result = build_transport().fetch_features( + result = build_api_client().fetch_features( options=PollingOptions(project_name=PROJECT_NAME) ) @@ -128,12 +128,12 @@ def test_fetch_features_sends_the_project_as_a_query_parameter(build_transport): @responses.activate -def test_fetch_features_drops_the_etag_on_failure(build_transport): +def test_fetch_features_drops_the_etag_on_failure(build_api_client): responses.add( responses.GET, PROJECT_URL, json={}, status=500, headers={"etag": ETAG_VALUE} ) - result = build_transport().fetch_features( + result = build_api_client().fetch_features( options=PollingOptions(project_name=PROJECT_NAME) ) @@ -143,10 +143,10 @@ def test_fetch_features_drops_the_etag_on_failure(build_transport): @responses.activate -def test_fetch_features_sends_if_none_match_when_an_etag_is_known(build_transport): +def test_fetch_features_sends_if_none_match_when_an_etag_is_known(build_api_client): responses.add(responses.GET, PROJECT_URL, status=304, headers={"etag": ETAG_VALUE}) - result = build_transport().fetch_features( + result = build_api_client().fetch_features( ETAG_VALUE, options=PollingOptions(project_name=PROJECT_NAME) ) @@ -159,7 +159,7 @@ def test_fetch_features_sends_if_none_match_when_an_etag_is_known(build_transpor @responses.activate -def test_fetch_features_retries_and_recovers(build_transport): +def test_fetch_features_retries_and_recovers(build_api_client): responses.add(responses.GET, PROJECT_URL, json={}, status=500) responses.add( responses.GET, @@ -169,7 +169,7 @@ def test_fetch_features_retries_and_recovers(build_transport): headers={"etag": ETAG_VALUE}, ) - result = build_transport().fetch_features( + result = build_api_client().fetch_features( ETAG_VALUE, options=PollingOptions(project_name=PROJECT_NAME) ) @@ -179,14 +179,14 @@ def test_fetch_features_retries_and_recovers(build_transport): @responses.activate -def test_fetch_features_swallows_a_bad_custom_option(build_transport): +def test_fetch_features_swallows_a_bad_custom_option(build_api_client): responses.add( responses.GET, FULL_FEATURE_URL, json=MOCK_FEATURE_RESPONSE, status=200 ) - # `timeout` collides with the keyword the transport already passes. It is a + # `timeout` collides with the keyword the API client already passes. It is a # TypeError, not a RequestException, and fetch_features swallows everything. - result = build_transport(http=HttpOptions(options={"timeout": 5})).fetch_features( + result = build_api_client(http=HttpOptions(options={"timeout": 5})).fetch_features( options=PollingOptions() ) @@ -211,10 +211,10 @@ def test_fetch_features_swallows_a_bad_custom_option(build_transport): ), ), ) -def test_register(transport, payload, status, expected): +def test_register(api_client, payload, status, expected): responses.add(responses.POST, FULL_REGISTER_URL, **payload, status=status) - result = transport.register({"appName": APP_NAME}) + result = api_client.register({"appName": APP_NAME}) # No retry adapter on this one, unlike fetch_features. assert len(responses.calls) == 1 @@ -222,10 +222,10 @@ def test_register(transport, payload, status, expected): @responses.activate -def test_register_sends_the_payload_it_is_given(transport): +def test_register_sends_the_payload_it_is_given(api_client): responses.add(responses.POST, FULL_REGISTER_URL, json={}, status=202) - transport.register({"appName": APP_NAME, "strategies": []}) + api_client.register({"appName": APP_NAME, "strategies": []}) assert json.loads(responses.calls[0].request.body) == { "appName": APP_NAME, @@ -244,32 +244,32 @@ def skip_url_validation(monkeypatch): @responses.activate @pytest.mark.usefixtures("skip_url_validation") -def test_register_reraises_a_missing_schema(build_transport): - transport = build_transport(url="thisisnotavalidurl") +def test_register_reraises_a_missing_schema(build_api_client): + api_client = build_api_client(url="thisisnotavalidurl") # These four are re-raised rather than reported as False, so a url # requests rejects still fails initialize_client() loudly. with pytest.raises(MissingSchema): - transport.register({}) + api_client.register({}) @responses.activate @pytest.mark.usefixtures("skip_url_validation") -def test_register_reraises_an_invalid_schema(build_transport): - transport = build_transport(url="localhost:4242/api") +def test_register_reraises_an_invalid_schema(build_api_client): + api_client = build_api_client(url="localhost:4242/api") with pytest.raises(InvalidSchema): - transport.register({}) + api_client.register({}) @responses.activate -def test_register_lets_a_bad_custom_option_escape(build_transport): +def test_register_lets_a_bad_custom_option_escape(build_api_client): responses.add(responses.POST, FULL_REGISTER_URL, json={}, status=202) # A TypeError is not a RequestException, so unlike fetch_features this # propagates and fails initialize_client(). with pytest.raises(TypeError): - build_transport(http=HttpOptions(options={"timeout": 5})).register({}) + build_api_client(http=HttpOptions(options={"timeout": 5})).register({}) # send_metrics @@ -290,10 +290,10 @@ def test_register_lets_a_bad_custom_option_escape(build_transport): ), ), ) -def test_send_metrics(transport, payload, status, expected): +def test_send_metrics(api_client, payload, status, expected): responses.add(responses.POST, FULL_METRICS_URL, **payload, status=status) - result = transport.send_metrics(MOCK_METRICS_REQUEST) + result = api_client.send_metrics(MOCK_METRICS_REQUEST) request = json.loads(responses.calls[0].request.body) @@ -304,25 +304,25 @@ def test_send_metrics(transport, payload, status, expected): @responses.activate -def test_send_metrics_lets_a_bad_custom_option_escape(build_transport): +def test_send_metrics_lets_a_bad_custom_option_escape(build_api_client): responses.add(responses.POST, FULL_METRICS_URL, json={}, status=202) with pytest.raises(TypeError): - build_transport(http=HttpOptions(options={"timeout": 5})).send_metrics({}) + build_api_client(http=HttpOptions(options={"timeout": 5})).send_metrics({}) @responses.activate -def test_every_endpoint_gets_its_headers_from_the_factory(build_transport): +def test_every_endpoint_gets_its_headers_from_the_factory(build_api_client): responses.add( responses.GET, FULL_FEATURE_URL, json=MOCK_FEATURE_RESPONSE, status=200 ) responses.add(responses.POST, FULL_REGISTER_URL, json={}, status=202) responses.add(responses.POST, FULL_METRICS_URL, json={}, status=202) - transport = build_transport(metrics=MetricsOptions(interval=2)) + api_client = build_api_client(metrics=MetricsOptions(interval=2)) - transport.fetch_features(options=PollingOptions(interval=1)) - transport.register({}) - transport.send_metrics(MOCK_METRICS_REQUEST) + api_client.fetch_features(options=PollingOptions(interval=1)) + api_client.register({}) + api_client.send_metrics(MOCK_METRICS_REQUEST) fetch, register, metrics = (call.request.headers for call in responses.calls) # The custom header and the identification pair reach all three... @@ -371,40 +371,40 @@ def fetch_outcomes(events: RecordingEvents): ), ) def test_fetch_features_reports_how_the_fetch_ended( - build_transport, status, body, expected + build_api_client, status, body, expected ): responses.add(responses.GET, FULL_FEATURE_URL, json=body, status=status) events = RecordingEvents() - build_transport(events=events).fetch_features(options=PollingOptions()) + build_api_client(events=events).fetch_features(options=PollingOptions()) assert fetch_outcomes(events) == [expected] @responses.activate -def test_fetch_features_reports_an_empty_200_as_invalid(build_transport): +def test_fetch_features_reports_an_empty_200_as_invalid(build_api_client): responses.add(responses.GET, FULL_FEATURE_URL, body="", status=200) events = RecordingEvents() - build_transport(events=events).fetch_features(options=PollingOptions()) + build_api_client(events=events).fetch_features(options=PollingOptions()) assert fetch_outcomes(events) == [FetchStatus.INVALID_RESPONSE] @responses.activate def test_fetch_features_reports_unavailable_when_no_response_arrives( - build_transport, + build_api_client, ): responses.add(responses.GET, FULL_FEATURE_URL, body=ConnectionError("refused")) events = RecordingEvents() - build_transport(events=events).fetch_features(options=PollingOptions()) + build_api_client(events=events).fetch_features(options=PollingOptions()) assert fetch_outcomes(events) == [FetchStatus.UNAVAILABLE] @responses.activate -def test_fetch_features_survives_a_listener_that_raises(build_transport): +def test_fetch_features_survives_a_listener_that_raises(build_api_client): responses.add( responses.GET, FULL_FEATURE_URL, json=MOCK_FEATURE_RESPONSE, status=200 ) @@ -413,7 +413,7 @@ class BrokenEvents: def emit_event(self, event): raise RuntimeError("listener is broken") - result = build_transport(events=BrokenEvents()).fetch_features( + result = build_api_client(events=BrokenEvents()).fetch_features( options=PollingOptions() ) From 1abab9af4c83a8a805af4f773fa19513cc2082dd Mon Sep 17 00:00:00 2001 From: Alexandra Szanto Date: Tue, 3 Nov 2026 13:17:55 +0100 Subject: [PATCH 7/7] refactor: default transport --- UnleashClient/_transport.py | 108 +++++++++++++ UnleashClient/_unleash_api_client.py | 90 ++++++----- tests/unit_tests/test_transport.py | 158 ++++++++++++++++++++ tests/unit_tests/test_unleash_api_client.py | 61 ++++++++ 4 files changed, 377 insertions(+), 40 deletions(-) create mode 100644 UnleashClient/_transport.py create mode 100644 tests/unit_tests/test_transport.py diff --git a/UnleashClient/_transport.py b/UnleashClient/_transport.py new file mode 100644 index 00000000..6bc9c2ce --- /dev/null +++ b/UnleashClient/_transport.py @@ -0,0 +1,108 @@ +""" +HTTP layer under the Unleash API client: sends one request and returns the +response. +""" + +from dataclasses import dataclass, field +from typing import Any, Mapping, Optional, Protocol, runtime_checkable + +import requests +from requests.adapters import HTTPAdapter +from urllib3 import Retry + +from UnleashClient.constants import REQUEST_TIMEOUT + + +@dataclass(frozen=True, kw_only=True) +class Request: + """ + HTTP request. + + :param method: ``"GET"`` or ``"POST"``. + :param url: Absolute url, without the query string. + :param headers: Headers to send. + :param params: Query string parameters. + :param body: Request body, or None to send none. + """ + + method: str + url: str + headers: Mapping[str, str] + params: Mapping[str, str] = field(default_factory=dict) + body: Optional[bytes] = None + + +@dataclass(frozen=True, kw_only=True) +class Response: + """ + HTTP response. Every status is a response, not an error: deciding what + a status means is the API client's job. + + :param status: HTTP status code. + :param headers: Response headers, looked up case-insensitively. + :param body: Raw response body. + """ + + status: int + headers: Mapping[str, str] + body: bytes + + +@runtime_checkable +class Transport(Protocol): + """Sends requests for the sync client.""" + + def execute(self, request: Request) -> Response: + """ + Send ``request`` once and return the response, whatever its status. + + :raises Exception: only when no response arrived, e.g. a connection + error or a timeout. + """ + + +class RequestsTransport: + """ + Sends requests with ``requests``, on a fresh session for every request. + + :param timeout: Seconds to wait for the connection and for each read. + :param _retries: GET requests are retried this many times on a connection + error or a 500, 502 or 504, by urllib3. + :param _request_kwargs: extra keyword arguments for every ``requests`` call. + """ + + def __init__( + self, + *, + timeout: float = REQUEST_TIMEOUT, + # these two are are going away too + _retries: Optional[int] = None, + _request_kwargs: Optional[Mapping[str, Any]] = None, + ) -> None: + self._timeout = timeout + self._retries = _retries + self._request_kwargs = dict(_request_kwargs or {}) + + def execute(self, request: Request) -> Response: + kwargs: dict = {"headers": dict(request.headers), "timeout": self._timeout} + if request.params: + kwargs["params"] = dict(request.params) + if request.body is not None: + kwargs["data"] = request.body + + with requests.Session() as session: + if request.method == "GET" and self._retries is not None: + adapter = HTTPAdapter( + max_retries=Retry( + total=self._retries, status_forcelist=[500, 502, 504] + ) + ) + session.mount("https://", adapter) + session.mount("http://", adapter) + + resp = session.request( + request.method, request.url, **kwargs, **self._request_kwargs + ) + return Response( + status=resp.status_code, headers=resp.headers, body=resp.content + ) diff --git a/UnleashClient/_unleash_api_client.py b/UnleashClient/_unleash_api_client.py index d95f27c7..73beacf8 100644 --- a/UnleashClient/_unleash_api_client.py +++ b/UnleashClient/_unleash_api_client.py @@ -3,13 +3,11 @@ from typing import Any, Dict, NamedTuple, Optional, Tuple import requests -from requests import Response -from requests.adapters import HTTPAdapter from requests.exceptions import InvalidHeader, InvalidSchema, InvalidURL, MissingSchema -from urllib3 import Retry from UnleashClient._event_dispatcher import _EventDispatcher from UnleashClient._headers import _HeaderFactory +from UnleashClient._transport import Request, RequestsTransport, Response, Transport from UnleashClient.config import PollingOptions, UnleashConfig from UnleashClient.constants import FEATURES_URL, METRICS_URL, REGISTER_URL from UnleashClient.events import ( @@ -78,19 +76,27 @@ def _normalized_url(url: str, path: str) -> str: return f"{url}{path}" +def _text(resp: Response) -> str: + return resp.body.decode("utf-8", errors="replace") + + def _log_resp_info(resp: Response) -> None: - LOGGER.debug("HTTP status code: %s", resp.status_code) + LOGGER.debug("HTTP status code: %s", resp.status) LOGGER.debug("HTTP headers: %s", resp.headers) - LOGGER.debug("HTTP content: %s", resp.text) + LOGGER.debug("HTTP content: %s", _text(resp)) class _UnleashApiClient: """ - Sends the SDK's requests to the Unleash server: feature fetches, client - registration and metrics submission. + Talks to the Unleash API: feature fetches, client registration and metrics + submission. Builds each request and decides what its response means; the + transport only sends it. :param config: Client configuration. :param headers: Builds the header set each endpoint needs. + :param events: Receives ``FETCH_COMPLETED`` after every feature fetch. + :param transport: Sends the requests. Defaults to a :class:`RequestsTransport` + built from ``config.http``. """ def __init__( @@ -98,10 +104,18 @@ def __init__( config: UnleashConfig, headers: _HeaderFactory, events: Optional[_EventDispatcher] = None, + transport: Optional[Transport] = None, ) -> None: self._config: UnleashConfig = config self._headers: _HeaderFactory = headers self._events: Optional[_EventDispatcher] = events + + self._transport: Transport = transport or RequestsTransport( + timeout=config.http.timeout, + _retries=config.http.retries, + # this is going to go away soon, just like the whole httpOptions in the client constructor + _request_kwargs=config.http.options, + ) # pylint: disable=broad-except def fetch_features(self, etag: str = "", *, options: PollingOptions) -> FetchResult: @@ -146,31 +160,24 @@ def _fetch_features( if options.project_name: base_params = {"project": options.project_name} - adapter = HTTPAdapter( - max_retries=Retry( - total=config.http.retries, status_forcelist=[500, 502, 504] - ) - ) - with requests.Session() as session: - session.mount("https://", adapter) - session.mount("http://", adapter) - resp = session.get( - base_url, + resp = self._transport.execute( + Request( + method="GET", + url=base_url, headers={ **self._headers.polling(options), **request_specific_headers, }, params=base_params, - timeout=config.http.timeout, - **config.http.options, ) + ) - http_status_code = resp.status_code - if resp.status_code not in [200, 304]: + http_status_code = resp.status + if resp.status not in [200, 304]: _log_resp_info(resp) LOGGER.warning( "Unleash Client feature fetch failed due to unexpected HTTP status code: %s", - resp.status_code, + resp.status, ) raise Exception( "Unleash Client feature fetch failed!" @@ -180,13 +187,14 @@ def _fetch_features( if "etag" in resp.headers.keys(): fetched_etag = resp.headers["etag"] - if resp.status_code == 304: + if resp.status == 304: return ( FetchResult(None, fetched_etag, not_modified=True), FetchStatus.NOT_CHANGED, ) - return FetchResult(resp.text, fetched_etag), _fetched_status(resp.text) + raw_state = _text(resp) + return FetchResult(raw_state, fetched_etag), _fetched_status(raw_state) except Exception as exc: LOGGER.exception( "Unleash Client feature fetch failed due to exception: %s", exc @@ -212,19 +220,20 @@ def register(self, payload: Dict[str, Any]) -> bool: LOGGER.info("Registering unleash client with unleash @ %s", config.url) LOGGER.info("Registration request information: %s", payload) - resp = requests.post( - _normalized_url(config.url, REGISTER_URL), - data=json.dumps(payload), - headers=self._headers.base(), - timeout=config.http.timeout, - **config.http.options, + resp = self._transport.execute( + Request( + method="POST", + url=_normalized_url(config.url, REGISTER_URL), + headers=self._headers.base(), + body=json.dumps(payload).encode("utf-8"), + ) ) - if resp.status_code not in {200, 202}: + if resp.status not in {200, 202}: _log_resp_info(resp) LOGGER.warning( "Unleash Client registration failed due to unexpected HTTP status code: %s", - resp.status_code, + resp.status, ) return False @@ -255,19 +264,20 @@ def send_metrics(self, payload: Dict[str, Any]) -> bool: LOGGER.info("Sending messages to with unleash @ %s", config.url) LOGGER.info("unleash metrics information: %s", payload) - resp = requests.post( - _normalized_url(config.url, METRICS_URL), - data=json.dumps(payload), - headers=self._headers.metrics(), - timeout=config.http.timeout, - **config.http.options, + resp = self._transport.execute( + Request( + method="POST", + url=_normalized_url(config.url, METRICS_URL), + headers=self._headers.metrics(), + body=json.dumps(payload).encode("utf-8"), + ) ) - if resp.status_code != 202: + if resp.status != 202: _log_resp_info(resp) LOGGER.warning( "Unleash Client metrics submission due to unexpected HTTP status code: %s", - resp.status_code, + resp.status, ) return False diff --git a/tests/unit_tests/test_transport.py b/tests/unit_tests/test_transport.py new file mode 100644 index 00000000..66e37548 --- /dev/null +++ b/tests/unit_tests/test_transport.py @@ -0,0 +1,158 @@ +import json + +import pytest +import responses +from requests import ConnectionError + +from tests.utilities.testing_constants import URL +from UnleashClient._transport import Request, RequestsTransport, Transport + +ENDPOINT = URL + "/client/features" + + +def test_requests_transport_is_a_transport(): + assert isinstance(RequestsTransport(), Transport) + + +@responses.activate +def test_execute_sends_a_get_with_its_headers_and_params(): + responses.add(responses.GET, ENDPOINT, status=200) + + RequestsTransport().execute( + Request( + method="GET", + url=ENDPOINT, + headers={"X-Test": "value"}, + params={"project": "ivan"}, + ) + ) + + sent = responses.calls[0].request + assert sent.method == "GET" + assert sent.url == ENDPOINT + "?project=ivan" + assert sent.headers["X-Test"] == "value" + assert sent.body is None + + +@responses.activate +def test_execute_sends_a_post_with_its_body(): + responses.add(responses.POST, ENDPOINT, status=202) + + RequestsTransport().execute( + Request(method="POST", url=ENDPOINT, headers={}, body=b'{"appName": "x"}') + ) + + sent = responses.calls[0].request + assert sent.method == "POST" + assert sent.url == ENDPOINT + assert json.loads(sent.body) == {"appName": "x"} + + +@responses.activate +def test_execute_returns_the_status_headers_and_body(): + responses.add( + responses.GET, + ENDPOINT, + body=b'{"version": 1}', + status=200, + headers={"ETag": "v1"}, + ) + + response = RequestsTransport().execute( + Request(method="GET", url=ENDPOINT, headers={}) + ) + + assert response.status == 200 + assert response.body == b'{"version": 1}' + # looked up case-insensitively + assert response.headers["etag"] == "v1" + + +@responses.activate +@pytest.mark.parametrize("status", [304, 401, 500]) +def test_execute_returns_any_status_instead_of_raising(status): + responses.add(responses.GET, ENDPOINT, status=status) + + response = RequestsTransport().execute( + Request(method="GET", url=ENDPOINT, headers={}) + ) + + assert response.status == status + + +@responses.activate +def test_execute_raises_when_no_response_arrives(): + responses.add(responses.GET, ENDPOINT, body=ConnectionError("refused")) + + with pytest.raises(ConnectionError): + RequestsTransport().execute(Request(method="GET", url=ENDPOINT, headers={})) + + +@responses.activate +def test_execute_retries_a_get_when_retries_are_set(): + responses.add(responses.GET, ENDPOINT, status=500) + responses.add(responses.GET, ENDPOINT, body=b"{}", status=200) + + response = RequestsTransport(_retries=1).execute( + Request(method="GET", url=ENDPOINT, headers={}) + ) + + assert len(responses.calls) == 2 + assert response.status == 200 + + +@responses.activate +def test_execute_does_not_retry_without_retries(): + responses.add(responses.GET, ENDPOINT, status=500) + + response = RequestsTransport().execute( + Request(method="GET", url=ENDPOINT, headers={}) + ) + + assert len(responses.calls) == 1 + assert response.status == 500 + + +@responses.activate +def test_execute_never_retries_a_post(): + responses.add(responses.POST, ENDPOINT, status=500) + + response = RequestsTransport(_retries=3).execute( + Request(method="POST", url=ENDPOINT, headers={}, body=b"{}") + ) + + assert len(responses.calls) == 1 + assert response.status == 500 + + +@responses.activate +def test_execute_passes_the_extra_request_kwargs(): + responses.add( + responses.GET, ENDPOINT, status=302, headers={"Location": URL + "/elsewhere"} + ) + + response = RequestsTransport(_request_kwargs={"allow_redirects": False}).execute( + Request(method="GET", url=ENDPOINT, headers={}) + ) + + assert response.status == 302 + + +def test_execute_rejects_an_extra_kwarg_it_already_sets(): + transport = RequestsTransport(_request_kwargs={"timeout": 5}) + + with pytest.raises(TypeError): + transport.execute(Request(method="GET", url=ENDPOINT, headers={})) + + +@responses.activate +def test_execute_uses_a_fresh_session_for_every_request(): + responses.add(responses.GET, ENDPOINT, status=200, headers={"Set-Cookie": "a=1"}) + responses.add(responses.GET, ENDPOINT, status=200) + transport = RequestsTransport() + + transport.execute(Request(method="GET", url=ENDPOINT, headers={})) + transport.execute(Request(method="GET", url=ENDPOINT, headers={})) + + # A shared session would send the cookie back. + assert "Cookie" not in responses.calls[1].request.headers diff --git a/tests/unit_tests/test_unleash_api_client.py b/tests/unit_tests/test_unleash_api_client.py index c556b005..3375b7e5 100644 --- a/tests/unit_tests/test_unleash_api_client.py +++ b/tests/unit_tests/test_unleash_api_client.py @@ -24,6 +24,7 @@ URL, ) from UnleashClient._headers import _HeaderFactory +from UnleashClient._transport import Request, Response from UnleashClient._unleash_api_client import _UnleashApiClient from UnleashClient.config import ( HttpOptions, @@ -418,3 +419,63 @@ def emit_event(self, event): ) assert json.loads(result.raw_state)["version"] == 1 + + +# transport + + +class RecordingTransport: + """Answers every request with one canned response, keeping the requests.""" + + def __init__(self, response: Response) -> None: + self.response = response + self.requests: list = [] + + def execute(self, request: Request) -> Response: + self.requests.append(request) + return self.response + + +def test_fetch_features_sends_its_request_through_the_transport(): + transport = RecordingTransport( + Response(status=200, headers={"etag": ETAG_VALUE}, body=b'{"version": 1}') + ) + config = UnleashConfig(URL, APP_NAME) + api_client = _UnleashApiClient(config, _HeaderFactory(config), transport=transport) + + result = api_client.fetch_features( + "previous-etag", options=PollingOptions(project_name=PROJECT_NAME) + ) + + (request,) = transport.requests + assert request.method == "GET" + assert request.url == FULL_FEATURE_URL + assert request.params == {"project": PROJECT_NAME} + assert request.headers["If-None-Match"] == "previous-etag" + assert result == ('{"version": 1}', ETAG_VALUE, False) + + +def test_register_posts_its_payload_through_the_transport(): + transport = RecordingTransport(Response(status=202, headers={}, body=b"")) + config = UnleashConfig(URL, APP_NAME) + api_client = _UnleashApiClient(config, _HeaderFactory(config), transport=transport) + + assert api_client.register({"appName": APP_NAME}) + + (request,) = transport.requests + assert request.method == "POST" + assert request.url == FULL_REGISTER_URL + assert json.loads(request.body) == {"appName": APP_NAME} + + +def test_send_metrics_posts_its_payload_through_the_transport(): + transport = RecordingTransport(Response(status=202, headers={}, body=b"")) + config = UnleashConfig(URL, APP_NAME) + api_client = _UnleashApiClient(config, _HeaderFactory(config), transport=transport) + + assert api_client.send_metrics({"appName": APP_NAME}) + + (request,) = transport.requests + assert request.method == "POST" + assert request.url == FULL_METRICS_URL + assert json.loads(request.body) == {"appName": APP_NAME}