diff --git a/packages/client/README.md b/packages/client/README.md index c3895102..1819cb52 100644 --- a/packages/client/README.md +++ b/packages/client/README.md @@ -33,8 +33,8 @@ No code changes are required — `init_client()` detects the packages at runtime | Variable | Required | Description | |---|---|---| | `LD_SDK_KEY` | Yes | LaunchDarkly server-side SDK key | -| `LD_BASE_URI` | No | Override the LaunchDarkly polling base URI (e.g. for staging) | -| `LD_STREAM_URI` | No | Override the streaming URI | +| `LD_BASE_URI` | No | Override the LaunchDarkly polling base URI (e.g. for staging). `FDv2SkillStore` reads it too when given no URI option | +| `LD_STREAM_URI` | No | Override the streaming URI. `FDv2SkillStore` reads it too when given no URI option | | `LD_EVENTS_URI` | No | Override the events URI | | `LD_SERVICE_NAME` | No | OTel `service.name` resource attribute (default: `python-sdk`) | | `LD_ENVIRONMENT` | No | `deployment.environment` resource attribute attached to telemetry | @@ -788,6 +788,11 @@ never followed, so a 3xx stops delivery instead of forwarding the key to the `Lo **Polling and streaming have separate hosts.** By default `/sdk/poll` goes to `https://sdk.launchdarkly.com` and `/sdk/stream` to `https://stream.launchdarkly.com`. Pass `base_uri` alone to use one host for both, or `stream_uri` as well to set them independently. +With neither option, the store reads `LD_BASE_URI` and `LD_STREAM_URI` in their place, once, when +it is constructed, the same variables `init_client()` reads. So a process pointed at a staging or +private instance by its environment gets its skills from there too. `LD_BASE_URI` alone moves both +endpoints, as `base_uri` alone does. That differs from `init_client()`, where flag streaming stays +on its default host unless `LD_STREAM_URI` is set. **`close()` is final.** A closed store still answers from the content it received, but `start()` afterwards raises, so delivery is guaranteed stopped even if the join times out. diff --git a/packages/client/agents.md b/packages/client/agents.md index 5b3a93f9..c3bfe485 100644 --- a/packages/client/agents.md +++ b/packages/client/agents.md @@ -303,6 +303,14 @@ resumes in place once the cause is fixed, clearing the terminal reason through answer) surfaces as an `HTTPError` that `_classify_status` maps to a fatal, non-retried failure. +**Polling and streaming have different default hosts.** `GET /sdk/poll` uses `DEFAULT_BASE_URI` +(`sdk.launchdarkly.com`) and `GET /sdk/stream` uses `DEFAULT_STREAM_URI` +(`stream.launchdarkly.com`). A lone `base_uri` applies to both. When neither option is given, the +constructor reads `LD_BASE_URI` / `LD_STREAM_URI` (blank is unset) in their place and applies the +same rule. An option given turns the environment off for both, and `_require_https_uri` names the +variable when its value is refused. `test_skills_fdv2.py` clears both variables with an autouse +fixture, so a developer shell cannot move the default-host assertions. + **A known stream event whose data is not JSON drops the connection; it is never skipped.** `_iter_sse` logs a WARNING and raises `_RecoverableTransportError`, as the base SDK's FDv2 stream interrupts on a `JSONDecodeError`, so the in-flight payload is abandoned and the diff --git a/packages/client/src/launchdarkly_ai_server/lifecycle.py b/packages/client/src/launchdarkly_ai_server/lifecycle.py index 0d626f2d..3690ec78 100644 --- a/packages/client/src/launchdarkly_ai_server/lifecycle.py +++ b/packages/client/src/launchdarkly_ai_server/lifecycle.py @@ -290,9 +290,9 @@ async def init_client( ) config_kwargs: dict[str, str] = {} - base_uri = opts.get("baseUri") or os.environ.get("LD_BASE_URI") - stream_uri = opts.get("streamUri") or os.environ.get("LD_STREAM_URI") - events_uri = opts.get("eventsUri") or os.environ.get("LD_EVENTS_URI") + base_uri = opts.get("baseUri") or _env("LD_BASE_URI") + stream_uri = opts.get("streamUri") or _env("LD_STREAM_URI") + events_uri = opts.get("eventsUri") or _env("LD_EVENTS_URI") if base_uri: config_kwargs["base_uri"] = base_uri if stream_uri: diff --git a/packages/client/src/launchdarkly_ai_server/skills_fdv2.py b/packages/client/src/launchdarkly_ai_server/skills_fdv2.py index 2c797f07..ecb49fd7 100644 --- a/packages/client/src/launchdarkly_ai_server/skills_fdv2.py +++ b/packages/client/src/launchdarkly_ai_server/skills_fdv2.py @@ -21,6 +21,7 @@ import json import logging import math +import os import random import re import socket @@ -73,6 +74,17 @@ than it polls from; a *base_uri* given without a *stream_uri* is used for both. """ + +def _endpoint_env(name: str) -> str | None: + """ + Reads ``LD_BASE_URI`` or ``LD_STREAM_URI``, the variables ``init_client`` + reads for the base SDK. Blank counts as unset: copied ``.env`` templates + ship these empty. + """ + value = os.environ.get(name, "").strip() + return value or None + + POLL_PATH = "/sdk/poll" STREAM_PATH = "/sdk/stream" @@ -1401,8 +1413,14 @@ def __init__( Given without *stream_uri*, it is used for streaming too (relays and private instances). - *stream_uri*: origin for ``GET /sdk/stream`` (default - ``DEFAULT_STREAM_URI``). Both must be ``https://``; ``http://`` is - accepted only for ``localhost``, ``127.0.0.1`` or ``::1``. + ``DEFAULT_STREAM_URI``, or the base URI when one is given or read from + ``LD_BASE_URI``). Both must be ``https://``; ``http://`` is accepted + only for ``localhost``, ``127.0.0.1`` or ``::1``. + + When neither is given, ``LD_BASE_URI`` and ``LD_STREAM_URI`` are read + in their place, once, here, so a process pointed at a non-production + instance sends skill delivery there too. The defaults apply after + that. - *mode*: ``"stream"`` (default, recommended: revocations arrive in seconds) or ``"poll"`` (revocations arrive within one *poll_interval*). - *poll_interval*: seconds between polls; positive and finite. @@ -1418,13 +1436,21 @@ def __init__( fatal status stops delivery and sets ``failed``. """ _require_server_side_credential(sdk_key) - # A lone ``base_uri`` serves both endpoints. + # The environment stands in for the options only when neither is + # given, so an explicit host is never joined with one read from the + # environment. + base_option, stream_option = "base_uri", "stream_uri" + if base_uri is None and stream_uri is None: + base_uri = _endpoint_env("LD_BASE_URI") + stream_uri = _endpoint_env("LD_STREAM_URI") + base_option, stream_option = "LD_BASE_URI", "LD_STREAM_URI" + # A lone base URI serves both endpoints. if stream_uri is None: stream_uri = DEFAULT_STREAM_URI if base_uri is None else base_uri if base_uri is None: base_uri = DEFAULT_BASE_URI - _require_https_uri(base_uri) - _require_https_uri(stream_uri, "stream_uri") + _require_https_uri(base_uri, base_option) + _require_https_uri(stream_uri, stream_option) if mode not in ("stream", "poll"): raise ValueError(f'mode must be "stream" or "poll", got {mode!r}') # ``nan`` and ``inf`` pass a ``<= 0`` check, but would make the poll diff --git a/packages/client/tests/test_lifecycle.py b/packages/client/tests/test_lifecycle.py index b9ddb4e1..c3729562 100644 --- a/packages/client/tests/test_lifecycle.py +++ b/packages/client/tests/test_lifecycle.py @@ -391,6 +391,42 @@ async def test_cancelled_call_closes_the_client_still_being_built(self) -> None: stub.close.assert_awaited_once() assert lifecycle_module._client is None + @staticmethod + async def _config_kwargs(env: dict[str, str]) -> dict[str, Any]: + mock_ld = MagicMock() + mock_ld.Config = MagicMock(return_value=MagicMock()) + mock_ld.LDClient = MagicMock(return_value=_make_stub_client()) + names = ("LD_BASE_URI", "LD_STREAM_URI", "LD_EVENTS_URI") + base = {k: v for k, v in os.environ.items() if k not in names} + with patch.dict( + os.environ, {**base, "LD_SDK_KEY": "test-key", **env}, clear=True + ): + with patch.object(lifecycle_module, "_setup_telemetry", return_value=None): + with patch("importlib.import_module", return_value=mock_ld): + await init_client() + return dict(mock_ld.Config.call_args.kwargs) + + async def test_strips_whitespace_from_the_uri_variables(self) -> None: + kwargs = await self._config_kwargs( + { + "LD_BASE_URI": " https://sdk.example.com\n", + "LD_STREAM_URI": "\thttps://stream.example.com ", + "LD_EVENTS_URI": " https://events.example.com ", + } + ) + assert kwargs == { + "base_uri": "https://sdk.example.com", + "stream_uri": "https://stream.example.com", + "events_uri": "https://events.example.com", + } + + @pytest.mark.parametrize("blank", ["", " "]) + async def test_a_blank_uri_variable_is_unset(self, blank: str) -> None: + kwargs = await self._config_kwargs( + {"LD_BASE_URI": blank, "LD_STREAM_URI": blank, "LD_EVENTS_URI": blank} + ) + assert kwargs == {} + # --------------------------------------------------------------------------- # OTel degradation diff --git a/packages/client/tests/test_skills_fdv2.py b/packages/client/tests/test_skills_fdv2.py index b35c5d72..26b323cc 100644 --- a/packages/client/tests/test_skills_fdv2.py +++ b/packages/client/tests/test_skills_fdv2.py @@ -74,7 +74,19 @@ _tombstone_from_delete, ) -pytestmark = pytest.mark.usefixtures("reset_skill_state") +pytestmark = pytest.mark.usefixtures("reset_skill_state", "clear_endpoint_env") + + +@pytest.fixture +def clear_endpoint_env(monkeypatch: pytest.MonkeyPatch) -> None: + """ + A store given no URI option reads ``LD_BASE_URI`` and ``LD_STREAM_URI``, so + a developer shell with either set would move every default this module + asserts. Both are cleared around each test, and restored after. + """ + monkeypatch.delenv("LD_BASE_URI", raising=False) + monkeypatch.delenv("LD_STREAM_URI", raising=False) + SDK_KEY = "sdk-00000000-0000-4000-8000-000000000000" SKILL_BODY = "---\nname: PDF Extraction\n---\nExtract text from PDFs.\n" @@ -4050,6 +4062,110 @@ def test_the_stream_request_goes_to_the_stream_host( assert endpoint.requests == [] +class TestEndpointsFromTheEnvironment: + """ + ``init_client`` reads ``LD_BASE_URI`` and ``LD_STREAM_URI`` for the base + SDK. A store that ignored them resolved flags against the instance the + environment named and sent skill delivery to production, with that + instance's SDK key: a fatal 401 in ``failed``, not a configuration error. + With no URI option, the store reads them as if they were the options. + """ + + @staticmethod + def _origins(store: FDv2SkillStore) -> tuple[str, str]: + requester = store._requester + return requester._base_uri, requester._stream_uri + + def test_ld_base_uri_alone_moves_both_endpoints( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + # The store's own rule for a lone base URI, not ``init_client``'s, + # which leaves flag streaming on its default host. + monkeypatch.setenv("LD_BASE_URI", "https://sdk.catfood.example.com") + assert self._origins(FDv2SkillStore(SDK_KEY)) == ( + "https://sdk.catfood.example.com", + "https://sdk.catfood.example.com", + ) + + def test_both_variables_set_each_endpoint( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("LD_BASE_URI", "https://sdk.catfood.example.com") + monkeypatch.setenv("LD_STREAM_URI", "https://stream.catfood.example.com/") + assert self._origins(FDv2SkillStore(SDK_KEY)) == ( + "https://sdk.catfood.example.com", + "https://stream.catfood.example.com", + ) + + def test_ld_stream_uri_alone_leaves_polling_on_its_default( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("LD_STREAM_URI", "https://stream.catfood.example.com") + assert self._origins(FDv2SkillStore(SDK_KEY)) == ( + DEFAULT_BASE_URI, + "https://stream.catfood.example.com", + ) + + def test_either_option_turns_both_variables_off( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Options win as a set: an explicit host is never mixed with one read + from the environment.""" + monkeypatch.setenv("LD_BASE_URI", "https://sdk.catfood.example.com") + monkeypatch.setenv("LD_STREAM_URI", "https://stream.catfood.example.com") + assert self._origins( + FDv2SkillStore(SDK_KEY, base_uri="https://relay.example.com") + ) == ("https://relay.example.com", "https://relay.example.com") + assert self._origins( + FDv2SkillStore(SDK_KEY, stream_uri="https://stream.example.com") + ) == (DEFAULT_BASE_URI, "https://stream.example.com") + + @pytest.mark.parametrize("blank", ["", " "]) + def test_a_blank_value_is_unset( + self, monkeypatch: pytest.MonkeyPatch, blank: str + ) -> None: + monkeypatch.setenv("LD_BASE_URI", blank) + monkeypatch.setenv("LD_STREAM_URI", blank) + assert self._origins(FDv2SkillStore(SDK_KEY)) == ( + DEFAULT_BASE_URI, + DEFAULT_STREAM_URI, + ) + + @pytest.mark.parametrize("name", ["LD_BASE_URI", "LD_STREAM_URI"]) + def test_a_plain_http_value_is_refused_by_the_variable_name( + self, monkeypatch: pytest.MonkeyPatch, name: str + ) -> None: + """The message names the variable, not an option the caller never + passed.""" + monkeypatch.setenv(name, "http://sdk.catfood.example.com") + with pytest.raises(ValueError, match="cleartext") as excinfo: + FDv2SkillStore(SDK_KEY) + message = str(excinfo.value) + assert name in message + assert "base_uri" not in message + assert "stream_uri" not in message + + def test_the_variables_are_read_once_at_construction( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("LD_BASE_URI", "https://sdk.catfood.example.com") + store = FDv2SkillStore(SDK_KEY) + monkeypatch.setenv("LD_BASE_URI", "https://sdk.elsewhere.example.com") + monkeypatch.setenv("LD_STREAM_URI", "https://stream.elsewhere.example.com") + assert self._origins(store) == ( + "https://sdk.catfood.example.com", + "https://sdk.catfood.example.com", + ) + + def test_the_module_defaults_keep_the_production_hosts( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("LD_BASE_URI", "https://sdk.catfood.example.com") + FDv2SkillStore(SDK_KEY) + assert DEFAULT_BASE_URI == "https://sdk.launchdarkly.com" + assert DEFAULT_STREAM_URI == "https://stream.launchdarkly.com" + + # --------------------------------------------------------------------------- # The eager re-reconcile, end to end over the transport # --------------------------------------------------------------------------- @@ -4350,6 +4466,7 @@ def test_it_adds_no_dependency(self) -> None: "json", "logging", "math", + "os", "random", "re", "socket",