feat: Add DynamoDB document schemas #137

khvn26 · 2025-12-11T11:44:05Z

Contributes to #128.

In this PR, we add Python stdlib-based DynamoDB document schemas for Flagsmith's Edge API data model.

The goals are to:

Preserve a single source of truth for Edge API's data model.
Thoroughly document it, and uphold high documentation standards in the future.
Decouple the schema from its validation and serialisation code so the data can be consumed by typed Python code without having to rely on Pydantic dependency.
At the same time, make it usable for serialisation tools such as Pydantic and preserve compatibility with existing validation and serialisation behaviour.

Initially, I was hesitant to add validation code to this package. I'm considering to add it now, however, in a subsequent PR, so it can be consumed in environments where Pydantic is still possible to use.

I've added a bunch of TODOs that I expect to resolve prior to merging the current PR, and highlighted them by starting relevant review threads.

codecov-commenter · 2025-12-11T11:45:25Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.17%. Comparing base (51c529b) to head (c743785).

Additional details and impacted files

@@                Coverage Diff                 @@
##           ci/uv-renovate     #137      +/-   ##
==================================================
+ Coverage           94.72%   95.17%   +0.44%     
==================================================
  Files                  76       79       +3     
  Lines                2444     2671     +227     
==================================================
+ Hits                 2315     2542     +227     
  Misses                129      129

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

khvn26 · 2025-12-15T18:46:53Z

src/flagsmith_schemas/types.py

These are placeholder types now. They might be annotated with optional validation code in the future.

khvn26 · 2025-12-15T18:48:16Z

src/flagsmith_schemas/dynamodb.py

+    """Represents a multivariate feature state value assigned to an identity or environment."""
+
+    id: NotRequired[int | None]
+    """Unique identifier for the multivariate feature state value in Core. TODO: document why and when this can be `None`."""


It seems Core API currently stores both id and mv_fs_value_uuid. I'd like the field docstrings to explain why, but I need input from the team to do it properly.

CC @gagantrivedi @matthewelwell

khvn26 · 2025-12-15T18:51:40Z

src/flagsmith_schemas/dynamodb.py

+    django_id: NotRequired[int | None]
+    """Unique identifier for the feature state in Core. TODO: document why and when this can be `None`."""


Similarly to https://github.com/Flagsmith/flagsmith-common/pull/137/changes#r2620470807, we currently store both django_id and featurestate_uuid, and I'd like to document the order precedence and historical decisions here, if possible.

src/flagsmith_models/__init__.py

khvn26 · 2025-12-15T18:58:20Z

src/flagsmith_schemas/dynamodb.py

+    django_id: NotRequired[int | None]
+    """Unique identifier for the identity in Core. TODO: document why and when this can be `None`."""


Apparently, this is None when identities are created by Edge. There's really no workaround for this so far; this is related to Flagsmith/flagsmith#2186.

Let me know if I'm missing anything, @matthewelwell @gagantrivedi.

I believe this answers your other two comments as well. If you're creating an identity override (for edge identity), the feature state ID will be None, and if that feature state is multivariate, the ID of the multivariate feature state value will also be None.

Isn't this a bug then? I.e. I don't see why Core shouldn't have access to numeric IDs for feature states and multivariate feature states when an identity override gets created?

Is there an identity override creation path via Edge API I'm not familiar with?

Not a bug - this is by design. When identity overrides are created via the Edge API, they never go through Django models/Postgres - they're only stored in DynamoDB. So they don't get a Django ORM primary key.

gagantrivedi · 2025-12-16T07:14:45Z

src/flagsmith_schemas/dynamodb.py

@@ -0,0 +1,363 @@
+"""
+Types describing Flagsmith Edge API's data model.
+The schemas are written to DynamoDB documents by Core, and read by Edge.


This comment is very confusing: The comment says "schemas are written to DynamoDB" - but schemas describe the shape of data, they aren't the data itself

Addressed in 0179e00.

gagantrivedi · 2025-12-16T07:43:14Z

src/flagsmith_schemas/dynamodb.py

+    django_id: NotRequired[int | None]
+    """Unique identifier for the identity in Core. TODO: document why and when this can be `None`."""


I believe this answers your other two comments as well. If you're creating an identity override (for edge identity), the feature state ID will be None, and if that feature state is multivariate, the ID of the multivariate feature state value will also be None.

gagantrivedi · 2025-12-16T07:57:02Z

src/flagsmith_schemas/dynamodb.py

@@ -0,0 +1,363 @@
+"""


The comment density here is quite high. Many of these docstrings just restate what the field name already tells us (name: str with "Name of the feature", enabled: bool with "Whether the feature is enabled"). Could we trim these down to only the non-obvious stuff - constraints, edge cases, gotchas?

I disagree. I'd like to keep the docstrings because even obvious field descriptions help to remove ambiguity, and having all fields documented help to support the discipline to document new fields going forward.

If anything, I'd add a linter that makes sure every field has a docstring in this module.

I'm gonna have to disagree here. If a field name needs a docstring to explain what it is, that's a sign the field name itself isn't good enough. The name and type should tell the story. Comments should explain why, not what.

The maintenance burden is real too - every field change now requires updating two places. Comments that just mirror the code tend to drift out of sync and become misleading over time.

Comments should explain why, not what.

Sometimes the why and the what are the same, and sometimes they're different. The users of the code shouldn't have to guess. This is what I meant by removing ambiguity.

Comments that just mirror the code tend to drift out of sync and become misleading over time.

It's hard to imagine this scenario for me, given that document schemas are supposed to be forward compatible. Once the field is added, it stays. If its purpose changes or it gets deprecated, we only have to edit the docstring. Ideally, I don't see us changing the field names at all.

Let’s open the floor for a wider discussion.: @matthewelwell @Zaimwa9

Fair points. I still think there's a distinction worth making though - if the what isn't clear from the field name, that's a naming problem. But I take your point about forward compatibility reducing the drift risk.

Maybe a middle ground: keep docstrings for fields where there's genuine context to add (constraints, defaults, deprecation notes, edge cases), but skip the ones that are purely restating the name? That way
we're not just adding noise for the sake of consistency.

I do tend to agree with Gagan here.

As an example, this comment seems unnecessary to me, as I think Gagan already mentioned.

I'm happy for us to lean on the overly verbose side here, but I think this example is probably a little much.

gagantrivedi · 2025-12-16T07:58:34Z

src/flagsmith_schemas/dynamodb.py

@@ -0,0 +1,363 @@
+"""


The name of this module is pretty generic - doesn't tell us what these models are for. Something like edge_document.. or dynamodb_schemas.. would make the purpose clearer

Great suggestion! Renamed to flagsmith_schemas.dynamodb in ac9e8df.

feat: Add DynamoDB document schemas

8b18b1a

khvn26 added 3 commits December 11, 2025 11:56

add a question

815f0d3

fix 3.11 support

7c45ffb

add all root documents + tests

9d21d93

khvn26 commented Dec 15, 2025

View reviewed changes

khvn26 marked this pull request as ready for review December 15, 2025 18:58

khvn26 requested a review from a team as a code owner December 15, 2025 18:58

khvn26 requested review from emyller, gagantrivedi and matthewelwell and removed request for a team December 15, 2025 18:58

khvn26 added 2 commits December 15, 2025 19:07

add identity override test

a29f199

document environment name default

4d8c83a

gagantrivedi requested changes Dec 16, 2025

View reviewed changes

khvn26 added 3 commits December 16, 2025 11:55

rename to flagsmith_schemas.dynamodb

ac9e8df

Improve docstring

0179e00

consolidate scalar types in the types module

c743785

		django_id: NotRequired[int \| None]
		"""Unique identifier for the feature state in Core. TODO: document why and when this can be `None`."""

		django_id: NotRequired[int \| None]
		"""Unique identifier for the identity in Core. TODO: document why and when this can be `None`."""

feat: Add DynamoDB document schemas #137

Are you sure you want to change the base?

feat: Add DynamoDB document schemas #137

Uh oh!

Conversation

khvn26 commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

khvn26 Dec 16, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

khvn26 Dec 16, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

khvn26 Dec 16, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

khvn26 commented Dec 11, 2025 •

edited

Loading

codecov-commenter commented Dec 11, 2025 •

edited

Loading

khvn26 Dec 16, 2025 •

edited

Loading

khvn26 Dec 16, 2025 •

edited

Loading

khvn26 Dec 16, 2025 •

edited

Loading