Skip to content

Evaluate check_flags from the cache when the replicator is not ready - #133

Closed
ryanechternacht wants to merge 1 commit into
mainfrom
ryan/replicator-check-flags-not-ready
Closed

ryanechternacht wants to merge 1 commit into
mainfrom
ryan/replicator-check-flags-not-ready

Conversation

@ryanechternacht

Copy link
Copy Markdown
Member

Problem

In replicator mode the SDK polls the replicator's health endpoint and treats its ready field as "connected". When a customer's Schematic account is closed or Schematic is unreachable, the replicator stays up and keeps its Redis cache, but reports ready: false.

AsyncSchematic.check_flags gated its DataStream path on ds.is_connected(), which in replicator mode returns the replicator's ready flag. With the replicator not ready, check_flags skipped the cache entirely and called the bulk /flags/check API. In the closed-account or unreachable case that call fails too, so every requested key came back as its configured default with an error reason, even though the cache still held the company and flags.

Findings

Behavior today when the replicator reports ready: false:

  • check_flag / check_flag_with_entitlement: no readiness gate. DataStreamClient.check_flag in replicator mode evaluates with whatever the cache holds, matching Go. Unchanged by this PR.
  • check_flags (with keys): gated on is_connected(), so it went to the bulk API and returned defaults when that failed. Fixed here.
  • check_flags with no keys: always uses the bulk API (the "all flags" shape only exists there), same as Go. Unchanged.
  • The sync Schematic client has no DataStream support, so none of this applies to it.

The Go SDK's CheckFlags has no connection gate: it evaluates through DataStream whenever DataStream is configured and keys are given, and falls back to the API only when local evaluation fails. The gate here came in with the original DataStream port (#58) with no discussion of it in review.

Change

Remove ds.is_connected() from the check_flags DataStream condition. In replicator mode, keys now evaluate from the cache regardless of readiness. In WebSocket mode a disconnected client still answers from the cache when the entities are there, and raises when they are not, which falls back to the bulk API as before. That matches what check_flag already did in both modes.

The README's replicator section said the client would "fall back to direct API calls if the replicator is not available". That was already inaccurate for check_flag, so it now describes the cache-first behavior.

Test

  • New tests/custom/test_replicator_flag_checks.py builds an AsyncSchematic in replicator mode on a fake Redis, drives one health poll that returns {"ready": false}, seeds a company whose override grants the flag (flag default is false), and makes the API raise. check_flag and check_flags both return the cached verdict with the company id and never call the API. The check_flags test fails on main.
  • test_check_flags_skips_datastream_when_not_connected asserted the old gate. It is replaced by test_check_flags_uses_datastream_when_not_connected, which checks that a not-connected DataStream client is still consulted and the bulk API is not called. The existing test_check_flags_datastream_failure_falls_back_to_bulk_api still covers the fallback.
  • poetry run pytest -n auto .: 757 passed, 3 skipped. poetry run mypy .: clean. ruff check on the changed files: clean.

🤖 Generated with Claude Code

check_flags gated its DataStream path on is_connected(), which in
replicator mode is the replicator's ready flag. When a replicator reports
ready: false (account closed, Schematic unreachable) it keeps its Redis
cache, but check_flags skipped that cache and went to the bulk API, which
cannot answer either, so callers got flag defaults. check_flag had no such
gate and kept evaluating from the cache.

Drop the gate so check_flags matches check_flag and the Go SDK: evaluate
requested keys through DataStream and fall back to the bulk API only when
local evaluation raises.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@ryanechternacht

Copy link
Copy Markdown
Member Author

Closing in favor of fixing this in the Replicator. The SDK is right to gate flag checks on Replicator readiness; the Replicator should keep reporting ready in /health once its cache is fully loaded, including after it loses its connection to Schematic. A Replicator PR is coming for that. The separate track optimistic-update PR stays open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants