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

Warning about unsafe migrations #10129

Closed
mik-laj opened this issue Aug 3, 2020 · 4 comments
Closed

Warning about unsafe migrations #10129

mik-laj opened this issue Aug 3, 2020 · 4 comments
Assignees
Labels
area:upgrade Facilitating migration to a newer version of Airflow good first issue kind:feature Feature Requests

Comments

@mik-laj
Copy link
Member

mik-laj commented Aug 3, 2020

Helllo,

We currently have two migrations that cannot always be successful, which can lead to database corruption. All modern RDMSs should handle this case correctly and roll back the migrations, but ... it would be better if we detected these situations before starting the migration and not run the migration if we know that the operation will fail. This will also allow you to display much more user-friendly messages.
https://github.com/apache/airflow/blob/master/UPDATING.md#database-schema-changes

A similar check should be part of the airflow upgrade-check, but this command will be available in Airflow 1.10 and will not be run before migration starts.

Therefore, I propose that we write two small functions that will detect these two cases before starting the migration.
https://github.com/apache/airflow/blob/master/airflow/utils/db.py#L612
When this happens, we should display a message to the end-user along with an instruction describing the manual steps they have to do on their side.
Please note that if we do not have a table in the database, we should allow the migration to run. In other words, we should handle the case where the table does not exist in the database, so we cannot check for problematic database entries.

Best regards,
Kamil Breguła

@mik-laj mik-laj added kind:feature Feature Requests good first issue area:upgrade Facilitating migration to a newer version of Airflow labels Aug 3, 2020
@mik-laj mik-laj changed the title Warning about unsafe migration Warning about unsafe migrations Aug 3, 2020
@khyurri
Copy link
Contributor

khyurri commented Aug 3, 2020

Please, assign to me.

@mik-laj
Copy link
Member Author

mik-laj commented Aug 3, 2020

Done :-D Only 5 hours the task waiting for you to find it. I am very happy that we have such an active community. :-D

@khyurri
Copy link
Contributor

khyurri commented Aug 8, 2020

I've created PR.

Not sure, how to add this functionality into airflow upgrade-check, we can move this functions to a more suitable place.

@khyurri
Copy link
Contributor

khyurri commented Sep 28, 2020

@mik-laj
Please, review the PR: #10254

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:upgrade Facilitating migration to a newer version of Airflow good first issue kind:feature Feature Requests
Projects
None yet
Development

No branches or pull requests

4 participants