monitor config model updates #4888
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Passing run #7742 ↗︎
Details:
Review all test suite changes for PR #4888 ↗︎ |
||||||||||||||||||||
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4888 +/- ##
=======================================
Coverage 86.76% 86.76%
=======================================
Files 347 347
Lines 20927 20967 +40
Branches 2736 2740 +4
=======================================
+ Hits 18157 18192 +35
- Misses 2294 2297 +3
- Partials 476 478 +2 ☔ View full report in Codecov by Sentry. |
adamsachs
changed the title
| unique=False, | ||
| nullable=True, |
There was a problem hiding this comment.
This doesn't matter, but we don't really have to specify this nullable=True default or unique=False default - in some models we were only specifying non-default attributes. Existing classify_parms already has this though
There was a problem hiding this comment.
yeah i always go a bit back and forth - to your point, maybe i'll lean more toward not specifying, sometimes specifying can be confusing in its own right! i guess i just usually forget whether nullable is True or False by default 😬
(hence the back and forth 😆)
| @@ -65,28 +65,15 @@ def mask_sensitive_fields( | |||
| if connection_secrets is None: | |||
| return connection_secrets | |||
There was a problem hiding this comment.
Ah the return type of this is Optional then
| @@ -12,6 +12,8 @@ | |||
| class ConnectionConfigSecretsSchema(BaseModel, abc.ABC): | |||
| """Abstract Base Schema for updating Connection Configuration Secrets""" | |||
|
|
|||
| # NOTE: any fields not listed in `_required_components` must also have an | |||
| # annotated Optional type, in order to be treated effectively as optional fields | |||
| _required_components: List[str] | |||
There was a problem hiding this comment.
ah these _required_components is confusing at this point, thanks for improving with the docstring, This was the original intent: https://github.com/ethyca/solon/blame/085ace7d88ceee0e1de3a4b2105ce68e54e5e431/src/fidesops/schemas/connection_configuration/connection_secrets.py#L28
| For example, an `execution_start_date` of "2024-05-14 12:00:00+00:00": | ||
| - with an `execution_frequency` of "daily", it will result in daily | ||
| execution at 12:00:00+00:00; | ||
| - with an `execution_frequency` of "weekly", it will result in weekly | ||
| execution at 12:00:00+00:00 on every Tuesday, since 2024-05-14 is | ||
| a Tuesday. | ||
| - with an `execution_frequency` of "monthly", it will result in monthly | ||
| execution at 12:00:00+00:00 on the 14th day of every month. |
Along with associated plus PR, closes PROD-2075 PROD-2043
Description Of Changes
Model-level updates to support the following monitor configurations:
Also some general cleanup of detection/discovery related functionality along the way.
Code Changes
monitor_execution_triggerJSON column to store serializedcrontrigger params for APSchedulercreateandupdateoverrides) to help with translating themonitor_execution_triggerparam JSON to and from API schema fields related to monitor schedulingdatabasesstring array column to store the monitor config's scoped databasesconnection_config_keyproperty function to the model to support automatic serialization to the API schemaldatasetfield on the bigquery connection config secrets is properly marked as optional so as to make it not requiredSteps to Confirm
Pre-Merge Checklist
CHANGELOG.md