-
Notifications
You must be signed in to change notification settings - Fork 72
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
PROD-2147 Adds last_monitored and enabled attributes to MonitorConfig #4991
PROD-2147 Adds last_monitored and enabled attributes to MonitorConfig #4991
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
9af6308
to
4ff7c08
Compare
Passing run #8468 ↗︎
Details:
Review all test suite changes for PR #4991 ↗︎ |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4991 +/- ##
=======================================
Coverage 86.56% 86.56%
=======================================
Files 352 352
Lines 21874 21877 +3
Branches 2884 2884
=======================================
+ Hits 18935 18938 +3
Misses 2433 2433
Partials 506 506 ☔ View full report in Codecov by Sentry. |
4ff7c08
to
ebba34b
Compare
...alembic/migrations/versions/4fb779cc5f17_add_last_monitored_and_enabled_to_monitor_config.py
Outdated
Show resolved
Hide resolved
a752491
to
e997c2d
Compare
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.
this side of things looks good to me! one minor nit on the changelog and i do think the migration down rev needs to be bumped, but these model-level updates make sense and seem pretty straightforward! nice work on a clean update.
...alembic/migrations/versions/4fb779cc5f17_add_last_monitored_and_enabled_to_monitor_config.py
Outdated
Show resolved
Hide resolved
d1bb182
to
07452cf
Compare
Passing run #8470 ↗︎
Details:
Review all test suite changes for PR #4991 ↗︎ |
Closes PROD-2147
Description Of Changes
Two new fields have been added to the
MonitorConfig
model:last_monitored
, a nullable datetime field to store the datetime when the monitor was last executedenabled
, a boolean non-nullable field to toggle the monitor on / off. Defaults to True.These changes will be used by https://github.com/ethyca/fidesplus/pull/1465
Code Changes
MonitorConfig
modelSteps to Confirm
monitorconfig
table has the columnsenabled
andlast_monitored
You can also test that you can downgrade migration:
monitorconfig
table does not have the columnsenabled
andlast_monitored
Pre-Merge Checklist
CHANGELOG.md
main
downgrade()
migration is correct and works