Skip to content
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

Ensure that dag_id, run_id and execution_date are non-null on DagRun #18804

Merged
merged 9 commits into from
Oct 8, 2021

Conversation

ashb
Copy link
Member

@ashb ashb commented Oct 7, 2021

These should be non-nullable, and are always created as such. Without
this it was possible that someone had manually edited it which caused
problems with the TaskInstance FK migration not applying correctly.

Closes #18801


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.

@ashb ashb added the full tests needed We need to run full set of tests for this PR to merge label Oct 7, 2021
@ashb ashb requested review from kaxil and XD-DENG as code owners October 7, 2021 11:30
@ashb ashb added this to the Airflow 2.2.0 milestone Oct 7, 2021
Copy link
Member

@kaxil kaxil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find 306a0384de71 migration, did you miss a file?

@ashb
Copy link
Member Author

ashb commented Oct 7, 2021

🤦

@ashb ashb requested review from uranusjr and kaxil October 7, 2021 13:21
@ashb ashb force-pushed the non-null-dag-runid branch from c845434 to 3d12d07 Compare October 7, 2021 13:21
@kaxil kaxil self-requested a review October 7, 2021 13:40
@ashb
Copy link
Member Author

ashb commented Oct 7, 2021

This didn't apply on mysql, so rather than creating the FK in one migration, then dropping it to alter the column types and then recreating it, we should just change the column types before hand.

ashb and others added 7 commits October 7, 2021 22:57
These _should_ be non-nullable, and are always created as such. Without
this it was possible that someone had manually edited it which caused
problems with the TaskInstance FK migration not applying correctly.
@ashb ashb force-pushed the non-null-dag-runid branch from acce77c to fc1f5be Compare October 7, 2021 21:57
@ashb ashb merged commit e6c56c4 into apache:main Oct 8, 2021
@ashb ashb deleted the non-null-dag-runid branch October 8, 2021 00:32
@robinedwards
Copy link
Contributor

I am guessing this will be merged into v2-2-test at some point?

@potiuk
Copy link
Member

potiuk commented Oct 8, 2021

actually we keep on moving v2-2-test with main but generally speaking this one is marked for 2.2.0 so yeah - it will land in v2-2-test

@potiuk
Copy link
Member

potiuk commented Oct 8, 2021

Only after we cut an RC the branches will diverge

@kaxil
Copy link
Member

kaxil commented Oct 8, 2021

Yup this is now already part of v2-2-test and part of 2.2.0 rc1.

https://github.com/apache/airflow/commits/v2-2-test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full tests needed We need to run full set of tests for this PR to merge kind:documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrations fail to apply when there are null run_id's in the dag_run table
6 participants