[AIP-86] DeadlineAlerts UI integration - Part 1 #57464
Conversation
boring-cyborg
bot
added
area:db-migrations
| def upgrade(): | ||
| """Make changes to enable adding DeadlineAlerts to the UI.""" | ||
| # TODO: We may finally have come up with a better naming convention. For ease of migration, | ||
| # we are going to keep deadline_alert here to match the model's name, but in the near future |
There was a problem hiding this comment.
I do not understand why starting with deadline_alert and then renaming it to deadline_definition. Why not going with deadline_definition? You are writing the models in this PR, since it is a new table, so I do not get the "to match the model's name".
There was a problem hiding this comment.
Alerts is keeping with the existing naming. We're going t have to go through and change a lot more than just the contents of this one PR if we go forward with that idea
There was a problem hiding this comment.
But you'll do it anyway right? So why not doing it directly? You'll have to create a new migration to rename the DB, if we can avoid that, that'd be great
|
|
||
| def migrate_existing_deadline_alert_data_from_serialized_dag(): | ||
| """Extract DeadlineAlert data from serialized DAG data and populate deadline_alert table.""" | ||
| # TODO: Implement a batch-based migration to account for memory constraints |
There was a problem hiding this comment.
Do you intend to update this migration after it has been merged? It can disrupt developers. It is "better" to add new migrations and not modify existing ones in my opinion
ferruzzi
force-pushed
the
ferruzzi/deadlines/db-changes-for-ui/01-new-deadline-alert-table
branch
from
8095586 to
c95a855
Compare
2 participants
Had a chat with @bbovenzi and @ramitkataria at Summit about what we need to do to get Deadline Alerts integrated into the UI. The first main part is to move the DeadlineAlert data out of the serialized_dag and into a new table that can be accessed directly by the UI, so that's what we're tackling here. This is a bigger task than I imagined, so I am breaking it into a few parts.
Changes:
Part 1: Creates the new deadline_alerts table (and ORM) with fkey reference to the deadlines that they spawn, and lays out the plan for the data migration
What's next:
Testing:
Unit tests for the new ORM where possible with manual testing of the migration file since we don't generally do automated tests for those.
Qasi-related, I gave up on trying to think of a better name for these tables but @o-nikolas hit on a great one in a chat earlier today. I am going to deprecate
DeadlineAlertas a model/ORM/entity and replace it withDeadlineDefinition. Then when a user defines theDeadlineDefinition, a single instance of theDeadlineDefinitionis aDeadline. That flows much more logically than saying aDeadlineAlertspawnsDeadlines. But that's future work.cc @seanghaeli
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.