feat(SDK): Surface upcoming scheduled changes in the environment document#7977
feat(SDK): Surface upcoming scheduled changes in the environment document#7977cloorc wants to merge 1 commit into
Conversation
|
@cloorc is attempting to deploy a commit to the Flagsmith Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds opt-in scheduled-change data to environment-document feature states, including persisted future states and pending committed version changesets. The option propagates through engine, SDK, identity, segment, cache, database, and endpoint mappings. Scheduled data is omitted from serialised output unless present. The endpoint bypasses conditional caching for scheduled requests and documents the new query parameter. Regression tests cover versioning, malformed changesets, segment overrides, and earliest-candidate selection. Estimated code review effort: 4 (Complex) | ~60 minutes Comment |
github-actions
Bot
added
api
cloorc
force-pushed
the
feat/scheduled-changes-in-environment-document
branch
from
83d30c4 to
1e2fd62
Compare
cloorc
requested review from
adamvialpando and
emyller
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/environments/models.py (1)
585-609: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winClear the scheduled document cache on delete
api/environments/models.py:191-198should evict bothself.api_keyandf"{self.api_key}:scheduled".include_scheduled=Truestill goes throughenvironment_document_cachewhen document caching is enabled with a TTL, so a deleted environment can keep serving a stale scheduled document until expiry if that key was populated. The rename hook does not need the same change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ec28018c-a1a7-4a88-b16c-281e4fd4573f
📒 Files selected for processing (8)
api/environments/models.pyapi/environments/sdk/views.pyapi/tests/unit/util/mappers/test_scheduled_changes.pyapi/util/engine_models/features/models.pyapi/util/mappers/engine.pyapi/util/mappers/sdk.pydocs/docs/sdk-api/sdk-v-1-environment-document.api.mdxsdk/openapi.yaml
cloorc
force-pushed
the
feat/scheduled-changes-in-environment-document
branch
from
1e2fd62 to
6264a81
Compare
|
@coderabbitai Addressed both actionable findings in
Left the Full local test suite still green; added/updated tests pass ( |
|
Tip For best results, initiate chat on the files or code changes.
Your rationale for leaving the |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/environments/models.py (1)
191-198: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winEvict the scheduled cache entry on delete.
delete_environment_document_from_cache()only removesself.api_key, so a cachedf"{self.api_key}:scheduled"response can survive until TTL expiry and return stale data after an environment is deleted. Delete both keys here, or usedelete_many(...).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d82e1923-c2b4-4729-a5f3-d7aa005c9d8f
📒 Files selected for processing (8)
api/environments/models.pyapi/environments/sdk/views.pyapi/tests/unit/util/mappers/test_scheduled_changes.pyapi/util/engine_models/features/models.pyapi/util/mappers/engine.pyapi/util/mappers/sdk.pydocs/docs/sdk-api/sdk-v-1-environment-document.api.mdxsdk/openapi.yaml
cloorc
force-pushed
the
feat/scheduled-changes-in-environment-document
branch
from
6264a81 to
685d16c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 503cc131-eff3-4468-aa86-3600694b8423
📒 Files selected for processing (9)
api/environments/models.pyapi/environments/sdk/views.pyapi/tests/unit/environments/test_unit_environments_models.pyapi/tests/unit/util/mappers/test_scheduled_changes.pyapi/util/engine_models/features/models.pyapi/util/mappers/engine.pyapi/util/mappers/sdk.pydocs/docs/sdk-api/sdk-v-1-environment-document.api.mdxsdk/openapi.yaml
cloorc
force-pushed
the
feat/scheduled-changes-in-environment-document
branch
from
685d16c to
f4a76ff
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
api/util/mappers/engine.py (1)
880-925: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor duplication of "keep earliest" logic.
The earliest-wins comparison here (lines 904-912) duplicates
_keep_earliest_scheduled_change(lines 312-319), just inlined forFeatureStateinstead ofScheduledChangeModel. Could be unified with a small generic helper parameterised by alive_fromextractor, but not essential given the different types involved.api/environments/models.py (1)
533-555: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a direct test for the cache-bypass branch.
The mapper-level tests cover
map_environment_to_sdk_document, but nothing here exercisesEnvironment.get_environment_document(include_scheduled=True)to confirm it never touchesenvironment_document_cache.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9aa592d3-b7e6-45c4-88b0-c744bbe75f15
📒 Files selected for processing (8)
api/environments/models.pyapi/environments/sdk/views.pyapi/tests/unit/util/mappers/test_scheduled_changes.pyapi/util/engine_models/features/models.pyapi/util/mappers/engine.pyapi/util/mappers/sdk.pydocs/docs/sdk-api/sdk-v-1-environment-document.api.mdxsdk/openapi.yaml
cloorc
force-pushed
the
feat/scheduled-changes-in-environment-document
branch
from
f4a76ff to
f34129b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 286d9d8e-b5b1-4dc2-b79b-2267580fdcf7
📒 Files selected for processing (9)
api/environments/models.pyapi/environments/sdk/views.pyapi/tests/unit/environments/test_unit_environments_views_sdk_environment.pyapi/tests/unit/util/mappers/test_scheduled_changes.pyapi/util/engine_models/features/models.pyapi/util/mappers/engine.pyapi/util/mappers/sdk.pydocs/docs/sdk-api/sdk-v-1-environment-document.api.mdxsdk/openapi.yaml
…ment
Add an opt-in `include_scheduled` query param to the SDK environment-document
endpoint. When set, each feature state additionally carries a
`scheduled_change` field (`live_from`/`enabled`/`feature_state_value`)
describing its next not-yet-live version, if any — sourced from both a future
FeatureState/EnvironmentFeatureVersion already in the DB and a committed but
not-yet-materialised VersionChangeSet diff, with an earliest-wins rule when
both exist.
This lets SDKs running in local-evaluation mode resolve a scheduled change's
maturity from an already-cached document, without waiting for a new poll
after `live_from` passes — and keeps a fleet of instances in sync at the same
moment, since every instance resolves maturity from its own clock rather than
from when it happened to last poll.
Fully backward compatible: the field is omitted entirely when not requested,
so existing SDKs and cached documents are byte-for-byte unchanged.
Test names follow the repo's test_{subject}__{condition}__{expected}
convention with Given/When/Then comments, as flat functions per the
flagsmith-lint-tests pre-commit hook. The SDK API reference page for this
endpoint is regenerated from the updated OpenAPI spec.
cloorc
force-pushed
the
feat/scheduled-changes-in-environment-document
branch
from
f34129b to
08da695
Compare
1 participant
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to #7973
SDKs running in local-evaluation mode only see a scheduled (future
live_from) change on their next poll afterlive_frompasses, which can lag by a full refresh interval — and there's currently no way for a client to know a change is coming ahead of time. This adds an opt-in way for the SDK environment-document endpoint to advertise that, so a local-evaluation SDK can resolve maturity itself from an already-cached document, without waiting for a new poll.include_scheduledquery param to the SDK environment-document endpoint (GET /api/v1/environment-document/).scheduled_changefield (live_from/enabled/feature_state_value) describing its next not-yet-live version, sourced from either a futureFeatureState/EnvironmentFeatureVersionalready in the DB, or a committed-but-not-yet-materialisedVersionChangeSetdiff — whichever goes live first when both exist.environment.updated_at— trades away that optimization for guaranteed freshness on the opted-in path only.I'm holding off on opening companion PRs to
flagsmith-java-clientand the OpenFeaturejava-sdk-contribprovider (both already prototyped) until the API shape here is reviewed, in case it changes.Review effort: 3/5
How did you test this code?
Added
api/tests/unit/util/mappers/test_scheduled_changes.py(15 tests) covering: both scheduling mechanisms independently, the earliest-wins tie-break when both apply to the same feature, unpublished/uncommitted drafts correctly not surfacing, past-live_fromchanges correctly not surfacing, and malformed/partial changeset JSON degrading to "skip and log" rather than raising.Ran the full local test suite clean:
4243 passed, 22 skipped, plusruffandmypy --strictclean on every changed/added file.