Skip to content

Commit 5da2616

Browse files
authored
chore: Extract shared sans-I/O cores and add config read-protocols (#450)
1 parent 67f0661 commit 5da2616

14 files changed

Lines changed: 730 additions & 515 deletions

File tree

CONTRIBUTING.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@ A special case is the module `ldclient.impl`, and any modules within it. Everyth
7878

7979
So, if there is a class whose existence is entirely an implementation detail, it should be in `impl`. Similarly, classes that are _not_ in `impl` must not expose any public members (i.e. symbols that do not have an underscore prefix) that are not meant to be part of the supported public API. This is important because of our guarantee of backward compatibility for all public APIs within a major version: we want to be able to change our implementation details to suit the needs of the code, without worrying about breaking a customer's code. Due to how the language works, we can't actually prevent an application developer from referencing those classes in their code, but this convention makes it clear that such use is discouraged and unsupported.
8080

81+
### Sync/async parity
82+
83+
The SDK maintains parallel sync (`foo.py`) and async (`async_foo.py`) implementations by hand. When you change a method in a sync module, make the matching change in its `async_` sibling (and vice versa), and justify any difference beyond `async`/`await` keywords. Shared I/O-free ("sans-I/O") logic lives in modules with a `_common` suffix that are imported by both siblings: `impl/client_common.py`, `impl/datasystem/fdv2_common.py`, and `impl/events/event_processor_common.py`. Per-side logic that genuinely differs (more than `async`/`await`) lives directly in each sibling, not in a separate `_common` module — for example `impl/evaluator.py`/`impl/async_evaluator.py` and the in-memory feature stores `feature_store.py`/`async_feature_store.py`. Both the sync and async contract test suites must pass.
84+
8185
### Type hints
8286

8387
Python does not require the use of type hints, but they can be extremely helpful for spotting mistakes and for improving the IDE experience, so we should always use them in the SDK. Every method in the public API is expected to have type hints for all non-`self` parameters, and for its return value if any.

ldclient/client.py

Lines changed: 9 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
This submodule contains the client class that provides most of the SDK functionality.
33
"""
44

5-
import hashlib
6-
import hmac
75
import threading
86
import traceback
97
from typing import Any, Callable, Dict, List, Mapping, Optional, Tuple
@@ -19,6 +17,11 @@
1917
_EvaluationWithHookResult
2018
)
2119
from ldclient.impl.big_segments import BigSegmentStoreManager
20+
from ldclient.impl.client_common import (
21+
get_environment_metadata,
22+
get_plugin_hooks
23+
)
24+
from ldclient.impl.client_common import secure_mode_hash as _secure_mode_hash
2225
from ldclient.impl.datasource.feature_requester import FeatureRequesterImpl
2326
from ldclient.impl.datasource.polling import PollingUpdateProcessor
2427
from ldclient.impl.datasource.status import (
@@ -57,12 +60,7 @@
5760
ReadOnlyStore
5861
)
5962
from ldclient.migrations import OpTracker, Stage
60-
from ldclient.plugin import (
61-
ApplicationMetadata,
62-
EnvironmentMetadata,
63-
SdkMetadata
64-
)
65-
from ldclient.version import VERSION
63+
from ldclient.plugin import EnvironmentMetadata
6664
from ldclient.versioned_data_kind import FEATURES, SEGMENTS, VersionedDataKind
6765

6866
from .impl import AnyNum
@@ -239,8 +237,8 @@ def postfork(self, start_wait: float = 5):
239237
self.__start_up(start_wait)
240238

241239
def __start_up(self, start_wait: float):
242-
environment_metadata = self.__get_environment_metadata()
243-
plugin_hooks = self.__get_plugin_hooks(environment_metadata)
240+
environment_metadata = get_environment_metadata(self._config, "python-server-sdk")
241+
plugin_hooks = get_plugin_hooks(self._config, environment_metadata)
244242

245243
self.__hooks_lock = ReadWriteLock()
246244
self.__hooks = self._config.hooks + plugin_hooks # type: List[Hook]
@@ -305,36 +303,6 @@ def __start_up(self, start_wait: float):
305303
else:
306304
log.warning("Initialization timeout exceeded for LaunchDarkly Client or an error occurred. " "Feature Flags may not yet be available.")
307305

308-
def __get_environment_metadata(self) -> EnvironmentMetadata:
309-
sdk_metadata = SdkMetadata(
310-
name="python-server-sdk",
311-
version=VERSION,
312-
wrapper_name=self._config.wrapper_name,
313-
wrapper_version=self._config.wrapper_version
314-
)
315-
316-
application_metadata = None
317-
if self._config.application:
318-
application_metadata = ApplicationMetadata(
319-
id=self._config.application.get('id'),
320-
version=self._config.application.get('version'),
321-
)
322-
323-
return EnvironmentMetadata(
324-
sdk=sdk_metadata,
325-
application=application_metadata,
326-
sdk_key=self._config.sdk_key
327-
)
328-
329-
def __get_plugin_hooks(self, environment_metadata: EnvironmentMetadata) -> List[Hook]:
330-
hooks = []
331-
for plugin in self._config.plugins:
332-
try:
333-
hooks.extend(plugin.get_hooks(environment_metadata))
334-
except Exception as e:
335-
log.error("Error getting hooks from plugin %s: %s", plugin.metadata.name, e)
336-
return hooks
337-
338306
def __register_plugins(self, environment_metadata: EnvironmentMetadata):
339307
for plugin in self._config.plugins:
340308
try:
@@ -704,10 +672,7 @@ def secure_mode_hash(self, context: Context) -> str:
704672
:param context: the evaluation context
705673
:return: the hash string
706674
"""
707-
if not context.valid:
708-
log.warning("Context was invalid for secure_mode_hash (%s); returning empty hash" % context.error)
709-
return ""
710-
return hmac.new(str(self._config.sdk_key).encode(), context.fully_qualified_key.encode(), hashlib.sha256).hexdigest()
675+
return _secure_mode_hash(self._config, context)
711676

712677
def add_hook(self, hook: Hook):
713678
"""

ldclient/config.py

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,12 +160,87 @@ def disable_ssl_verification(self) -> bool:
160160
T_co = TypeVar("T_co", covariant=True)
161161

162162

163+
class SdkIdentityConfig(Protocol):
164+
"""
165+
The SDK's self-identity fields — read by the shared environment-metadata and
166+
secure-mode helpers, and included in request headers.
167+
"""
168+
169+
@property
170+
def sdk_key(self) -> Optional[str]:
171+
"""The SDK key used to authenticate requests."""
172+
173+
@property
174+
def application(self) -> dict:
175+
"""The application metadata included in request headers."""
176+
177+
@property
178+
def wrapper_name(self) -> Optional[str]:
179+
"""The wrapper name included in request headers."""
180+
181+
@property
182+
def wrapper_version(self) -> Optional[str]:
183+
"""The wrapper version included in request headers."""
184+
185+
186+
class DataSourceBuilderConfig(SdkIdentityConfig, Protocol):
187+
"""
188+
The subset of configuration that data source builders and the data sources
189+
they build read. Extends :class:`SdkIdentityConfig` with the transport and
190+
endpoint fields. It is the structural contract a config object must satisfy
191+
to be passed to :meth:`DataSourceBuilder.build`.
192+
"""
193+
194+
@property
195+
def base_uri(self) -> str:
196+
"""The base URI for polling requests."""
197+
198+
@property
199+
def stream_base_uri(self) -> str:
200+
"""The base URI for streaming requests."""
201+
202+
@property
203+
def http(self) -> HTTPConfig:
204+
"""The HTTP configuration used for requests."""
205+
206+
@property
207+
def initial_reconnect_delay(self) -> float:
208+
"""The initial reconnect delay for the streaming data source."""
209+
210+
@property
211+
def poll_interval(self) -> float:
212+
"""The interval between polling requests."""
213+
214+
@property
215+
def payload_filter_key(self) -> Optional[str]:
216+
"""The payload filter key applied as a query parameter."""
217+
218+
# Set by the client after construction, so it is a read-write attribute
219+
# rather than a read-only property.
220+
_instance_id: Optional[str]
221+
222+
223+
class PrivateAttributesConfig(Protocol):
224+
"""
225+
The private-attribute redaction settings read by the event output formatter
226+
when building analytics events.
227+
"""
228+
229+
@property
230+
def all_attributes_private(self) -> bool:
231+
"""Whether all context attributes should be treated as private."""
232+
233+
@property
234+
def private_attributes(self) -> List[str]:
235+
"""The context attribute references to treat as private."""
236+
237+
163238
class DataSourceBuilder(Protocol[T_co]): # pylint: disable=too-few-public-methods
164239
"""
165240
Protocol for building data sources.
166241
"""
167242

168-
def build(self, config: 'Config') -> T_co:
243+
def build(self, config: DataSourceBuilderConfig) -> T_co:
169244
"""
170245
Builds the data source.
171246
@@ -199,7 +274,7 @@ class DataSystemConfig:
199274
"""An optional fallback synchronizer that will read from FDv1"""
200275

201276

202-
class Config:
277+
class Config(DataSourceBuilderConfig, PrivateAttributesConfig):
203278
"""Advanced configuration options for the SDK client.
204279
205280
To use these options, create an instance of ``Config`` and pass it to either :func:`ldclient.set_config()`

ldclient/impl/client_common.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
"""
2+
Genuinely I/O-free, await-free helpers shared by the sync :class:`ldclient.client.LDClient`
3+
and async :class:`ldclient.async_client.AsyncLDClient`.
4+
5+
These functions contain no awaits and touch no store/network/event I/O, so they
6+
can live in a single module imported by both clients. Anything that reads the
7+
store, sends events, or runs hook stages is I/O-adjacent and is hand-duplicated
8+
across the two client classes instead (differing only in ``async``/``await``).
9+
"""
10+
11+
import hashlib
12+
import hmac
13+
from typing import List
14+
15+
from ldclient.config import Config, SdkIdentityConfig
16+
from ldclient.context import Context
17+
from ldclient.hook import Hook
18+
from ldclient.impl.util import log
19+
from ldclient.plugin import (
20+
ApplicationMetadata,
21+
EnvironmentMetadata,
22+
SdkMetadata
23+
)
24+
from ldclient.version import VERSION
25+
26+
27+
def get_environment_metadata(config: SdkIdentityConfig, sdk_name: str) -> EnvironmentMetadata:
28+
sdk_metadata = SdkMetadata(
29+
name=sdk_name,
30+
version=VERSION,
31+
wrapper_name=config.wrapper_name,
32+
wrapper_version=config.wrapper_version
33+
)
34+
35+
application_metadata = None
36+
if config.application:
37+
application_metadata = ApplicationMetadata(
38+
id=config.application.get('id'),
39+
version=config.application.get('version'),
40+
)
41+
42+
return EnvironmentMetadata(
43+
sdk=sdk_metadata,
44+
application=application_metadata,
45+
sdk_key=config.sdk_key
46+
)
47+
48+
49+
def get_plugin_hooks(config: Config, environment_metadata: EnvironmentMetadata) -> List[Hook]:
50+
hooks = []
51+
for plugin in config.plugins:
52+
try:
53+
hooks.extend(plugin.get_hooks(environment_metadata))
54+
except Exception as e:
55+
log.error("Error getting hooks from plugin %s: %s", plugin.metadata.name, e)
56+
return hooks
57+
58+
59+
def secure_mode_hash(config: SdkIdentityConfig, context: Context) -> str:
60+
"""Computes the secure-mode HMAC for a context, or an empty string for an
61+
invalid context. Pure: depends only on the SDK key and the context's
62+
fully-qualified key."""
63+
if not context.valid:
64+
log.warning("Context was invalid for secure_mode_hash (%s); returning empty hash" % context.error)
65+
return ""
66+
return hmac.new(str(config.sdk_key).encode(), context.fully_qualified_key.encode(), hashlib.sha256).hexdigest()

ldclient/impl/datasourcev2/polling.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@
1313

1414
import urllib3
1515

16-
from ldclient.config import Config, DataSourceBuilder, HTTPConfig
16+
from ldclient.config import (
17+
DataSourceBuilder,
18+
DataSourceBuilderConfig,
19+
HTTPConfig
20+
)
1721
from ldclient.impl.datasource.feature_requester import FDV1_POLLING_ENDPOINT
1822
from ldclient.impl.datasystem.protocolv2 import (
1923
DeleteObject,
@@ -257,7 +261,7 @@ class Urllib3PollingRequester(Requester):
257261
requests.
258262
"""
259263

260-
def __init__(self, config: Config, base_uri: str, http_options: HTTPConfig):
264+
def __init__(self, config: DataSourceBuilderConfig, base_uri: str, http_options: HTTPConfig):
261265
self._etag = None
262266
factory = HTTPFactory(_base_headers(config), http_options)
263267
self._http = factory.create_pool_manager(1, base_uri)
@@ -422,7 +426,7 @@ def requester(self, requester: Requester) -> 'PollingDataSourceBuilder':
422426
self.__requester = requester
423427
return self
424428

425-
def build(self, config: Config) -> PollingDataSource:
429+
def build(self, config: DataSourceBuilderConfig) -> PollingDataSource:
426430
"""Builds the PollingDataSource with the configured parameters."""
427431
requester = (
428432
self.__requester
@@ -465,7 +469,7 @@ def http_options(self, http_options: HTTPConfig) -> 'FallbackToFDv1PollingDataSo
465469
self.__http_options = http_options
466470
return self
467471

468-
def build(self, config: Config) -> PollingDataSource:
472+
def build(self, config: DataSourceBuilderConfig) -> PollingDataSource:
469473
"""Builds the PollingDataSource with the configured parameters."""
470474
builder = PollingDataSourceBuilder()
471475
builder.requester(
@@ -487,7 +491,7 @@ class Urllib3FDv1PollingRequester(Requester):
487491
requests.
488492
"""
489493

490-
def __init__(self, config: Config, base_uri: str, http_options: HTTPConfig):
494+
def __init__(self, config: DataSourceBuilderConfig, base_uri: str, http_options: HTTPConfig):
491495
self._etag = None
492496
self._http = HTTPFactory(_base_headers(config), http_options).create_pool_manager(
493497
1, base_uri

ldclient/impl/datasourcev2/streaming.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@
1818
)
1919
from ld_eventsource.errors import HTTPStatusError
2020

21-
from ldclient.config import Config, DataSourceBuilder, HTTPConfig
21+
from ldclient.config import (
22+
DataSourceBuilder,
23+
DataSourceBuilderConfig,
24+
HTTPConfig
25+
)
2226
from ldclient.impl.datasystem import DiagnosticAccumulator, DiagnosticSource
2327
from ldclient.impl.datasystem.protocolv2 import (
2428
DeleteObject,
@@ -59,7 +63,7 @@
5963
STREAMING_ENDPOINT = "/sdk/stream"
6064

6165
SseClientBuilder = Callable[
62-
[str, HTTPConfig, float, Config, SelectorStore],
66+
[str, HTTPConfig, float, DataSourceBuilderConfig, SelectorStore],
6367
Tuple[SSEClient, Optional[urllib3.PoolManager]],
6468
]
6569

@@ -68,7 +72,7 @@ def create_sse_client(
6872
base_uri: str,
6973
http_options: HTTPConfig,
7074
initial_reconnect_delay: float,
71-
config: Config,
75+
config: DataSourceBuilderConfig,
7276
ss: SelectorStore
7377
) -> Tuple[SSEClient, Optional[urllib3.PoolManager]]:
7478
""" "
@@ -158,7 +162,7 @@ def __init__(self,
158162
uri: str,
159163
http_options: HTTPConfig,
160164
initial_reconnect_delay: float,
161-
config: Config):
165+
config: DataSourceBuilderConfig):
162166
self.__uri = uri
163167
self.__http_options = http_options
164168
self.__initial_reconnect_delay = initial_reconnect_delay
@@ -519,7 +523,7 @@ def http_options(self, http_options: HTTPConfig) -> 'StreamingDataSourceBuilder'
519523
self.__http_options = http_options
520524
return self
521525

522-
def build(self, config: Config) -> StreamingDataSource:
526+
def build(self, config: DataSourceBuilderConfig) -> StreamingDataSource:
523527
"""Builds a StreamingDataSource instance with the configured parameters."""
524528
return StreamingDataSource(
525529
self.__base_uri or config.stream_base_uri,

0 commit comments

Comments
 (0)