From fe277b391d0b26568cb4deb028c7b1652cd84c19 Mon Sep 17 00:00:00 2001 From: zeevdr Date: Mon, 8 Jun 2026 13:06:08 +0300 Subject: [PATCH] Send typed wire values matching Python value's runtime type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit set()/set_many() always wrapped the value in TypedValue(string_value=...), but the server requires the populated oneof variant to match the field's declared schema type exactly — so writing to any non-string field raised InvalidArgumentError ("expected bool value", etc). make_typed_value() now picks the wire variant from value's Python runtime type (bool, int, float, datetime, timedelta, dict/list, str), mirroring convert_value() on the read side — no schema lookup or extra round trip needed. URL becomes a real str subtype so url-typed fields (which also use a string wire representation, but a distinct oneof variant) can be targeted by passing URL(...) instead of a plain str. Closes #139 --- sdk/src/opendecree/_convert.py | 17 +++++-- sdk/src/opendecree/_stubs.py | 56 ++++++++++++++++++--- sdk/src/opendecree/async_client.py | 21 +++++--- sdk/src/opendecree/client.py | 25 ++++++---- sdk/src/opendecree/types.py | 15 +++++- sdk/tests/test_convert.py | 10 ++++ sdk/tests/test_stubs.py | 79 ++++++++++++++++++++++++++++++ 7 files changed, 194 insertions(+), 29 deletions(-) create mode 100644 sdk/tests/test_stubs.py diff --git a/sdk/src/opendecree/_convert.py b/sdk/src/opendecree/_convert.py index 9126460..6cfa126 100644 --- a/sdk/src/opendecree/_convert.py +++ b/sdk/src/opendecree/_convert.py @@ -12,8 +12,16 @@ from opendecree.errors import TypeMismatchError -# Type alias for url-typed fields — semantically distinct from plain str. -URL = str + +class URL(str): + """String subtype for url-typed fields. + + Reads behave identically to ``str`` — pass ``URL`` to ``get()`` purely to + express intent in your own annotations. Writes use the subtype to pick the + ``url_value`` oneof variant instead of ``string_value`` (see + ``make_typed_value`` in ``_stubs.py``) — the server validates the two + variants against different schema field types. + """ def _parse_timedelta(s: str) -> timedelta: @@ -65,12 +73,13 @@ def convert_value(raw: str, target_type: type) -> object: """Convert a raw string value to the target Python type. Supported types: str, int, float, bool, datetime, timedelta, dict, list. - URL is an alias for str and is handled identically. + URL is a str subtype and is read identically — the wire value is the same + string regardless of which TypedValue oneof variant the server populated. Raises: TypeMismatchError: If the value cannot be converted to the target type. """ - if target_type is str: + if target_type is str or target_type is URL: return raw try: if target_type is int: diff --git a/sdk/src/opendecree/_stubs.py b/sdk/src/opendecree/_stubs.py index 9c4a0c1..e2faee2 100644 --- a/sdk/src/opendecree/_stubs.py +++ b/sdk/src/opendecree/_stubs.py @@ -2,9 +2,14 @@ from __future__ import annotations +import json +from datetime import datetime, timedelta from types import ModuleType from typing import Any +from opendecree._convert import URL +from opendecree.types import ConfigValue + def ensure_stubs() -> tuple[ModuleType, ModuleType]: """Lazy-load ConfigService proto stubs on first use. @@ -21,15 +26,54 @@ def ensure_stubs() -> tuple[ModuleType, ModuleType]: return cs_pb2, cs_grpc -def make_string_typed_value(value: str) -> Any: - """Create a TypedValue proto with string_value set. - - The server accepts string values for all field types and performs - type coercion based on the schema definition. +def make_typed_value(value: ConfigValue) -> Any: + """Build a TypedValue proto whose oneof variant matches `value`'s Python type. + + This is the write-side mirror of `convert_value`: the server requires the + populated oneof variant to match the field's declared schema type exactly + (no coercion), so the SDK picks the variant from the value's runtime type + rather than looking up the schema (which would cost an extra round trip). + + | Python type | TypedValue variant | + |-----------------------|--------------------| + | `bool` | `bool_value` | + | `URL` | `url_value` | + | `str` | `string_value` | + | `int` | `integer_value` | + | `float` | `number_value` | + | `datetime` | `time_value` | + | `timedelta` | `duration_value` | + | `dict` / `list` | `json_value` | + + `bool` is checked before `int` (it's a subclass), and `URL` before `str` + (same reason) — `URL` is a `str` subtype precisely so a write can + distinguish a `url`-typed field from a plain `string`-typed one. """ + from google.protobuf import duration_pb2, timestamp_pb2 + from opendecree._generated.centralconfig.v1 import types_pb2 - return types_pb2.TypedValue(string_value=value) + if isinstance(value, bool): + return types_pb2.TypedValue(bool_value=value) + if isinstance(value, URL): + return types_pb2.TypedValue(url_value=value) + if isinstance(value, str): + return types_pb2.TypedValue(string_value=value) + if isinstance(value, int): + return types_pb2.TypedValue(integer_value=value) + if isinstance(value, float): + return types_pb2.TypedValue(number_value=value) + if isinstance(value, datetime): + ts = timestamp_pb2.Timestamp() + ts.FromDatetime(value) + return types_pb2.TypedValue(time_value=ts) + if isinstance(value, timedelta): + d = duration_pb2.Duration() + d.FromTimedelta(value) + return types_pb2.TypedValue(duration_value=d) + if isinstance(value, (dict, list)): + return types_pb2.TypedValue(json_value=json.dumps(value)) + raise TypeError(f"unsupported value type for set(): {type(value).__name__}") def process_get_response( diff --git a/sdk/src/opendecree/async_client.py b/sdk/src/opendecree/async_client.py index 167b8da..caaef4c 100644 --- a/sdk/src/opendecree/async_client.py +++ b/sdk/src/opendecree/async_client.py @@ -23,12 +23,12 @@ from opendecree._retry import RetryConfig, async_with_retry, write_safe_config from opendecree._stubs import ( ensure_stubs, - make_string_typed_value, + make_typed_value, process_get_all_response, process_get_response, ) from opendecree.errors import map_grpc_error -from opendecree.types import FieldUpdate, ServerVersion +from opendecree.types import ConfigValue, FieldUpdate, ServerVersion class AsyncConfigClient: @@ -291,7 +291,7 @@ async def set( self, tenant_id: str, field_path: str, - value: str, + value: ConfigValue, *, description: str | None = None, value_description: str | None = None, @@ -300,13 +300,18 @@ async def set( ) -> None: """Set a config value. - The value is sent as a string — the server coerces it to the - schema-defined type (integer, bool, etc.). + `value` is a native Python value — ``str``, ``int``, ``float``, + ``bool``, ``datetime``, ``timedelta``, ``dict``, ``list``, or ``URL`` + (for ``url``-typed fields; a plain ``str`` targets ``string``-typed + fields). The SDK picks the wire representation matching `value`'s + runtime type, which the server then validates against the field's + declared schema type — passing the wrong Python type raises + ``InvalidArgumentError``. Args: tenant_id: Tenant UUID. field_path: Dot-separated field path (e.g., ``"payments.fee"``). - value: The value as a string. + value: The value, as a Python type matching the field's schema type. description: Optional version-level description for the audit log. value_description: Optional description stored with this specific value. expected_checksum: When set, the server rejects the write if the @@ -332,7 +337,7 @@ async def _call() -> None: self._pb2.SetFieldRequest( tenant_id=tenant_id, field_path=field_path, - value=make_string_typed_value(value), + value=make_typed_value(value), description=description, value_description=value_description, expected_checksum=expected_checksum, @@ -378,7 +383,7 @@ async def _call() -> None: proto_updates = [ self._pb2.FieldUpdate( field_path=u.field_path, - value=make_string_typed_value(u.value), + value=make_typed_value(u.value), expected_checksum=u.expected_checksum, value_description=u.value_description, ) diff --git a/sdk/src/opendecree/client.py b/sdk/src/opendecree/client.py index abd7b28..9666384 100644 --- a/sdk/src/opendecree/client.py +++ b/sdk/src/opendecree/client.py @@ -6,7 +6,9 @@ - watch() factory for live config subscriptions (Phase 4) - Automatic retry with exponential backoff -All writes send string values — the server coerces to the schema-defined type. +Writes accept native Python values (str, int, float, bool, datetime, timedelta, +dict, list, URL) — the SDK picks the wire representation matching the value's +runtime type, mirroring the conversions get() does on the read side. """ from __future__ import annotations @@ -26,12 +28,12 @@ from opendecree._retry import RetryConfig, with_retry, write_safe_config from opendecree._stubs import ( ensure_stubs, - make_string_typed_value, + make_typed_value, process_get_all_response, process_get_response, ) from opendecree.errors import map_grpc_error -from opendecree.types import FieldUpdate, ServerVersion +from opendecree.types import ConfigValue, FieldUpdate, ServerVersion class ConfigClient: @@ -298,7 +300,7 @@ def set( self, tenant_id: str, field_path: str, - value: str, + value: ConfigValue, *, description: str | None = None, value_description: str | None = None, @@ -307,13 +309,18 @@ def set( ) -> None: """Set a config value. - The value is sent as a string — the server coerces it to the - schema-defined type (integer, bool, etc.). + `value` is a native Python value — ``str``, ``int``, ``float``, + ``bool``, ``datetime``, ``timedelta``, ``dict``, ``list``, or ``URL`` + (for ``url``-typed fields; a plain ``str`` targets ``string``-typed + fields). The SDK picks the wire representation matching `value`'s + runtime type, which the server then validates against the field's + declared schema type — passing the wrong Python type raises + ``InvalidArgumentError``. Args: tenant_id: Tenant UUID. field_path: Dot-separated field path (e.g., ``"payments.fee"``). - value: The value as a string. + value: The value, as a Python type matching the field's schema type. description: Optional version-level description for the audit log. value_description: Optional description stored with this specific value. expected_checksum: When set, the server rejects the write if the @@ -339,7 +346,7 @@ def _call() -> None: self._pb2.SetFieldRequest( tenant_id=tenant_id, field_path=field_path, - value=make_string_typed_value(value), + value=make_typed_value(value), description=description, value_description=value_description, expected_checksum=expected_checksum, @@ -384,7 +391,7 @@ def _call() -> None: proto_updates = [ self._pb2.FieldUpdate( field_path=u.field_path, - value=make_string_typed_value(u.value), + value=make_typed_value(u.value), expected_checksum=u.expected_checksum, value_description=u.value_description, ) diff --git a/sdk/src/opendecree/types.py b/sdk/src/opendecree/types.py index 5990f21..97a8d40 100644 --- a/sdk/src/opendecree/types.py +++ b/sdk/src/opendecree/types.py @@ -6,6 +6,14 @@ from __future__ import annotations from dataclasses import dataclass +from datetime import datetime, timedelta +from typing import Any + +#: Native Python value accepted by `set`/`set_many`/`FieldUpdate`. The SDK +#: picks the wire `TypedValue` variant from the value's runtime type — see +#: `make_typed_value` in `_stubs.py`. `URL` (a `str` subtype) is covered by +#: `str` here but still selects the distinct `url_value` wire variant. +ConfigValue = str | int | float | bool | datetime | timedelta | dict[str, Any] | list[Any] @dataclass(frozen=True, slots=True) @@ -33,14 +41,17 @@ class FieldUpdate: Attributes: field_path: Dot-separated field path (e.g., ``"payments.fee"``). - value: The value as a string. + value: The value as a native Python type matching the field's schema + type — ``str``, ``int``, ``float``, ``bool``, ``datetime``, + ``timedelta``, ``dict``, ``list``, or ``URL`` (for ``url``-typed + fields). The SDK converts it to the matching wire representation. expected_checksum: When set, the server rejects the write if the current value's checksum does not match (optimistic concurrency). value_description: Optional description stored with this specific value. """ field_path: str - value: str + value: ConfigValue expected_checksum: str | None = None value_description: str | None = None diff --git a/sdk/tests/test_convert.py b/sdk/tests/test_convert.py index 5a40bf2..58c288d 100644 --- a/sdk/tests/test_convert.py +++ b/sdk/tests/test_convert.py @@ -134,6 +134,16 @@ def test_convert_url(): assert isinstance(result, str) +def test_url_is_str_subtype(): + # URL must remain a plain str for read-side compatibility, but be + # distinguishable at runtime so writes can pick the url_value wire variant. + u = URL("https://example.com") + assert isinstance(u, str) + assert isinstance(u, URL) + assert not isinstance("https://example.com", URL) + assert u == "https://example.com" + + def test_parse_timedelta_empty(): assert _parse_timedelta("") == timedelta() diff --git a/sdk/tests/test_stubs.py b/sdk/tests/test_stubs.py new file mode 100644 index 0000000..cef7597 --- /dev/null +++ b/sdk/tests/test_stubs.py @@ -0,0 +1,79 @@ +"""Tests for write-side TypedValue construction.""" + +from datetime import UTC, datetime, timedelta + +import pytest + +from opendecree._convert import URL +from opendecree._generated.centralconfig.v1 import types_pb2 +from opendecree._stubs import make_typed_value + + +def test_make_typed_value_string(): + tv = make_typed_value("hello") + assert tv.WhichOneof("kind") == "string_value" + assert tv.string_value == "hello" + + +def test_make_typed_value_url(): + tv = make_typed_value(URL("https://example.com")) + assert tv.WhichOneof("kind") == "url_value" + assert tv.url_value == "https://example.com" + + +def test_make_typed_value_bool(): + tv = make_typed_value(True) + assert tv.WhichOneof("kind") == "bool_value" + assert tv.bool_value is True + + +def test_make_typed_value_bool_not_integer(): + # bool is a subclass of int — must be checked first. + tv = make_typed_value(False) + assert tv.WhichOneof("kind") == "bool_value" + + +def test_make_typed_value_integer(): + tv = make_typed_value(42) + assert tv.WhichOneof("kind") == "integer_value" + assert tv.integer_value == 42 + + +def test_make_typed_value_number(): + tv = make_typed_value(3.14) + assert tv.WhichOneof("kind") == "number_value" + assert tv.number_value == pytest.approx(3.14) + + +def test_make_typed_value_time(): + dt = datetime(2024, 1, 15, 12, 30, tzinfo=UTC) + tv = make_typed_value(dt) + assert tv.WhichOneof("kind") == "time_value" + assert tv.time_value.ToDatetime(tzinfo=UTC) == dt + + +def test_make_typed_value_duration(): + tv = make_typed_value(timedelta(hours=1, minutes=30)) + assert tv.WhichOneof("kind") == "duration_value" + assert tv.duration_value.ToTimedelta() == timedelta(hours=1, minutes=30) + + +def test_make_typed_value_json_dict(): + tv = make_typed_value({"a": 1}) + assert tv.WhichOneof("kind") == "json_value" + assert tv.json_value == '{"a": 1}' + + +def test_make_typed_value_json_list(): + tv = make_typed_value([1, 2, 3]) + assert tv.WhichOneof("kind") == "json_value" + assert tv.json_value == "[1, 2, 3]" + + +def test_make_typed_value_unsupported_type(): + with pytest.raises(TypeError, match="unsupported value type for set"): + make_typed_value(object()) + + +def test_make_typed_value_returns_typed_value_proto(): + assert isinstance(make_typed_value("x"), types_pb2.TypedValue)