-
Notifications
You must be signed in to change notification settings - Fork 11
feat: Lazy identity-flag evaluation in local-eval mode #200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7b1e3af
57208f3
5a7d7dc
3a9e0c4
783c683
7ce45cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,9 +3,37 @@ | |||||||||||||||||||
| import typing | ||||||||||||||||||||
| from dataclasses import dataclass, field | ||||||||||||||||||||
|
|
||||||||||||||||||||
| from flag_engine import engine | ||||||||||||||||||||
| from flag_engine.context.types import SegmentContext | ||||||||||||||||||||
|
|
||||||||||||||||||||
| from flagsmith.analytics import AnalyticsProcessor, PipelineAnalyticsProcessor | ||||||||||||||||||||
| from flagsmith.exceptions import FlagsmithFeatureDoesNotExistError | ||||||||||||||||||||
| from flagsmith.types import SDKEvaluationResult, SDKFlagResult | ||||||||||||||||||||
| from flagsmith.types import ( | ||||||||||||||||||||
| FeatureMetadata, | ||||||||||||||||||||
| SDKEvaluationContext, | ||||||||||||||||||||
| SDKEvaluationResult, | ||||||||||||||||||||
| SDKFlagResult, | ||||||||||||||||||||
| SegmentMetadata, | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| SegmentOverridesIndex = typing.Dict[ | ||||||||||||||||||||
| str, typing.List[SegmentContext[SegmentMetadata, FeatureMetadata]] | ||||||||||||||||||||
| ] | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| def build_segment_overrides_index( | ||||||||||||||||||||
| context: SDKEvaluationContext, | ||||||||||||||||||||
| ) -> SegmentOverridesIndex: | ||||||||||||||||||||
| """Map feature_name -> segments that carry an override for that feature. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Computed once per environment-document refresh so the lazy eval path | ||||||||||||||||||||
| can walk only the segments actually relevant to a given flag. | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| index: SegmentOverridesIndex = {} | ||||||||||||||||||||
| for segment_context in (context.get("segments") or {}).values(): | ||||||||||||||||||||
| for override in segment_context.get("overrides") or (): | ||||||||||||||||||||
| index.setdefault(override["name"], []).append(segment_context) | ||||||||||||||||||||
| return index | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| @dataclass | ||||||||||||||||||||
|
|
@@ -60,6 +88,14 @@ class Flags: | |||||||||||||||||||
| _pipeline_analytics_processor: typing.Optional[PipelineAnalyticsProcessor] = None | ||||||||||||||||||||
| _identity_identifier: typing.Optional[str] = None | ||||||||||||||||||||
| _traits: typing.Optional[typing.Dict[str, typing.Any]] = None | ||||||||||||||||||||
| # Lazy-evaluation state. When `_context` is set, `flags` is a | ||||||||||||||||||||
| # per-feature memo rather than a fully-materialised snapshot; unseen | ||||||||||||||||||||
| # features are resolved on demand via the engine primitives and | ||||||||||||||||||||
| # cached back into `flags`. Left as `None` by the eager code | ||||||||||||||||||||
| # paths (`from_evaluation_result` / `from_api_flags`). | ||||||||||||||||||||
| _context: typing.Optional[SDKEvaluationContext] = None | ||||||||||||||||||||
| _overrides_index: typing.Optional[SegmentOverridesIndex] = None | ||||||||||||||||||||
| _fully_materialised: bool = False | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @classmethod | ||||||||||||||||||||
| def from_evaluation_result( | ||||||||||||||||||||
|
|
@@ -86,6 +122,37 @@ def from_evaluation_result( | |||||||||||||||||||
| _traits=traits, | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @classmethod | ||||||||||||||||||||
| def from_evaluation_context( | ||||||||||||||||||||
| cls, | ||||||||||||||||||||
| context: SDKEvaluationContext, | ||||||||||||||||||||
| overrides_index: SegmentOverridesIndex, | ||||||||||||||||||||
| analytics_processor: typing.Optional[AnalyticsProcessor], | ||||||||||||||||||||
| default_flag_handler: typing.Optional[typing.Callable[[str], DefaultFlag]], | ||||||||||||||||||||
| pipeline_analytics_processor: typing.Optional[ | ||||||||||||||||||||
| PipelineAnalyticsProcessor | ||||||||||||||||||||
| ] = None, | ||||||||||||||||||||
| identity_identifier: typing.Optional[str] = None, | ||||||||||||||||||||
| traits: typing.Optional[typing.Dict[str, typing.Any]] = None, | ||||||||||||||||||||
| ) -> Flags: | ||||||||||||||||||||
| """Build a lazy `Flags` backed by an evaluation context. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| No engine work is done here — flags are resolved on first access | ||||||||||||||||||||
| via :meth:`_resolve_flag`. Reusing the same `overrides_index` | ||||||||||||||||||||
| across calls amortises its construction cost (it's rebuilt only | ||||||||||||||||||||
| when the environment doc refreshes, not per identity). | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| return cls( | ||||||||||||||||||||
| flags={}, | ||||||||||||||||||||
| default_flag_handler=default_flag_handler, | ||||||||||||||||||||
| _analytics_processor=analytics_processor, | ||||||||||||||||||||
| _pipeline_analytics_processor=pipeline_analytics_processor, | ||||||||||||||||||||
| _identity_identifier=identity_identifier, | ||||||||||||||||||||
| _traits=traits, | ||||||||||||||||||||
| _context=context, | ||||||||||||||||||||
| _overrides_index=overrides_index, | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @classmethod | ||||||||||||||||||||
| def from_api_flags( | ||||||||||||||||||||
| cls, | ||||||||||||||||||||
|
|
@@ -116,8 +183,21 @@ def all_flags(self) -> typing.List[Flag]: | |||||||||||||||||||
| """ | ||||||||||||||||||||
| Get a list of all Flag objects. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| In lazy mode, the caller has signalled they want every flag, so | ||||||||||||||||||||
| we run the bulk evaluator once on the full context and copy the | ||||||||||||||||||||
| results into the per-flag cache. Cheaper than asking the engine | ||||||||||||||||||||
| for each feature one at a time. | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
Comment on lines
+186
to
+190
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: This comment, and a few similar others, read a bit like deixis. This SDK exports no such thing as "lazy mode" — seemingly, it's the only mode now. |
||||||||||||||||||||
| :return: list of Flag objects. | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| if self._context is not None and not self._fully_materialised: | ||||||||||||||||||||
| result = engine.get_evaluation_result(self._context) | ||||||||||||||||||||
| for feature_name, flag_result in result["flags"].items(): | ||||||||||||||||||||
| if feature_name not in self.flags: | ||||||||||||||||||||
| self.flags[feature_name] = Flag.from_evaluation_result( | ||||||||||||||||||||
| flag_result, | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| self._fully_materialised = True | ||||||||||||||||||||
| return list(self.flags.values()) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| def is_feature_enabled(self, feature_name: str) -> bool: | ||||||||||||||||||||
|
|
@@ -151,11 +231,23 @@ def get_flag(self, feature_name: str) -> typing.Union[DefaultFlag, Flag]: | |||||||||||||||||||
| try: | ||||||||||||||||||||
| flag = self.flags[feature_name] | ||||||||||||||||||||
| except KeyError: | ||||||||||||||||||||
| if self.default_flag_handler: | ||||||||||||||||||||
| # Lazy path: if this `Flags` wraps an evaluation context and | ||||||||||||||||||||
| # the feature exists in it, resolve and memoise now. Otherwise | ||||||||||||||||||||
| # fall through to the default_flag_handler / not-found error, | ||||||||||||||||||||
| # preserving the eager-mode behaviour byte-for-byte. | ||||||||||||||||||||
| if ( | ||||||||||||||||||||
| self._context is not None | ||||||||||||||||||||
| and self._overrides_index is not None | ||||||||||||||||||||
| and feature_name in (self._context.get("features") or {}) | ||||||||||||||||||||
| ): | ||||||||||||||||||||
|
Comment on lines
+238
to
+242
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||
| flag = self._resolve_flag(feature_name) | ||||||||||||||||||||
| self.flags[feature_name] = flag | ||||||||||||||||||||
| elif self.default_flag_handler: | ||||||||||||||||||||
| return self.default_flag_handler(feature_name) | ||||||||||||||||||||
| raise FlagsmithFeatureDoesNotExistError( | ||||||||||||||||||||
| "Feature does not exist: %s" % feature_name | ||||||||||||||||||||
| ) | ||||||||||||||||||||
| else: | ||||||||||||||||||||
| raise FlagsmithFeatureDoesNotExistError( | ||||||||||||||||||||
| "Feature does not exist: %s" % feature_name | ||||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if self._analytics_processor and hasattr(flag, "feature_name"): | ||||||||||||||||||||
| self._analytics_processor.track_feature(flag.feature_name) | ||||||||||||||||||||
|
|
@@ -171,6 +263,35 @@ def get_flag(self, feature_name: str) -> typing.Union[DefaultFlag, Flag]: | |||||||||||||||||||
|
|
||||||||||||||||||||
| return flag | ||||||||||||||||||||
|
|
||||||||||||||||||||
| def _resolve_flag(self, feature_name: str) -> Flag: | ||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lazy path skips identity-key enrichment (multivariate + percentage_split break) flagsmith/models.py:265–311 — Flags._resolve_flag
flagsmith/flagsmith.py:431–456 — _get_identity_flags_from_document
flagsmith/mappers.py:88–99 — map_context_and_identity_data_to_context
Maybe we should consider bringing back the testing harness we had that used to cover all SDKs but was limited to just the engine during the last context value work — to avoid issues like this.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks — all this fixed by using
I'll have a think on this... there's a bit of a chicken & egg problem when it comes to remote evaluation. |
||||||||||||||||||||
| """Evaluate a single feature against the lazy context. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Goes through the engine's public `get_evaluation_result` so | ||||||||||||||||||||
| identity-key enrichment, multivariate hashing, percentage-split | ||||||||||||||||||||
| rules and override-priority handling all stay where they | ||||||||||||||||||||
| belong (in the engine). The performance win comes from passing | ||||||||||||||||||||
| a *trimmed* context — just the queried feature plus the segments | ||||||||||||||||||||
| that could override it, looked up in O(1) via the precomputed | ||||||||||||||||||||
| reverse index — so the engine's full pipeline runs against an | ||||||||||||||||||||
| input small enough to evaluate in ~1 µs. | ||||||||||||||||||||
| """ | ||||||||||||||||||||
| context = self._context | ||||||||||||||||||||
| overrides_index = self._overrides_index | ||||||||||||||||||||
| # `get_flag` / `all_flags` gate this call behind the same | ||||||||||||||||||||
| # non-None checks; assert here so type checkers can narrow. | ||||||||||||||||||||
| assert context is not None and overrides_index is not None | ||||||||||||||||||||
|
|
||||||||||||||||||||
| trimmed: SDKEvaluationContext = { | ||||||||||||||||||||
| **context, | ||||||||||||||||||||
| "features": {feature_name: context["features"][feature_name]}, | ||||||||||||||||||||
| "segments": { | ||||||||||||||||||||
| segment_context["key"]: segment_context | ||||||||||||||||||||
| for segment_context in overrides_index.get(feature_name, ()) | ||||||||||||||||||||
| }, | ||||||||||||||||||||
| } | ||||||||||||||||||||
| result = engine.get_evaluation_result(trimmed) | ||||||||||||||||||||
| return Flag.from_evaluation_result(result["flags"][feature_name]) | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| @dataclass | ||||||||||||||||||||
| class Segment: | ||||||||||||||||||||
|
|
||||||||||||||||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: possible, but unharmful, misuse os Python name mangling.