feat: Add a task for writing (edge) identity overrides #3127
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
khvn26
changed the title
|
Uffizzi Preview |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #3127 +/- ##
==========================================
+ Coverage 95.90% 95.95% +0.04%
==========================================
Files 1048 1053 +5
Lines 31384 31588 +204
==========================================
+ Hits 30100 30311 +211
+ Misses 1284 1277 -7 ☔ View full report in Codecov by Sentry. |
khvn26
changed the title
khvn26
requested review from
a team,
zachaysan,
matthewelwell and
gagantrivedi
and removed request for
a team
api/conftest.py
Outdated
| @pytest.fixture() | ||
| def aws_credentials(): | ||
| """Mocked AWS Credentials for moto.""" | ||
| os.environ["AWS_ACCESS_KEY_ID"] = "testing" | ||
| os.environ["AWS_SECRET_ACCESS_KEY"] = "testing" | ||
| os.environ["AWS_SECURITY_TOKEN"] = "testing" | ||
| os.environ["AWS_SESSION_TOKEN"] = "testing" | ||
| os.environ["AWS_DEFAULT_REGION"] = "eu-west-2" | ||
|
|
||
|
|
||
| @pytest.fixture() | ||
| def dynamodb(aws_credentials): | ||
| # TODO: move all wrapper tests to using moto | ||
| with mock_dynamodb(): | ||
| yield boto3.resource("dynamodb") | ||
|
|
||
|
|
||
| @pytest.fixture() | ||
| def flagsmith_environments_v2_table(dynamodb) -> Table: | ||
| return dynamodb.create_table( | ||
| TableName="flagsmith_environments_v2", | ||
| KeySchema=[ | ||
| { | ||
| "AttributeName": "environment_id", | ||
| "KeyType": "HASH", | ||
| }, | ||
| { | ||
| "AttributeName": "document_key", | ||
| "KeyType": "RANGE", | ||
| }, | ||
| ], | ||
| AttributeDefinitions=[ | ||
| {"AttributeName": "environment_id", "AttributeType": "N"}, | ||
| {"AttributeName": "document_key", "AttributeType": "S"}, | ||
| ], | ||
| BillingMode="PAY_PER_REQUEST", | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def dynamodb_wrapper_v2( | ||
| settings: SettingsWrapper, | ||
| flagsmith_environments_v2_table: Table, | ||
| ) -> DynamoEnvironmentV2Wrapper: | ||
| settings.ENVIRONMENTS_V2_TABLE_NAME_DYNAMO = flagsmith_environments_v2_table.name | ||
| return DynamoEnvironmentV2Wrapper() |
There was a problem hiding this comment.
@matthewelwell I've relocated these from your endpoint PR so they can be used by some other tests too.
| key_expression_condition = Key(self.ENVIRONMENT_ID_ATTRIBUTE).eq( | ||
| environment_id | ||
| ) & Key(self.DOCUMENT_KEY_ATTRIBUTE).begins_with(document_key_begins_with) | ||
| def get_table_name(self): |
There was a problem hiding this comment.
@matthewelwell Various improvements here that will introduce conflicts in your PR:
- Constants live in their own module now.
get_identity_overridesis nowget_identity_overrides_by_feature_idto avoid dead code paths — to my best knowledge, only Edge API will need to fetch overrides by environment id alone.
| def map_environment_to_environment_v2_document( | ||
| environment: "Environment", | ||
| ) -> Document: | ||
| return { | ||
| **map_environment_to_environment_document(environment), | ||
| "document_key": "META", | ||
| "environment_id": str(environment.id), | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
@matthewelwell This part is also from your PR, with slight changes.
khvn26
force-pushed
the
feat/environments-v2-task
branch
from
9bb5473 to
5a7a551
Compare
| ) -> dict[str, typing.Any]: | ||
| return { | ||
| **feature_state.dict(), | ||
| "feature_state_value": feature_state.get_value(identity_id), |
There was a problem hiding this comment.
Just making a note here as well that this is different to the previous behaviour - this used to be "value" I think?
There was a problem hiding this comment.
From what I can tell, this data is used in the generate_audit_log_records task (as well as the new task you created) but the value is not actually used so I think this is fine.
There was a problem hiding this comment.
Yes, "value" was not used. I made sure to modify the generate_audit_log_records tests accordingly.
| class IdentityOverrideV2(BaseModel): | ||
| environment_id: str | ||
| document_key: str | ||
| environment_api_key: str | ||
| feature_state: FeatureStateModel |
There was a problem hiding this comment.
I might want to integrate this with my code too.
There was a problem hiding this comment.
I've done this, but I also had to add a new attribute: identifier: str to it as well. More detail in a comment on my PR here.
Closes #3106.
pre-committo check lintingdocs/if required so people know about the feature!Changes
This PR adds the following:
update_flagsmith_environments_v2_identity_overridestask responsible for parsing changes generated inEdgeIdentity.save()method and reflecting them in DynamoDBenvironments_v2_table.EdgeIdentity.save()method, in case identity override changes are detected, along with the audit log entries generation task.EdgeIdentity.save()method is now invoked during the identity override trimming routine by thesync_identity_document_featurestask whenever features are deleted.environments_v2_table's newly string-typedenvironment_idcolumn.Various touch-ups and improvements:
datetimearguments.DynamoEnvironmentV2Wrapperslightly reworked (see comment)How did you test this code?
The code is covered by unit tests.
I've also tested this by pointing local Core API instance at
environments_v2_tablein staging.