fix: Accept a path object as an override file path by kinyoklion · Pull Request #572 · launchdarkly/python-server-sdk

kinyoklion · 2026-11-09T23:14:44Z

Requirements

I have added test coverage for new or changed functionality
I have followed the repository's pull request submission guidelines
I have validated my changes against all supported platform versions
Sync/async parity: matching changes made to async_* siblings (or N/A). N/A: LDClient and AsyncLDClient use the same FileOverrideSourceBuilder.

Related issues

Bugbot finding on feat: Add the file-based override source #529: feat: Add the file-based override source #529 (comment)

Describe the solution you've provided

FileOverrideSourceBuilder accepted one path as a str, or a list of paths. A single pathlib.Path, or any other os.PathLike, was passed to list(). A path object is not iterable, so the constructor raised TypeError: 'PosixPath' object is not iterable. The Bugbot finding describes a different failure, a path split into its components that fails every reload, but that cannot happen. The constructor raises before a client exists.

The constructor now treats a str or an os.PathLike as a single path, and converts every path with os.fspath. A path that does not name a string path raises TypeError from the constructor, with a message that names the value. That includes bytes (given alone, in a list, or named by a path object), an int, and None. bytes and bytearray count as single values, so a bare bytes path is named whole instead of being read as a sequence of integers. The type hint is Sequence rather than List, so that type checkers accept a list of path objects but not a set, whose order is not fixed. The docstring says that paths may be path objects.

Tests cover:

a single path object;
a list of path objects, which already worked;
a path object whose string form differs from its path, as with os.DirEntry;
a source that loads a file given as a path object;
the rejection of each of the values above.

Each of six broken variants of the constructor fails at least one of these tests, including the original code and a variant that converts with str() instead of os.fspath.

Describe alternatives you've considered

Leaving the builder to accept only strings, as its type hint said. pathlib is the usual way to build paths, a list of path objects already worked, and the error did not say what was wrong.

Additional context

This PR is stacked on #530, the top of the overrides stack, so that the approved PRs below it do not change. Retarget it as the PRs below it merge.

The review of this change found three existing gaps that this PR leaves alone:

An empty path resolves to the working directory, and its reads keep failing.
In polling mode, a path that contains a NUL character makes the source fail to start, so the client runs without overrides. In watching mode, the reload fails and keeps retrying.
The file data sources treat a tuple of paths differently from a list.

Testing: mypy ldclient, isort --check and pycodestyle pass locally. The test suite passes (2,111 tests). It was run without the Redis, DynamoDB and Consul store integration tests, which need servers that were not running. CI runs only for PRs based on main or feat/**, so it will not run on this PR until it is retargeted.

_{Opened from termag}

Note

Overview
Fixes a constructor bug where a single pathlib.Path (or other os.PathLike) was passed to list(), causing TypeError: 'PosixPath' object is not iterable.

FileOverrideSourceBuilder now wraps a lone str, bytes/bytearray, or PathLike as one path, normalizes every entry with os.fspath() (so objects like os.DirEntry use their filesystem path, not str()), and stores string paths. Invalid inputs (bytes paths, ints, None, path objects that do not resolve to a str) fail fast with an explicit TypeError. Type hints use Sequence plus PathLike so callers can pass path objects while unordered collections like set are discouraged.

New tests cover single/list path objects, __fspath__ behavior, invalid types, and loading overrides from a path object.

^{Reviewed by Cursor Bugbot for commit 154b5b8. Bugbot is set up for automated code reviews on this repo. Configure here.}

kinyoklion · 2026-11-09T23:14:54Z

bugbot review

cursor

Stale Bugbot comment from a previous run.

kinyoklion · 2026-11-09T23:34:11Z

bugbot review

cursor

✅ 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 a1bda43. Configure here.}

FileOverrideSourceBuilder passed anything other than a str to list(), so a single pathlib.Path raised TypeError from the constructor. A str or an os.PathLike is now one path, and every path is converted with os.fspath. A path that does not name a string path raises TypeError.

A bare bytes or bytearray value is a single path, and a TypeError from os.fspath is replaced by the builder's own message, so every rejected path is named. The type hint is Sequence, so that type checkers reject a set, whose order is not fixed. Tests cover a path object whose string form differs from its path and every rejected value.

**Requirements** - [x] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [ ] I have validated my changes against all supported platform versions (run on Python 3.13 only; CI does not run for this base) - [x] Sync/async parity: matching changes made to `async_*` siblings (or N/A). The fix is in the override layer and sink, which `LDClient` and `AsyncLDClient` share. The tests cover both clients. **Related issues** - Implements the "reject deleted entries" rule of launchdarkly/sdk-specs#288 (OVERRIDE Requirements 1.1.1.1, 1.2.8.1 and 1.3.6.2). - Fixes the deleted-entry defect found in the review of #527. - Stacked on #572 (`rlamb/overrides-python-file-source-paths`) as a new PR, so the PRs below it do not change. **Describe the solution you've provided** Before this change, a flag or segment with `"deleted": true` went into the override layer, from a custom source or from an override file. The file source counted the entry as an override in effect. The overlay served it, and `variation()` and `variation_detail()` raised `AttributeError: _track_events` into the application, in both `LDClient` and `AsyncLDClient`. A flag that depended on it evaluated with reason `EXCEPTION` and without `overrideAffected`. After this change: - **Sink and layer (1.1.1.1, 1.2.8.1).** When a snapshot contains a flag or segment marked as deleted, as a dictionary or a model object, the sink rejects the whole snapshot. The layer is unchanged and no change notifications are sent. The rejection is logged at Error level and names the kind and the key, for example `Rejected an override snapshot; the overrides in effect stay: flag "f" is marked as deleted; an override is removed by leaving it out of the snapshot`. `set_overrides` raises `ValueError`, as it already does for an invalid definition, so the data system handles both the same way. If a source lets the error propagate out of `start`, the existing start-failure handling applies: the error is logged, the source is closed, and the client runs without overrides. A rejected later update keeps the overrides in effect. The error goes back to the source's own caller. - **File override source (1.3.6.2).** The source checks for deleted entries when it parses each file, before it calls the sink. The reload fails the same way as for a file that cannot be parsed. The previous overrides stay, and the failure is logged like other load failures, naming the file and the key: `Unable to load flag data: flag "f" in /path/overrides.json is marked as deleted; remove the entry to remove its override`. The existing retry applies. - An entry with `"deleted": false` is an ordinary definition and still loads. - **Docs.** `FileOverrideSourceBuilder` and `OverrideSink.set_overrides` each get one user-level sentence. **The ordinary file data sources are unchanged on purpose.** `Files.new_data_source` and `Files.new_data_source_v2` do not use the override source's parsing code, and they still accept deleted entries. Rejecting deleted entries there would be a breaking change, so it is left for a later major version. New tests in `test_file_data_sources_pinned_behavior.py` pin the current behavior: the FDv1 source stores such entries as deleted, and the FDv2 source passes them on unchanged. **Tests** - `testing/impl/test_overrides.py`: the layer and the sink reject a deleted flag and a deleted segment, each as a dictionary and as a model, with and without listeners. In every case the layer is unchanged, nothing is notified, exactly one Error is logged, and the next snapshot is accepted. A definition with `deleted: false` is accepted. - `testing/test_ldclient_overrides.py` and `testing/test_async_client_overrides.py` use a custom source. If it supplies a deleted flag at start, the client serves LaunchDarkly data for the flag and its dependent. If it supplies one in a later update, `ValueError` is raised to the source, the override and the dependent's `overrideAffected` reason stay, and nothing is notified. In neither case do `variation()` or `variation_detail()` raise. - `testing/integrations/test_file_override_source.py`: - An initial load with a deleted flag or segment fails, in JSON and in YAML, and applies none of the other file's overrides. - A reload with a deleted entry keeps the previous overrides and logs the file and the key. The retry then picks up the fix with no change signal. - `deleted: false` loads. - A client with such a file serves no override. - `testing/integrations/test_filedata.py`: `load_files` and `read_file` raise `DeletedEntryError`, a `FileReadError`, and its message names the file and the key. `deleted: false` loads. - `testing/integrations/test_file_data_sources_pinned_behavior.py`: pins that the ordinary data sources are unchanged. I ran every rejection test against the base commit without the fix, and each one fails there. The `deleted: false` tests and the pins pass both with and without the fix, as expected. How I ran them, with the clone's venv and this worktree on `PYTHONPATH`: ``` python -m pytest -q -p no:cacheprovider ldclient --ignore-glob='*dynamodb*' --ignore-glob='*redis*' --ignore-glob='*consul*' # 2142 passed mypy ldclient # no issues isort --check --atomic ldclient contract-tests # clean pycodestyle ldclient contract-tests # clean ``` **Describe alternatives you've considered** - Dropping only the deleted entry and applying the rest. The spec requires rejecting the whole snapshot, so that the layer always holds exactly one snapshot that a source supplied. - Serving a deleted entry as an absent flag. That would hide LaunchDarkly data for the key, and overrides only add or replace definitions. **Additional context** Python CI runs only for PRs based on `main` or `feat/**`, so it does not run on this PR.   Opened from termag  --- > [!NOTE] > **Overview** > Override snapshots and file-based override loads now **reject** any flag or segment with `"deleted": true`, instead of accepting them into the override layer (which could break evaluation). > > **File parsing (`filedata`)** adds `DeletedEntryError` and `_reject_deleted_entries`, run after parse in `read_file` / `_parse_file`, so a reload with a deleted entry fails like a parse error and prior overrides stay in effect. > > **Override application** adds `DeletedItemError` in `OverrideLayer.set_all`; `OverrideSinkImpl` logs and re-raises so nothing is applied and listeners are not notified. Public docs on `FileOverrideSourceBuilder` and `OverrideSink.set_overrides` state that removal is done by **omitting** the entry, not by marking it deleted. > > **Legacy file data sources (FDv1/FDv2)** are unchanged and still accept deleted entries; new pinned tests document that behavior. > > Tests cover layer, sink, sync/async clients, file override source (JSON/YAML), and `load_files`. > > Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c6dcabc. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).

…ees the malformed write (#576)  **Requirements** - [x] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [ ] I have validated my changes against all supported platform versions - [x] Sync/async parity: matching changes made to `async_*` siblings (or N/A). N/A: a test-only change to the file override source tests. **Related issues** `test_source_retries_a_failed_load_without_a_change_signal` failed on `windows (3.10)` in the CI of #572, with `AssertionError: the load failure was not logged`. The test passed on every other Windows and Linux version. **Describe the solution you've provided** The test writes valid content, then malformed content, then fixed content. The fixed content keeps the malformed content's size and modification time, so that only the automatic retry can recover. The initial content had that same size too. The poller compares a file's modification time and size. Where file times are coarse, as on Windows, the initial and malformed writes can share a modification time, so the poller saw no change. Nothing was then logged, and the test timed out. The initial content now has a different size from the malformed content, so the poller always sees the malformed write. The part of the test that depends on an unchanged size and time is unchanged. To confirm the cause, I patched `_observe` to round modification times down to whole seconds and ran the test. It failed after its 10-second timeout without this change, and it passes with it. The whole test file passes normally (72 tests), and `isort` and `pycodestyle` are clean. **Describe alternatives you've considered** Setting the file's modification time after the malformed write. Changing the size states the intent in the content itself, and needs no extra system call. **Additional context** This merges cleanly with #574, which adds tests to the same file.   Opened from termag  --- > [!NOTE] > **Overview** > Stabilizes **`test_source_retries_a_failed_load_without_a_change_signal`** on Windows by changing the **initial** override JSON so its **file size differs** from the later malformed payload. > > The test still relies on malformed and fixed content having the **same length and mtime** so recovery must come from **automatic retry**, not polling. On platforms with coarse mtimes, the old initial content could match the malformed write on size and time, so the poller missed the bad edit and the expected load-failure log never appeared. Comments were added to document that intent. > > Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 0d9ba1c. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).

BEGIN_COMMIT_OVERRIDE feat: Add experimental support for flag overrides, read from local JSON or YAML files or supplied by a custom override source feat: Mark evaluations that a flag override affected with `overrideAffected` in the evaluation reason, and count them under separate summary counters fix: Stop the components that already started when the client fails to start END_COMMIT_OVERRIDE Every change in this PR was already reviewed and merged through its own PR against `feat/overrides`: #525, #526, #527, #528, #529, #530, #572, #574, #576, #577. This PR merges the branch into `main`. Flag overrides let an operator force flags and segments to known definitions on a running client, whether or not the client can reach LaunchDarkly, and remove the override when the incident is over. They are an option of the FDv2 data system, on both `LDClient` and `AsyncLDClient`, with a file-based source in `ldclient.integrations.overrides.FileOverrideSourceBuilder`, and are experimental. The one change outside the feature: #527 stops the components that had already started when the data system fails to start, so a failed client construction no longer leaves threads behind. `main` was merged into `feat/overrides` in 46e5f0e, so the branch has every `main` commit. That merge resolved two conflicts and made one follow-up change: - The capability lists of both contract test services keep both sides: `flag-overrides` from this branch, and the retry conformance, service endpoints, wrapper, proxy and TLS capabilities from `main`. - One async override test used `AsyncHook`, which #548 removed from `main`. It now uses `Hook`, which the async client accepts. Before the push: - the unit test suite passed (2,357 tests, without the Redis, DynamoDB and Consul store tests, which need servers); - `mypy`, `isort` and `pycodestyle` were clean; - the contract test harness's `flag overrides` suite passed against both the sync and the async test service (88 tests each).   Opened from termag  --- > [!NOTE] > **Overview** > Adds **experimental flag overrides** so operators can force known flag/segment definitions at runtime (via local JSON/YAML files through `FileOverrideSourceBuilder`, or a custom `OverrideSource`), integrated into the FDv2 data system for sync and async clients through `datasystem_config.override_source` / `ConfigBuilder.overrides()`. > > The data system now maintains an **override layer** merged over the normal store, serves override-only flags when LaunchDarkly data is not ready, and fans out **dependency-aware flag-change notifications** when LD data affects overridden prerequisites/segments. Evaluations propagate an **`overrideAffected`** reason marker; analytics treat those evaluations differently (summary counters only, `all_flags_state` disables per-flag tracking for them). > > Contract test services advertise **`flag-overrides`** and build override sources from harness config; docs add `ldclient.integrations.overrides`. > > Also **fixes client startup**: if the data system fails to start, already-started components are stopped so failed construction does not leak threads. > > Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 46e5f0e. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).

cursor Bot reviewed Nov 9, 2026

View reviewed changes

kinyoklion mentioned this pull request Nov 10, 2026

fix: Reject deleted flags and segments in flag overrides #574

Merged

3 of 4 tasks

kinyoklion marked this pull request as ready for review November 10, 2026 20:55

kinyoklion requested a review from a team as a code owner November 10, 2026 20:55

keelerm84 approved these changes Nov 10, 2026

View reviewed changes

kinyoklion force-pushed the rlamb/overrides-python-contract-tests branch from 7e42fc2 to 5d19e57 Compare November 10, 2026 22:24

kinyoklion force-pushed the rlamb/overrides-python-file-source-paths branch from a1bda43 to 5981654 Compare November 10, 2026 22:24

Base automatically changed from rlamb/overrides-python-contract-tests to feat/overrides November 10, 2026 23:27

kinyoklion added 2 commits November 10, 2026 23:29

kinyoklion force-pushed the rlamb/overrides-python-file-source-paths branch from 5981654 to 154b5b8 Compare November 10, 2026 23:29

kinyoklion merged commit 25323bf into feat/overrides Nov 10, 2026
14 of 16 checks passed

kinyoklion deleted the rlamb/overrides-python-file-source-paths branch November 10, 2026 23:32

kinyoklion mentioned this pull request Nov 10, 2026

test: Change the file size in the override retry test so the poller sees the malformed write #576

Merged

3 of 4 tasks

kinyoklion mentioned this pull request Nov 12, 2026

feat: Add experimental support for flag overrides #580

Merged

Navigation Menu

Repository navigation

fix: Accept a path object as an override file path - #572

fix: Accept a path object as an override file path#572
kinyoklion merged 2 commits into
feat/overridesfrom
rlamb/overrides-python-file-source-paths

kinyoklion commented Nov 9, 2026 •

edited by cursor Bot

Loading

Uh oh!

kinyoklion commented Nov 9, 2026

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

kinyoklion commented Nov 9, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

kinyoklion commented Nov 9, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kinyoklion commented Nov 9, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kinyoklion commented Nov 9, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kinyoklion commented Nov 9, 2026 •

edited by cursor Bot

Loading

cursor Bot left a comment •

edited

Loading