Skip to content

Commit b2f719e

Browse files
committed
fix: Type the sink-or-store helper for each side instead of erasing it
The shared helper took unbound TypeVars, which accepted any sink beside any store and returned a union, so it checked nothing. Splitting it into sink_or_store and async_sink_or_store lets each name its own sink and store types. main did not need this because the async caller sat in an unannotated method, whose body mypy skips. Real types surfaced a latent mismatch in _process_message: FEATURES and SEGMENTS are VersionedDataKindWithOrdering, and Mapping's key type is invariant, so the inferred dict did not satisfy init's parameter. The literal now carries the declared type.
1 parent 816df7a commit b2f719e

3 files changed

Lines changed: 28 additions & 21 deletions

File tree

‎ldclient/impl/datasource/async_polling.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
from ldclient.async_config import AsyncConfig
1111
from ldclient.impl.aio.concurrency import AsyncEvent, AsyncRepeatingTask
12-
from ldclient.impl.datasource.datasource_common import sink_or_store
12+
from ldclient.impl.datasource.datasource_common import async_sink_or_store
1313
from ldclient.impl.retry import (
1414
FailureKind,
1515
RetryState,
@@ -73,7 +73,7 @@ async def _fetch_and_store(self) -> None:
7373
"""Makes one poll request and records the outcome on the retry state."""
7474
try:
7575
all_data = await self._requester.get_all_data()
76-
await sink_or_store(self._data_source_update_sink, self._store).init(all_data)
76+
await async_sink_or_store(self._data_source_update_sink, self._store).init(all_data)
7777

7878
if self._data_source_update_sink is not None:
7979
self._data_source_update_sink.update_status(DataSourceState.VALID, None)

‎ldclient/impl/datasource/async_streaming.py‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import asyncio
88
import json
99
import time
10-
from typing import Any, Callable, Optional
10+
from typing import Any, Callable, Mapping, Optional
1111
from urllib import parse
1212

1313
from ld_eventsource.actions import Event, Fault, Start
@@ -18,8 +18,8 @@
1818
from ldclient.impl.datasource.datasource_common import (
1919
STREAM_ALL_PATH,
2020
StreamClosedError,
21-
parse_path,
22-
sink_or_store
21+
async_sink_or_store,
22+
parse_path
2323
)
2424
from ldclient.impl.retry import (
2525
FailureKind,
@@ -34,7 +34,7 @@
3434
DataSourceErrorKind,
3535
DataSourceState
3636
)
37-
from ldclient.versioned_data_kind import FEATURES, SEGMENTS
37+
from ldclient.versioned_data_kind import FEATURES, SEGMENTS, VersionedDataKind
3838

3939

4040
class AsyncStreamingUpdateProcessor(AsyncUpdateProcessor):
@@ -196,10 +196,10 @@ def initialized(self):
196196
# Returns True if we initialized the feature store
197197
async def _process_message(self, msg: Event) -> bool:
198198
"""Process a single SSE event. Returns True on a successful ``put``."""
199-
target = sink_or_store(self._data_source_update_sink, self._store)
199+
target = async_sink_or_store(self._data_source_update_sink, self._store)
200200
if msg.event == 'put':
201201
all_data = json.loads(msg.data)
202-
init_data = {FEATURES: all_data['data']['flags'], SEGMENTS: all_data['data']['segments']}
202+
init_data: Mapping[VersionedDataKind, Mapping[str, dict]] = {FEATURES: all_data['data']['flags'], SEGMENTS: all_data['data']['segments']}
203203
log.debug("Received put event with %d flags and %d segments", len(init_data[FEATURES]), len(init_data[SEGMENTS]))
204204
await target.init(init_data)
205205
return True

‎ldclient/impl/datasource/datasource_common.py‎

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,15 @@
55
# currently excluded from documentation - see docs/README.md
66

77
from collections import namedtuple
8-
from typing import (
9-
Mapping,
10-
Optional,
11-
Protocol,
12-
TypeVar,
13-
Union,
14-
runtime_checkable
15-
)
8+
from typing import Mapping, Optional, Protocol, Union, runtime_checkable
169

1710
from ldclient.impl.util import _LD_ENVID_HEADER
11+
from ldclient.interfaces import (
12+
AsyncDataSourceUpdateSink,
13+
AsyncFeatureStore,
14+
DataSourceUpdateSink,
15+
FeatureStore
16+
)
1817
from ldclient.versioned_data_kind import FEATURES, SEGMENTS
1918

2019
STREAM_ALL_PATH = '/all'
@@ -40,11 +39,7 @@ def __init__(self):
4039
super().__init__("the server closed the stream connection")
4140

4241

43-
_Sink = TypeVar('_Sink')
44-
_Store = TypeVar('_Store')
45-
46-
47-
def sink_or_store(sink: Optional[_Sink], store: _Store) -> Union[_Sink, _Store]:
42+
def sink_or_store(sink: Optional[DataSourceUpdateSink], store: FeatureStore) -> Union[DataSourceUpdateSink, FeatureStore]:
4843
"""
4944
The original implementation of the data sources relied on the feature store
5045
directly, which we are trying to move away from. Customers who might have
@@ -62,6 +57,18 @@ def sink_or_store(sink: Optional[_Sink], store: _Store) -> Union[_Sink, _Store]:
6257
return sink
6358

6459

60+
def async_sink_or_store(sink: Optional[AsyncDataSourceUpdateSink], store: AsyncFeatureStore) -> Union[AsyncDataSourceUpdateSink, AsyncFeatureStore]:
61+
"""
62+
The async counterpart of :func:`sink_or_store`, kept separate so each side
63+
names its own sink and store types. One shared generic function accepted a
64+
sync sink beside an async store without complaint.
65+
"""
66+
if sink is None:
67+
return store
68+
69+
return sink
70+
71+
6572
@runtime_checkable
6673
class EnvironmentIdSink(Protocol):
6774
"""

0 commit comments

Comments
 (0)