Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📝 WalkthroughWalkthroughThe API schemas add typed experiment metadata and optional metadata fields to Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🔵 Low · up to Invalid experiment start timestamps can be returned in API metadata. Add UTC-preserving validation before merging. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #272 +/- ##
==========================================
+ Coverage 97.61% 97.64% +0.03%
==========================================
Files 109 110 +1
Lines 4859 4921 +62
==========================================
+ Hits 4743 4805 +62
Misses 116 116 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 2bb7e9b7-4d90-41be-b59f-b330aa30bc4e
📒 Files selected for processing (4)
src/flagsmith_schemas/api.pysrc/flagsmith_schemas/dynamodb.pytests/integration/flagsmith_schemas/test_api.pytests/integration/flagsmith_schemas/test_dynamodb.py
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
| """Unique identifier for the experiment in Core.""" | ||
| name: str | ||
| """Name of the experiment.""" | ||
| started_at: str |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Validate started_at as UTC without changing the wire value.
ExperimentMetadata.started_at is declared as str, so TypeAdapter(FeatureState).validate_python(...) accepts "not-a-timestamp" despite the documented ISO 8601 UTC contract.
DateTimeStr is not a drop-in fix. Its ValidateAs(datetime, lambda dt: dt.isoformat()) validator accepts naive or non-UTC datetimes and normalises "2026-07-30T00:00:00Z" to a +00:00 form. This conflicts with test_feature_state__metadata_experiment__expected_result, which requires the validated payload to equal the input.
Add an API-specific validated string type that requires a UTC timestamp and preserves the original serialised string.
8d5056b to
271fe0b
Compare
|
@themis-blindfold review |
⚖️ Themis review: 🧹 Ship it, nits insideTL;DR: The new envelope is carried through the public SDK and DynamoDB schemas, including compressed environment documents. CI passed for lint-test on Python 3.11 and 3.12, CodeRabbit, and pre-commit; one schema-documentation nit remains open.
🧹 Nits
📝 Walkthrough
🧪 How to verify
Product take: A small but useful compatibility addition: experiment context can now reach SDK consumers without being stripped. Its value is concentrated in experiment-aware clients. 🧭 Assumptions & unverified claimsNo unverified assumptions or claims. Metadata made it through the gzip tunnel intact · reviewed at 271fe0b |
271fe0b to
2b8e57e
Compare
2b8e57e to
4af9b4e
Compare
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to Flagsmith/edge-api#719
Core is adding an optional
metadatadict to feature states.Its first key is
experiment { id, name, started_at, in_experiment }, present only while the feature's experiment is running.This PR adds the envelope to the schemas so nothing strips it on the way through.
api.pyExperimentMetadataandFeatureStateMetadataTypedDicts.metadata: NotRequired[FeatureStateMetadata]onFeatureState(environment document) andV1Flag(/identities).FeatureStateMetadatakeeps unknown keys (PEP 728extra_items, same pattern asJsonRecord), so future keys land without a schema change.dynamodb.pyFeatureState.metadata: NotRequired[dict[str, Any]], opaque.How did you test this code?
9 new tests:
experimentmetadatapreservedexperiment.idtype still rejecteduv run --all-extras pytest: 406 passed.pre-commit run --all-filesandmypy src tests: green.