Skip to content

Commit

Permalink
Ensure percentage split evaluations are consistent in Core API and Lo…
Browse files Browse the repository at this point in the history
…cal Evaluation (#171)

* Replace `use_mv_v2_evaluation` with `use_identity_composite_key_for_hashing`

* Update pre-commit libs

* Add unit tests to verify that unknown fields are ignored

* Update get_hash_key method so it can be used from core

* Update git submodules

* Add return type hints
  • Loading branch information
matthewelwell authored Jun 28, 2023
1 parent d1ffed0 commit 7f5d318
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 21 deletions.
2 changes: 1 addition & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[submodule "tests/engine_tests/engine-test-data"]
path = tests/engine_tests/engine-test-data
url = git@github.com:Flagsmith/engine-test-data.git
url = https://github.com/flagsmith/engine-test-data.git
14 changes: 10 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,19 +1,25 @@
repos:
- repo: https://github.com/PyCQA/isort
rev: 5.9.3
rev: 5.12.0
hooks:
- id: isort
name: isort (python)
- repo: https://github.com/psf/black
rev: 22.6.0
rev: 23.3.0
hooks:
- id: black
language_version: python3
exclude: migrations
- repo: https://github.com/pycqa/flake8
rev: 3.9.2
rev: 6.0.0
hooks:
- id: flake8
name: flake8
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.4.0
hooks:
- id: check-yaml
- repo: https://github.com/pre-commit/mirrors-prettier
rev: v2.4.1
rev: v2.7.1
hooks:
- id: prettier
2 changes: 1 addition & 1 deletion flag_engine/environments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ class EnvironmentModel:
name: str = None
allow_client_traits: bool = True
updated_at: datetime = field(default_factory=utcnow_with_tz)
use_mv_v2_evaluation: bool = False
hide_sensitive_data: bool = False
use_identity_composite_key_for_hashing: bool = False

amplitude_config: typing.Optional[IntegrationModel] = None
dynatrace_config: typing.Optional[IntegrationModel] = None
Expand Down
2 changes: 1 addition & 1 deletion flag_engine/environments/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class BaseEnvironmentSchema(Schema):
name = fields.Str(required=False)
allow_client_traits = fields.Bool(required=False, default=True)
hide_disabled_flags = fields.Bool(required=False, allow_none=True)
use_mv_v2_evaluation = fields.Bool(required=False, default=False)
use_identity_composite_key_for_hashing = fields.Bool(required=False, default=False)
hide_sensitive_data = fields.Bool(required=False, default=False)

amplitude_config = fields.Nested(IntegrationSchema, required=False, allow_none=True)
Expand Down
8 changes: 6 additions & 2 deletions flag_engine/identities/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,12 @@ def composite_key(self) -> str:
def generate_composite_key(env_key: str, identifier: str) -> str:
return f"{env_key}_{identifier}"

def get_hash_key(self, use_mv_v2_evaluation: bool) -> str:
return self.composite_key if use_mv_v2_evaluation else self.identifier
def get_hash_key(self, use_identity_composite_key_for_hashing: bool) -> str:
return (
self.composite_key
if use_identity_composite_key_for_hashing
else self.identifier
)

def update_traits(
self, traits: typing.List[TraitModel]
Expand Down
60 changes: 60 additions & 0 deletions tests/unit/environments/test_environments_builders.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,3 +271,63 @@ def test_build_environment_api_key_model():

# Then
assert environment_key_model.key == environment_key_dict["key"]


def test_build_environment_model_with_deprecated_field() -> None:
# Given
environment_dict = {
"id": 1,
"api_key": "api-key",
"project": {
"id": 1,
"name": "test project",
"organisation": {
"id": 1,
"name": "Test Org",
"stop_serving_flags": False,
"persist_trait_data": True,
"feature_analytics": True,
},
"hide_disabled_flags": False,
},
"feature_states": [],
"use_mv_v2_evaluation": False,
}

# When
environment_model = build_environment_model(environment_dict)

# Then
assert environment_model
assert environment_model.use_identity_composite_key_for_hashing is False


def test_build_environment_model_with_unknown_field() -> None:
"""
Test to make sure that unknown fields are ignored when building an environment model.
"""
# Given
environment_dict = {
"id": 1,
"api_key": "api-key",
"project": {
"id": 1,
"name": "test project",
"organisation": {
"id": 1,
"name": "Test Org",
"stop_serving_flags": False,
"persist_trait_data": True,
"feature_analytics": True,
},
"hide_disabled_flags": False,
},
"feature_states": [],
"unknown_field": "foooobaaarrr",
}

# When
environment_model = build_environment_model(environment_dict)

# Then
assert environment_model
22 changes: 10 additions & 12 deletions tests/unit/identities/test_identities_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,17 +142,15 @@ def test_prune_features_only_keeps_valid_features(
assert identity.identity_features == [feature_state_1]


def test_get_hash_key_with_use_mv_v2_evaluation_enabled(identity):
# Given
use_mv_v2_evaluations = True

# When/ Then
assert identity.get_hash_key(use_mv_v2_evaluations) == identity.composite_key

def test_get_hash_key_with_use_identity_composite_key_for_hashing_enabled(identity):
assert (
identity.get_hash_key(use_identity_composite_key_for_hashing=True)
== identity.composite_key
)

def test_get_hash_key_with_use_mv_v2_evaluation_disabled(identity):
# Given
use_mv_v2_evaluations = False

# When/ Then
assert identity.get_hash_key(use_mv_v2_evaluations) == identity.identifier
def test_get_hash_key_with_use_identity_composite_key_for_hashing_disabled(identity):
assert (
identity.get_hash_key(use_identity_composite_key_for_hashing=False)
== identity.identifier
)

0 comments on commit 7f5d318

Please sign in to comment.