EG-4747: turn payload helpers into private functions

CHANGELOG.md

@@ -17,7 +17,7 @@
 * (Minor): The in-progress asynchronous client now builds an internal `_AsyncMetricsReporter`, in the private `UnleashClient._metrics` module, over an internal `_AsyncTransport`, in the private `UnleashClient._async_transport` module, which is not part of the public API and may change or disappear without notice, and exposes `impact_metrics` like the synchronous client does. Nothing starts it yet, as `initialize_client()` still raises `NotImplementedError`, so no metrics are sent, and nothing changes for code using `UnleashClient`. The request body, the impact-metrics collection and the restore-after-a-failed-send are shared with the synchronous reporter; only the request itself and the recurring schedule are separate.
 * (Minor): The asynchronous reporter schedules its sends through the asynchronous client's `_AsyncScheduler` rather than the APScheduler-backed scheduler, because the send has to be awaited and APScheduler runs jobs on worker threads that cannot await. Like the synchronous reporter, it reads `unleash_metrics_interval` and `unleash_metrics_jitter` once, when reporting starts, and the interval and the one-sided jitter are identical. `UnleashClient` and its scheduler are unaffected, and the `scheduler` and `scheduler_executor` constructor arguments still work exactly as before.
 * (Minor): The asynchronous client does not carry over `metrics_headers` or `metric_job`. `metrics_headers` has been informational since the `_Transport` change below (set `unleash_custom_headers` instead), and the metrics job handle is internal. Both are unchanged on `UnleashClient`.
-* (Minor): Metrics reporting now happens through one internal `_MetricsReporter` object, in the private `UnleashClient._metrics` module, which is not part of the public API and may change or disappear without notice, instead of being spread across the module-level `aggregate_and_send_metrics`, the job registration in `initialize_client()` and a second, slightly different call in `destroy()`. The request body is now assembled by `build_metrics_payload` in `UnleashClient.payloads`, alongside the registration payload. The interval, the jitter, the fields sent and the impact-metrics restore-on-failure are exactly what they were, apart from the two entries below. `UnleashClient.periodic_tasks` and its `aggregate_and_send_metrics` are gone, so this affects code importing from `UnleashClient.periodic_tasks` directly. `metric_job`, `metrics_headers` and `impact_metrics` are unchanged on `UnleashClient`. Nothing changes for code using `UnleashClient`.
+* (Minor): Metrics reporting now happens through one internal `_MetricsReporter` object, in the private `UnleashClient._metrics` module, which is not part of the public API and may change or disappear without notice, instead of being spread across the module-level `aggregate_and_send_metrics`, the job registration in `initialize_client()` and a second, slightly different call in `destroy()`. The request body is now assembled by `_build_metrics_payload` in the private `UnleashClient._payloads` module, alongside the registration payload. The interval, the jitter, the fields sent and the impact-metrics restore-on-failure are exactly what they were, apart from the two entries below. `UnleashClient.periodic_tasks` and its `aggregate_and_send_metrics` are gone, so this affects code importing from `UnleashClient.periodic_tasks` directly. `metric_job`, `metrics_headers` and `impact_metrics` are unchanged on `UnleashClient`. Nothing changes for code using `UnleashClient`.
 * (Minor): The metrics request body is now read from the configuration on every send. As a result, reassigning `unleash_app_name`, `unleash_instance_id`, `unleash_sdk_flavor` or `unleash_sdk_flavor_version` after `initialize_client()` now takes effect on the next metrics send, where those used to be captured when the job was registered and changes to them afterwards were silently ignored. This mirrors the read-through `_Transport` already does for urls and headers. `unleash_metrics_interval` and `unleash_metrics_jitter` are still read once, when the job is registered, as before.
 * (Bugfix): The metrics flush on `destroy()` now sends `sdkFlavor` and `sdkFlavorVersion` like every other metrics send. The final send of a client's life used to omit them. Only affects clients configured with `sdk_flavor`.
 * (Minor): `ImpactMetrics` gains `collect()` and `restore()`, so draining impact metrics for a send and handing them back after a failed one go through the object that owns them rather than reaching into the engine directly.
@@ -27,7 +27,7 @@
 * (Minor): Outbound HTTP now happens through one internal `_Transport` object, in the private `UnleashClient._transport` module, instead of the three module-level functions in `UnleashClient.api`. The module is not part of the public API and may change or disappear without notice. The requests on the wire (urls, methods, headers, bodies, status handling, the retry adapter on feature fetches, and the fatal-URL exceptions that registration re-raises) are exactly what they were. `UnleashClient.api` and its `get_feature_toggles`, `send_metrics`, `register_client` and `build_normalized_url` are gone, as is `UnleashClient.utils.log_resp_info`; `PollingConnector` now takes a `transport` instead of `url`, `app_name`, `instance_id`, `headers`, `custom_options`, `request_timeout`, `request_retries` and `project`, and `aggregate_and_send_metrics` takes one in first position instead of `url`, `headers`, `custom_options` and `request_timeout`. This only affects code importing from `UnleashClient.api`, `UnleashClient.connectors` or `UnleashClient.periodic_tasks` directly. Nothing changes for code using `UnleashClient`.
 * (Minor): `_Transport` asks the internal `HeaderFactory` for the header set each request needs, rather than being handed a dict built once at startup. As a result, reassigning `unleash_url`, `unleash_custom_headers`, `unleash_custom_options`, `unleash_request_timeout`, `unleash_request_retries`, `unleash_project_name`, `unleash_app_name` or `unleash_instance_id` after `initialize_client()` now takes effect on the next poll and the next metrics send. Those used to be captured when the connector and the metrics job were created, and changes to them afterwards were silently ignored. Registration always read them at call time and is unaffected. The headers on the wire are otherwise unchanged.
 * (Minor): The `metrics_headers` attribute has been removed from `UnleashClient`. It used to hold the header dict handed to the metrics job at `initialize_client()`, so reassigning it changed what the metrics request sent; the `_Transport` now builds that header set per request. Set `unleash_custom_headers` instead, as it is read on every send.
-* (Minor): The registration request body is now assembled by `build_register_payload` in the new `UnleashClient.payloads` module rather than inline in `register_client`. The fields sent are unchanged, `started` is still stamped at the moment of the request, and nothing changes for code using `UnleashClient`.
+* (Minor): The registration request body is now assembled by `_build_register_payload` in the new private `UnleashClient._payloads` module rather than inline in `register_client`. The module is not part of the public API and may change or disappear without notice. The fields sent are unchanged, `started` is still stamped at the moment of the request, and nothing changes for code using `UnleashClient`.
 * (Minor): Job scheduling now happens through one internal `_Scheduler` object, in the private `UnleashClient._scheduler` module, instead of being re-implemented by the client and each connector. The module is not part of the public API and may change or disappear without notice. The jobs, intervals, jitter and executors are exactly what they were. `PollingConnector` and `OfflineConnector` now take that `_Scheduler` rather than an APScheduler instance, and no longer take `scheduler_executor`, so code importing from `UnleashClient.connectors` directly must build one. The `scheduler` and `scheduler_executor` constructor arguments, and the `unleash_scheduler` and `unleash_executor_name` attributes, are unchanged.
 * (Minor): Passing a `scheduler` that is already running no longer raises `SchedulerAlreadyRunningError`. Starting an already-started scheduler is now a no-op.
 * (Minor): The unused `fl_job` attribute has been removed from `UnleashClient`. It was always `None`, since the connectors own their own jobs, and nothing read it.

UnleashClient/_async_transport.py

@@ -160,7 +160,7 @@ async def register(self, payload: Dict[str, Any]) -> bool:
         request against at all are re-raised rather than swallowed.
 
         :param payload: as built by
-                        :func:`UnleashClient.payloads.build_register_payload`.
+                        :func:`UnleashClient._payloads._build_register_payload`.
         :raises AlreadyClosedError: if the transport has been closed.
         """
         self._raise_if_closed()

UnleashClient/_metrics.py

@@ -6,11 +6,11 @@
 
 from UnleashClient._async_scheduler import _AsyncJob, _AsyncScheduler
 from UnleashClient._async_transport import _AsyncTransport
+from UnleashClient._payloads import _build_metrics_payload
 from UnleashClient._scheduler import _ScheduledJob, _Scheduler
 from UnleashClient._transport import _Transport
 from UnleashClient.config import UnleashConfig
 from UnleashClient.impact_metrics import ImpactMetrics
-from UnleashClient.payloads import build_metrics_payload
 from UnleashClient.utils import LOGGER
 
 
@@ -81,7 +81,7 @@ def flush(self) -> None:
             LOGGER.debug("No feature flags with metrics, skipping metrics submission.")
             return
 
-        payload = build_metrics_payload(self._config, bucket, impact_metrics)
+        payload = _build_metrics_payload(self._config, bucket, impact_metrics)
         if not self._transport.send_metrics(payload) and impact_metrics:
             self._impact_metrics.restore(impact_metrics)
 
@@ -160,7 +160,7 @@ async def flush(self) -> None:
             LOGGER.debug("No feature flags with metrics, skipping metrics submission.")
             return
 
-        payload = build_metrics_payload(self._config, bucket, impact_metrics)
+        payload = _build_metrics_payload(self._config, bucket, impact_metrics)
         sent = False
         try:
             sent = await self._transport.send_metrics(payload)

UnleashClient/payloads.py → UnleashClient/_payloads.py

@@ -28,7 +28,7 @@ def _client_metadata(config: UnleashConfig) -> Dict[str, Any]:
     return metadata
 
 
-def build_register_payload(
+def _build_register_payload(
     config: UnleashConfig, strategies: Dict[str, Any]
 ) -> Dict[str, Any]:
     """
@@ -48,7 +48,7 @@ def build_register_payload(
     }
 
 
-def build_metrics_payload(
+def _build_metrics_payload(
     config: UnleashConfig,
     bucket: Optional[Dict[str, Any]],
     impact_metrics: Optional[Any] = None,

UnleashClient/clients/unleash_client.py

@@ -13,6 +13,7 @@
 from UnleashClient._feature_store import _FeatureStore
 from UnleashClient._instance_registry import _get_instance_registry
 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.cache import BaseCache, FileCache
@@ -43,7 +44,6 @@
 )
 from UnleashClient.headers import HeaderFactory
 from UnleashClient.impact_metrics import ImpactMetrics
-from UnleashClient.payloads import build_register_payload
 from UnleashClient.utils import (
     LOGGER,
     InstanceAllowType,
@@ -493,7 +493,7 @@ def initialize_client(self, fetch_toggles: bool = True) -> None:
                 # Register app
                 if not self.unleash_disable_registration:
                     self._transport.register(
-                        build_register_payload(self._config, self.strategy_mapping)
+                        _build_register_payload(self._config, self.strategy_mapping)
                     )
                 mode = self.connector_mode.get("type", "polling")
 

tests/unit_tests/test_payloads.py

@@ -1,6 +1,6 @@
+from UnleashClient._payloads import _build_metrics_payload, _build_register_payload
 from UnleashClient.config import UnleashConfig
 from UnleashClient.constants import CLIENT_SPEC_VERSION, SDK_NAME, SDK_VERSION
-from UnleashClient.payloads import build_metrics_payload, build_register_payload
 
 URL = "http://localhost:4242/api"
 APP_NAME = "pytest"
@@ -13,7 +13,7 @@ def build_config(**kwargs) -> UnleashConfig:
 def test_register_payload_identifies_the_client():
     config = build_config(instance_id="123", metrics_interval=30)
 
-    payload = build_register_payload(config, {})
+    payload = _build_register_payload(config, {})
 
     assert payload["appName"] == APP_NAME
     assert payload["instanceId"] == "123"
@@ -23,7 +23,7 @@ def test_register_payload_identifies_the_client():
 
 
 def test_register_payload_includes_metadata():
-    payload = build_register_payload(build_config(), {})
+    payload = _build_register_payload(build_config(), {})
 
     assert payload["yggdrasilVersion"] is not None
     assert payload["specVersion"] == CLIENT_SPEC_VERSION
@@ -32,7 +32,7 @@ def test_register_payload_includes_metadata():
 
 
 def test_register_payload_sends_only_the_strategy_names():
-    payload = build_register_payload(
+    payload = _build_register_payload(
         build_config(), {"default": object(), "gradualRollout": object()}
     )
 
@@ -44,7 +44,7 @@ def test_register_payload_includes_sdk_flavor_when_set():
         sdk_flavor="unleash-openfeature-python-provider", sdk_flavor_version="1.2.3"
     )
 
-    payload = build_register_payload(config, {})
+    payload = _build_register_payload(config, {})
 
     assert payload["sdkFlavor"] == "unleash-openfeature-python-provider"
     assert payload["sdkFlavorVersion"] == "1.2.3"
@@ -53,7 +53,7 @@ def test_register_payload_includes_sdk_flavor_when_set():
 
 
 def test_register_payload_omits_sdk_flavor_when_unset():
-    payload = build_register_payload(build_config(), {})
+    payload = _build_register_payload(build_config(), {})
 
     assert "sdkFlavor" not in payload
     assert "sdkFlavorVersion" not in payload
@@ -62,8 +62,8 @@ def test_register_payload_omits_sdk_flavor_when_unset():
 def test_register_payload_stamps_started_at_call_time():
     config = build_config()
 
-    first = build_register_payload(config, {})
-    second = build_register_payload(config, {})
+    first = _build_register_payload(config, {})
+    second = _build_register_payload(config, {})
 
     # `started` is the moment of the call, so a config reused across two
     # registrations does not carry a stale timestamp.
@@ -81,27 +81,27 @@ def test_register_payload_stamps_started_at_call_time():
 def test_metrics_payload_identifies_the_client():
     config = build_config(instance_id="123")
 
-    payload = build_metrics_payload(config, BUCKET)
+    payload = _build_metrics_payload(config, BUCKET)
 
     assert payload["appName"] == APP_NAME
     assert payload["instanceId"] == "123"
     assert payload["connectionId"] == config.connection_id
 
 
 def test_metrics_payload_carries_the_bucket_it_was_given():
-    payload = build_metrics_payload(build_config(), BUCKET)
+    payload = _build_metrics_payload(build_config(), BUCKET)
 
     assert payload["bucket"] == BUCKET
 
 
 def test_metrics_payload_carries_an_absent_bucket_as_none():
-    payload = build_metrics_payload(build_config(), None, [{"name": "purchases"}])
+    payload = _build_metrics_payload(build_config(), None, [{"name": "purchases"}])
 
     assert payload["bucket"] is None
 
 
 def test_metrics_payload_includes_metadata():
-    payload = build_metrics_payload(build_config(), BUCKET)
+    payload = _build_metrics_payload(build_config(), BUCKET)
 
     assert payload["yggdrasilVersion"] is not None
     assert payload["specVersion"] == CLIENT_SPEC_VERSION
@@ -112,29 +112,29 @@ def test_metrics_payload_includes_metadata():
 def test_metrics_payload_includes_impact_metrics_when_there_are_some():
     impact_metrics = [{"name": "purchases", "type": "counter"}]
 
-    payload = build_metrics_payload(build_config(), BUCKET, impact_metrics)
+    payload = _build_metrics_payload(build_config(), BUCKET, impact_metrics)
 
     assert payload["impactMetrics"] == impact_metrics
 
 
 def test_metrics_payload_omits_impact_metrics_when_there_are_none():
-    assert "impactMetrics" not in build_metrics_payload(build_config(), BUCKET)
-    assert "impactMetrics" not in build_metrics_payload(build_config(), BUCKET, [])
+    assert "impactMetrics" not in _build_metrics_payload(build_config(), BUCKET)
+    assert "impactMetrics" not in _build_metrics_payload(build_config(), BUCKET, [])
 
 
 def test_metrics_payload_includes_sdk_flavor_when_set():
     config = build_config(
         sdk_flavor="unleash-openfeature-python-provider", sdk_flavor_version="1.2.3"
     )
 
-    payload = build_metrics_payload(config, BUCKET)
+    payload = _build_metrics_payload(config, BUCKET)
 
     assert payload["sdkFlavor"] == "unleash-openfeature-python-provider"
     assert payload["sdkFlavorVersion"] == "1.2.3"
 
 
 def test_metrics_payload_omits_sdk_flavor_when_unset():
-    payload = build_metrics_payload(build_config(), BUCKET)
+    payload = _build_metrics_payload(build_config(), BUCKET)
 
     assert "sdkFlavor" not in payload
     assert "sdkFlavorVersion" not in payload
@@ -143,9 +143,9 @@ def test_metrics_payload_omits_sdk_flavor_when_unset():
 def test_metrics_payload_reads_the_config_at_call_time():
     config = build_config()
 
-    first = build_metrics_payload(config, BUCKET)
+    first = _build_metrics_payload(config, BUCKET)
     config.app_name = "renamed"
-    second = build_metrics_payload(config, BUCKET)
+    second = _build_metrics_payload(config, BUCKET)
 
     assert first["appName"] == APP_NAME
     assert second["appName"] == "renamed"