Repository navigation
feat: Add file data loading code with reload, retry, and polling - #525
kinyoklion wants to merge 6 commits into
Conversation
c478bd1 to
7e64242
Compare
Adds ldclient.impl.integrations.files.filedata, the file reading, parsing, and merging logic that the file-based override source is built on. A document that starts with an opening brace is parsed as JSON and any other document as YAML. Definitions are decoded into the flag and segment models while the file is read, so an invalid definition fails that load. Files are merged in the configured order with a duplicate keys handling of fail or ignore, and the result records how many entries each file supplied. The Reloader owns the reload cycle: it serializes reloads, debounces change signals with a settle window, keeps the last good data by not applying a failed load, retries a failed load after a bounded delay, reports an identical failure once, and skips an application whose file contents did not change. It can treat a configured file that does not exist as a file with no content. The Poller detects changes by comparing modification time and size on an interval, including files that appear or disappear. The Watcher uses the watchdog package, watches the directory of each file so an absent file is picked up when it appears, matches the destination of a move so a file written by rename is detected, retries a directory that does not exist yet, and reacts only to notifications that can change a file's content or presence. The existing file data sources are not changed and keep their current behavior. A new test module pins that behavior: the flagValues expansion and its evaluation reason, the version fallback, the failure messages, the FDv2 status and error kinds, and the polling and watching rules.
7e64242 to
ee3234c
Compare
…st watch The reloader treats an exception from apply like a load failure: it is reported, retried, and the result is not remembered as the last good one. An exception from the error callback is logged and does not stop the retry. The watcher retries when the observer cannot start, watches a directory again after it is deleted and recreated, and closes without error when the observer never started.
|
bugbot review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b309a87. Configure here.
A plain rewrite is not atomic: the truncation and the write are separate steps, and a poll that lands between them legitimately reports an extra change. The poller promises generous detection, not one callback per logical change; the reloader coalesces and skips the excess. The test that expected exactly one callback could therefore fail on a busy machine. It now drains the callbacks of the write and then requires silence, which still fails for a poller that does not update its baseline.
|
The file data poller test that expected exactly one callback per rewrite is replaced by one that checks the poller goes quiet once the file is stable, following launchdarkly/go-server-sdk#473. A plain rewrite is not atomic (the write and the modification-time update are separate steps), so a poll that lands between them legitimately reports an extra change; the poller promises generous detection and the reloader absorbs the excess. The test writes in the ordinary way, drains the callbacks of the write, and then requires silence, which still fails for a poller that does not update its baseline. Written by Claude Code on behalf of @kinyoklion. |
…failure is reported The reloader test wrote a file with a new failure while the automatic retries were running and expected the next retry to report it. The write is ordinary, not atomic, so a retry can read the file while it is empty or half written. An empty document is valid, so such a retry succeeds and ends the retries, and the new failure is never read; a half-written file is reported as a failure of its own. The test now loads the final content on demand and inspects the reports as a whole, and a fixture helper collects the callbacks that arrive within a settle period.
|
The Reproduced locally with a deliberately slow write (truncate, sleep 200 ms, write): the old test shape failed 3/3 with the same timeout and the new one passed 3/3. The test now loads the final content with Written by Claude Code on behalf of @kinyoklion. |
Summary
Adds
ldclient.impl.integrations.files.filedata, the file reading, parsing, and merging logic that the file-based override source described by the OVERRIDE specification is built on. The specification requires ordered multi-file merging with duplicate keys handling, a reload that keeps the last good data across a malformed edit and retries, debouncing of change notifications, a polling change detector as well as a watching one, and correct handling of a configured file that does not exist yet. This module provides those pieces; the override source itself is added in a later PR of the stack.The existing file data sources are not changed. The FDv1 file update processor (
Files.new_data_source) and the FDv2 file initializer and synchronizer (Files.new_data_source_v2,datasystem.file_ds_builder) keep their current implementation and behavior in every respect: theflagValuesexpansion (an on flag with fallthrough variation 0 and version 1, evaluated with aFALLTHROUGHreason), the version fallback that stamps version 1 on entries without one, the failure messages and their levels, the FDv2 synchronizer'sOFFstate on a failed initial load and itsINVALID_DATAerror kind, and the polling and watching rules. A new test module,test_file_data_sources_pinned_behavior.py, pins each of those behaviors so the override feature stays purely additive. The baseline test files for the existing sources are unmodified.What the new module provides for the override source:
pyyamlis installed, and with the standard JSON parser otherwise. The content is never inspected to choose a parser. Flag and segment definitions are decoded into the model classes while the file is read, so an invalid definition fails that load. A definition may omit its own key and version, which are filled in from the map key and a version of 1.fail(the load fails) orignore(the first file's entry is kept). The result records how many entries each file supplied.Reloader. Serializes reloads, debounces change signals with a settle window that each signal extends, keeps the last good data by not applying a failed load, retries a failed load after a bounded delay so a file observed mid-write recovers without another notification, reports an identical failure once, skips an application whose file contents are byte-identical to the last applied contents, and applies a success after a failure even when the contents did not change. It can treat a configured file that does not exist as a file with no content. Closing does not wait for an in-flight reload. An exception raised byapplyoron_erroris reported like any other load failure and retried, and nothing from that load is remembered.Poller. Compares modification time and size on an interval, so a same-size rewrite and a same-time size change are both detected, as are files that appear or disappear.Watcher. Uses thewatchdogpackage. It watches the directory of each file so an absent file is picked up when it appears, matches the destination of a move event so a file written by rename is detected, retries a directory that does not exist yet, and reacts only to notifications that can change a file's content or presence (the watchdog inotify mask also reports a file being opened or read). A failed observer start is retried every second, and a watched directory that is deleted is watched again once it exists, with one change signaled.Tests cover parsing, merging, loading, the reloader (initial load, failure retention, debounce coalescing and window extension, retry with and without further signals, identical-failure reporting, skip-unchanged and recovery, serialization of concurrent reloads, close semantics, worker thread lifecycle), the poller, and the watcher, plus the pinned behavior of the existing sources.
SDK-3250
Note
Overview
Introduces
ldclient.impl.integrations.files.filedata, shared infrastructure for a future file-based flag override source: JSON/YAML parsing (flags,flagValues,segments), ordered multi-file merge with fail vs ignore duplicate keys, and optional skip missing paths.Adds a
Reloader(debounced triggers, bounded retries, last-good data on failure, SHA-256 skip unchanged, recovery apply after errors) plusPoller(mtime + size, appear/disappear) andWatcher(watchdog with directory retry and filtered event types)—behavior that intentionally differs from today’s FDv1/FDv2 pollers/watchers.No changes to
Files.new_data_sourceorFiles.new_data_source_v2;test_file_data_sources_pinned_behavior.pylocks their current semantics (messages,flagValuesexpansion, polling/watching rules).test_filedata.pycovers the new module end-to-end.Reviewed by Cursor Bugbot for commit b309a87. Bugbot is set up for automated code reviews on this repo. Configure here.