You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/09/17 09:43:31 UTC
[GitHub] [airflow] potiuk opened a new issue #18323: Detect and fail any command when db is not migrated
potiuk opened a new issue #18323:
URL: https://github.com/apache/airflow/issues/18323
### Description
We had recently a number of questions from users who did not run `airflow db upgrade` and run airflow and it was failing because of missing columns etc.
It was not entirely clear for new users that they should do it and I am fixing it in #18282 , however I think we should do better
### Use case/motivation
I think we should check what migration is currently applied and fail any Airflow command that requires the DB hard with clear information that the DB is not migrated and what to do.
This should be rather easy, I think. SQLAlchemy keeps track of the migration and I believe it has already all the tools to determine "current migration" vs. "expected migration".
### Related issues
#18282
### Are you willing to submit a PR?
- [X] Yes I am willing to submit a PR!
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on issue #18323: Detect and fail any command when db is not migrated
Posted by GitBox <gi...@apache.org>.
eladkal commented on issue #18323:
URL: https://github.com/apache/airflow/issues/18323#issuecomment-1069383143
fixed in https://github.com/apache/airflow/pull/18439
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal closed issue #18323: Detect and fail any command when db is not migrated
Posted by GitBox <gi...@apache.org>.
eladkal closed issue #18323:
URL: https://github.com/apache/airflow/issues/18323
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr edited a comment on issue #18323: Detect and fail any command when db is not migrated
Posted by GitBox <gi...@apache.org>.
uranusjr edited a comment on issue #18323:
URL: https://github.com/apache/airflow/issues/18323#issuecomment-921800502
One minor thought (aside from we obviously can’t show this error for `airflow db upgrade`) is maybe we need to provide an override mechanism for local development, it’s occasionally useful knowingly run things without full migration (to fix an existing migration for example).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on issue #18323: Detect and fail any command when db is not migrated
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #18323:
URL: https://github.com/apache/airflow/issues/18323#issuecomment-921805978
> One minor thought (aside from we obviously can’t show this error for `airflow db upgrade`) is maybe we need to provide an override mechanism for local development, it’s occasionally useful knowingly run things without full migration (to fix an existing migration for example).
Yep. Good call. And yeah. I was toying with the thought of enabling the check for `airflow db upgrade` just for the fun of it.
I was thinking about adding it first and seeing if someone will find it during the review ;). But you spoiled my sneaky plan TP
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #18323: Detect and fail any command when db is not migrated
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #18323:
URL: https://github.com/apache/airflow/issues/18323#issuecomment-921661285
cc:@kaxil - WDYT? I do not know Alembic that well, but from what I understand, I believe this would be entirely possible to perform such a check and clearly raise error "Please migrate the db with `airflow db upgrade` in case the migration revisiion is different than expected?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on issue #18323: Detect and fail any command when db is not migrated
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #18323:
URL: https://github.com/apache/airflow/issues/18323#issuecomment-921661285
cc:@kaxil - WDYT? I do not know Alembic that well, but from what I understand, I believe this would be entirely possible to perform such a check and clearly raise error "Please migrate the db with `airflow db upgrade` " in case the migration revision is different than expected?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on issue #18323: Detect and fail any command when db is not migrated
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #18323:
URL: https://github.com/apache/airflow/issues/18323#issuecomment-922664742
Another one https://github.com/apache/airflow/discussions/18368
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on issue #18323: Detect and fail any command when db is not migrated
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #18323:
URL: https://github.com/apache/airflow/issues/18323#issuecomment-921805978
> One minor thought (aside from we obviously can’t show this error for `airflow db upgrade`) is maybe we need to provide an override mechanism for local development, it’s occasionally useful knowingly run things without full migration (to fix an existing migration for example).
Yep. Good call. And yeah. I was toying with the thought of enabling the check for `airflow db upgrade` just for the fun of it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bhavaniravi commented on issue #18323: Detect and fail any command when db is not migrated
Posted by GitBox <gi...@apache.org>.
bhavaniravi commented on issue #18323:
URL: https://github.com/apache/airflow/issues/18323#issuecomment-921686654
Yes, it's possible in the alembic. [Check this SO answer](https://stackoverflow.com/a/56085521/6340775)
A similar airflow snippet would look like, used `check_migrations` as a template
```
def compare_migrations():
from alembic.runtime.migration import MigrationContext
from alembic.script import ScriptDirectory
config = _get_alembic_config()
script_ = ScriptDirectory.from_config(config)
with settings.engine.connect() as connection:
context = MigrationContext.configure(connection)
if context.get_current_revision() != script_.get_current_head():
raise exceptions.DatabaseIsNotUpToDate('Upgrade the database.')
```
Do we do this check every time we start the webserver or scheduler?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on issue #18323: Detect and fail any command when db is not migrated
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #18323:
URL: https://github.com/apache/airflow/issues/18323#issuecomment-921661285
cc:@kaxil - WDYT? I do not know Alembic that well, but from what I understand, I believe this would be entirely possible to perform such a check and clearly raise error "Please migrate the db with `airflow db upgrade` in case the migration revision is different than expected?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on issue #18323: Detect and fail any command when db is not migrated
Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #18323:
URL: https://github.com/apache/airflow/issues/18323#issuecomment-921709060
Yea we have something similar that we already use in the Helm Chart: https://github.com/apache/airflow/blob/11621ce06616777f6709ffb7f029d733be78f297/airflow/utils/db.py#L622-L649
I will add that to the components
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on issue #18323: Detect and fail any command when db is not migrated
Posted by GitBox <gi...@apache.org>.
uranusjr commented on issue #18323:
URL: https://github.com/apache/airflow/issues/18323#issuecomment-921800502
One minor thought (aside from we obviously can’t show this error for `airflow db upgrade) is maybe we need to provide an override mechanism for local development, it’s occasionally useful knowingly run things without full migration (to fix an existing migration for example).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org