Skip to content

fix: paginate environment-document for local evaluation#204

Merged
emyller merged 4 commits intoFlagsmith:mainfrom
higordasneves:fix/identity-override-pagination
Apr 29, 2026
Merged

fix: paginate environment-document for local evaluation#204
emyller merged 4 commits intoFlagsmith:mainfrom
higordasneves:fix/identity-override-pagination

Conversation

@higordasneves
Copy link
Copy Markdown
Contributor

@higordasneves higordasneves commented Apr 28, 2026

Closes #205


Summary

Adds pagination support for the local-evaluation environment document, addressing Flagsmith/flagsmith#4664 on the
Go SDK side. Before this PR the Go client only consumed the first page, silently dropping identity overrides that fall on
subsequent pages.

What changed

  • UpdateEnvironment now follows the Link header, accumulating IdentityOverrides across pages, then maps the merged EnvironmentModel into the
    engine evaluation context exactly once at the end of the refresh cycle.
  • New helpers: ExtractNextPage (parses the page_id query param out of the Link header) and checkEnvironmentMemoryAlloc (per-refresh memory
    cap).
  • Two new client options:
    • WithLocalEvaluationPageLimit(int) — caps pages per refresh. Default is 1, which preserves pre-PR behaviour for users that haven't opted
      in. 0 means unlimited.
    • WithLocalEvaluationMemoryAllocLimit(int) — caps the accumulated identity_overrides JSON size (in bytes) across pages. 0 means no limit.
  • Fixtures: OverriddenIdentifierPage2, PageID, PageIDEncoded, EnvironmentJsonPage2, and a PaginatedEnvironmentDocumentHandler that serves a
    Link-paginated two-page response.

Behaviour notes

  • Backwards compatible by default. localEvaluationPageLimit defaults to 1, so existing users see the exact same single-page fetch they get
    today. They opt in to pagination by setting the limit to 0 (unlimited) or a value > 1.
  • First page is authoritative for everything except identity_overrides. Subsequent pages contribute only their IdentityOverrides;
    feature_states, project, segments, etc. are taken from page 1. This matches the Edge API's pagination contract.
  • Memory limit is a soft stop. When appending the next page would exceed the byte limit, that page (and any pages after it) are dropped and a
    warning is logged; the refresh otherwise succeeds with what was loaded.

Manual tests

Backward compatibility test

image

Pagination test

image

@higordasneves higordasneves marked this pull request as ready for review April 28, 2026 01:15
@higordasneves higordasneves requested a review from a team as a code owner April 28, 2026 01:15
@higordasneves higordasneves requested review from Zaimwa9 and removed request for a team April 28, 2026 01:15
@higordasneves higordasneves force-pushed the fix/identity-override-pagination branch from cada705 to fea5882 Compare April 28, 2026 01:21
Copy link
Copy Markdown
Contributor

@emyller emyller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! This will help us immensely in moving other projects too.

I've left one blocking comment, and another potential one.

Comment thread config.go Outdated
Comment thread config.go Outdated
@emyller
Copy link
Copy Markdown
Contributor

emyller commented Apr 28, 2026

I've opened #205, including updated acceptance criteria, and updated the description of this PR pointing to it. If you decide to meet all A/C — you're not required to! —, please update the comment to "closes".

Follow Link: rel="next" until exhaustion instead of gating pagination
behind opt-in limits, so local evaluation matches remote evaluation on
large environments by default. When a refresh takes longer than the
configured interval, log a warning naming both durations so operators
know to raise WithEnvironmentRefreshInterval or trim the environment.
@higordasneves
Copy link
Copy Markdown
Contributor Author

I've opened #205, including updated acceptance criteria, and updated the description of this PR pointing to it. If you decide to meet all A/C — you're not required to! —, please update the comment to "closes".

@emyller Pushed an update addressing the acceptance criteria from #205. Pagination now always follows Link: rel="next" to the end, the two opt-in limits are gone, and a warning is logged when a refresh exceeds WithEnvironmentRefreshInterval. I've also replied inline on both threads with the reasoning behind the original conservative approach for context.

emyller
emyller previously approved these changes Apr 28, 2026
Copy link
Copy Markdown
Contributor

@emyller emyller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, thank you! Adding to our release pipeline soon.

Comment thread client.go
@higordasneves
Copy link
Copy Markdown
Contributor Author

@emyller ops, linter issue solved haha

@emyller
Copy link
Copy Markdown
Contributor

emyller commented Apr 28, 2026

Sorry, it looks like we need to handle tooling updates around linting/CI. Allow us a bit to get that sorted.

@emyller
Copy link
Copy Markdown
Contributor

emyller commented Apr 29, 2026

@higordasneves Can you please update this branch with latest main? The CI error above should be gone.

@higordasneves
Copy link
Copy Markdown
Contributor Author

@emyller done

@emyller emyller merged commit cf40483 into Flagsmith:main Apr 29, 2026
3 checks passed
@emyller
Copy link
Copy Markdown
Contributor

emyller commented Apr 29, 2026

FYI this has been release as part of v5.1.0. Again, thanks so much for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Local evaluation silently drops identity overrides on large environments

2 participants