From 7424cef7c66743165873093ef85ba3d4d1c43b62 Mon Sep 17 00:00:00 2001 From: Tudor Date: Sun, 23 Aug 2026 10:52:55 +0100 Subject: [PATCH] feat(flags): the registry and a fail-closed Unleash client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unleash holds flag state; it does not hold the list of flags. REGISTRY is that list, because the SDK evaluates an unknown flag to False and without a registry that is an undeclared False — indistinguishable from a typo in a flag name. Fail-closed throughout, and never raises: an unset UNLEASH_URL, an unreachable server, a client that throws, an undeclared name — all False. A flag layer that can 500 a request path or stop the API booting is worse than one that is switched off. Every flag defaults to False, with no per-flag override, because a flag that defaults on is a kill switch and this is deliberately not one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj --- backend/config.py | 10 ++++ backend/flags.py | 111 ++++++++++++++++++++++++++++++++++++ backend/tests/test_flags.py | 74 ++++++++++++++++++++++++ requirements.txt | 2 +- 4 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 backend/flags.py create mode 100644 backend/tests/test_flags.py diff --git a/backend/config.py b/backend/config.py index bdaa9dc..554efc9 100644 --- a/backend/config.py +++ b/backend/config.py @@ -42,6 +42,16 @@ class Settings(BaseSettings): typesense_url: str = "http://localhost:8108" typesense_api_key: str = "" + # Feature flags (Unleash). An empty unleash_url disables flags entirely and + # every flag evaluates False — the correct behaviour for local development + # and CI, and the reason no test needs a running Unleash. + unleash_url: str = "" + unleash_api_token: str = "" + unleash_app_name: str = "schoolcompare-backend" + # On a named volume, so a restart during an Unleash outage keeps + # last-known state instead of reverting a released feature to dark. + unleash_cache_directory: str = "/app/.unleash" + # Analytics ga_measurement_id: Optional[str] = "G-J0PCVT14NY" # Google Analytics 4 Measurement ID diff --git a/backend/flags.py b/backend/flags.py new file mode 100644 index 0000000..f843828 --- /dev/null +++ b/backend/flags.py @@ -0,0 +1,111 @@ +"""Feature flags: what can be switched, and what is switched right now. + +Ship-dark, not a kill switch. Flags let work merge and deploy without becoming +visible; they are expected to flip about monthly, by a person, deliberately. +Nothing here does percentage rollouts or user targeting — the site has no user +identity to target. + +Unleash holds the state. It does not hold the list. REGISTRY below is that +list, and it exists for three reasons: the SDK evaluates an unknown flag to +False, so without a registry that is an *undeclared* False, indistinguishable +from a typo; /api/flags needs a key set to return when Unleash is unreachable; +and a flag in the UI but not in the registry is orphaned and should be visibly +so rather than quietly authoritative. + +Every flag defaults to False. There is no per-flag default, because a flag that +defaults on is a kill switch, and this is not one. +""" + +from __future__ import annotations + +import logging +from dataclasses import dataclass +from datetime import date + +from .config import settings + +logger = logging.getLogger(__name__) + +# A flag is temporary scaffolding. See test_a_flag_older_than_the_limit. +MAX_FLAG_AGE_DAYS = 90 + + +@dataclass(frozen=True) +class Flag: + # One string: the registry key, the Unleash flag name, and the JSON key in + # /api/flags. snake_case, matching the API's existing convention. No case + # transformation anywhere, so there is no mapping layer to get wrong. + name: str + description: str # one line: what turning this on reveals + added: date # for the staleness tripwire + + +REGISTRY: dict[str, Flag] = { + f.name: f for f in ( + Flag( + name="admission_distance", + description=( + "The last-distance-offered figure on the Admissions tile and " + "the 'How far away are you?' section on school pages." + ), + added=date(2026, 8, 23), + ), + ) +} + + +_client = None + + +def init() -> None: + """Start the Unleash client, or log why flags are all off. + + Called once from the app lifespan. Never raises: a flag system that can + stop the API from booting is worse than one that is switched off. + """ + global _client + if not settings.unleash_url or not settings.unleash_api_token: + logger.warning( + "Unleash is not configured (UNLEASH_URL / UNLEASH_API_TOKEN); " + "every feature flag evaluates to False.") + return + + try: + from UnleashClient import UnleashClient + + _client = UnleashClient( + url=settings.unleash_url, + app_name=settings.unleash_app_name, + custom_headers={"Authorization": settings.unleash_api_token}, + cache_directory=settings.unleash_cache_directory, + refresh_interval=15, + ) + _client.initialize_client() + logger.info("Unleash client initialised against %s", settings.unleash_url) + except Exception: + # Fail closed and keep serving. The SDK also evaluates everything False + # until its first successful sync, so this is the same direction. + _client = None + logger.exception("Unleash client failed to start; flags are all False.") + + +def is_enabled(name: str) -> bool: + """Whether `name` is on. False for anything unknown, unreachable or broken.""" + if name not in REGISTRY: + logger.error( + "undeclared feature flag %r was evaluated; returning False. " + "Add it to backend/flags.py REGISTRY or fix the name.", name) + return False + if _client is None: + return False + try: + return bool(_client.is_enabled( + name, fallback_function=lambda feature_name, context: False)) + except Exception: + logger.exception("flag %r failed to evaluate; returning False", name) + return False + + +def all_flags() -> dict[str, bool]: + """Every declared flag and its current value. Serves /api/flags.""" + return {name: is_enabled(name) for name in REGISTRY} diff --git a/backend/tests/test_flags.py b/backend/tests/test_flags.py new file mode 100644 index 0000000..5aa0147 --- /dev/null +++ b/backend/tests/test_flags.py @@ -0,0 +1,74 @@ +"""Tests for the feature flag layer (spec 2026-08-23). + +None of these need a running Unleash. That is the point: an unset UNLEASH_URL +means every flag is False, which is what local development and CI get. +""" + +from datetime import date, timedelta + +from backend import flags + + +def test_every_declared_flag_is_keyed_by_its_own_name(): + # One string is the registry key, the Unleash flag name and the JSON key. + # A mismatch here would mean the UI toggles a flag the code never reads. + for key, flag in flags.REGISTRY.items(): + assert key == flag.name + + +def test_flag_names_are_snake_case(): + # Matches the API's existing convention (admission_distance, + # rwm_expected_pct) so no case transformation exists to get wrong. + for name in flags.REGISTRY: + assert name == name.lower() + assert "-" not in name and " " not in name + + +def test_an_unconfigured_client_evaluates_every_flag_false(monkeypatch): + monkeypatch.setattr(flags, "_client", None) + for name in flags.REGISTRY: + assert flags.is_enabled(name) is False + + +def test_an_undeclared_flag_is_false_rather_than_an_error(monkeypatch): + # A typo'd flag name must not raise in a request path. It is logged as an + # error, because an undeclared flag is always a bug. + monkeypatch.setattr(flags, "_client", None) + assert flags.is_enabled("no_such_flag") is False + + +def test_an_exploding_client_is_false_rather_than_a_500(monkeypatch): + class Boom: + def is_enabled(self, *a, **kw): + raise RuntimeError("unleash is on fire") + + monkeypatch.setattr(flags, "_client", Boom()) + name = next(iter(flags.REGISTRY)) + assert flags.is_enabled(name) is False + + +def test_all_flags_reports_every_declared_flag(monkeypatch): + monkeypatch.setattr(flags, "_client", None) + assert set(flags.all_flags()) == set(flags.REGISTRY) + assert all(v is False for v in flags.all_flags().values()) + + +def test_a_flag_older_than_the_limit_fails_this_test(): + """A tripwire, not an assertion about correctness. + + Flags are temporary scaffolding and the failure mode of every flag system + is accumulation. This fails on the day a flag turns 90, on whatever PR + happens to be open — which is the point: someone has to decide. + + To fix: delete the flag and the branches that read it, or, if it genuinely + still needs to exist, move its `added` date and say why in the commit. + """ + stale = [ + f.name for f in flags.REGISTRY.values() + if date.today() - f.added > timedelta(days=flags.MAX_FLAG_AGE_DAYS) + ] + assert not stale, ( + f"Flags older than {flags.MAX_FLAG_AGE_DAYS} days: {stale}. " + "Remove the flag and the code branches it guards, or move its `added` " + "date deliberately." + ) diff --git a/requirements.txt b/requirements.txt index f5ab98a..3b82b3f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,4 +12,4 @@ slowapi==0.1.9 secure==0.3.0 typesense==0.21.0 numpy==1.26.4 - +UnleashClient==6.0.1