feat(flags): the registry and a fail-closed Unleash client
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
This commit is contained in:
1 parent
01ccbb8e82
commit
7424cef7c6
4 files changed
+196
-1
No files matched your search
@@ -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
|
||||
|
||||
|
||||
@@ -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}
|
||||
@@ -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."
|
||||
)
|
||||
+1
-1
@@ -12,4 +12,4 @@ slowapi==0.1.9
|
||||
secure==0.3.0
|
||||
typesense==0.21.0
|
||||
numpy==1.26.4
|
||||
|
||||
UnleashClient==6.0.1
|
||||
Reference in new issue
Block a user