Repository navigation
fix: Accept a path object as an override file path - #572
Merged
kinyoklion merged 2 commits intoNov 10, 2026
Merged
Conversation
Member
Author
|
bugbot review |
Member
Author
|
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 a1bda43. Configure here.
keelerm84
approved these changes
Nov 10, 2026
kinyoklion
force-pushed
the
rlamb/overrides-python-contract-tests
branch
from
November 10, 2026 22:24
7e42fc2 to
5d19e57
Compare
kinyoklion
force-pushed
the
rlamb/overrides-python-file-source-paths
branch
from
November 10, 2026 22:24
a1bda43 to
5981654
Compare
Base automatically changed from
rlamb/overrides-python-contract-tests
to
feat/overrides
November 10, 2026 23:27
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.
kinyoklion
force-pushed
the
rlamb/overrides-python-file-source-paths
branch
from
November 10, 2026 23:29
5981654 to
154b5b8
Compare
Open
3 of 4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requirements
async_*siblings (or N/A). N/A:LDClientandAsyncLDClientuse the sameFileOverrideSourceBuilder.Related issues
Describe the solution you've provided
FileOverrideSourceBuilderaccepted one path as astr, or a list of paths. A singlepathlib.Path, or any otheros.PathLike, was passed tolist(). A path object is not iterable, so the constructor raisedTypeError: '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
stror anos.PathLikeas a single path, and converts every path withos.fspath. A path that does not name a string path raisesTypeErrorfrom the constructor, with a message that names the value. That includesbytes(given alone, in a list, or named by a path object), anint, andNone.bytesandbytearraycount as single values, so a barebytespath is named whole instead of being read as a sequence of integers. The type hint isSequencerather thanList, 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:
os.DirEntry;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 ofos.fspath.Describe alternatives you've considered
Leaving the builder to accept only strings, as its type hint said.
pathlibis 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:
Testing:
mypy ldclient,isort --checkandpycodestylepass 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 onmainorfeat/**, 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 otheros.PathLike) was passed tolist(), causingTypeError: 'PosixPath' object is not iterable.FileOverrideSourceBuildernow wraps a lonestr,bytes/bytearray, orPathLikeas one path, normalizes every entry withos.fspath()(so objects likeos.DirEntryuse their filesystem path, notstr()), and stores string paths. Invalid inputs (bytespaths, ints,None, path objects that do not resolve to astr) fail fast with an explicitTypeError. Type hints useSequenceplusPathLikeso callers can pass path objects while unordered collections likesetare 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.