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

Create new databases from the ORM #24156

Merged
merged 14 commits into from
Aug 1, 2022

Conversation

ephraimbuddy
Copy link
Contributor

@ephraimbuddy ephraimbuddy commented Jun 3, 2022

This PR opens up to creating new databases from the ORM instead of going through the migration files.

airflow db init creates the new db.

Depends on ##24044

@boring-cyborg boring-cyborg bot added the area:webserver Webserver related Issues label Jun 3, 2022
@ephraimbuddy ephraimbuddy marked this pull request as draft June 3, 2022 08:50
@ephraimbuddy
Copy link
Contributor Author

Failing tests will be fixed when #24044 is merged. Also, I will need to update the migration in #24044 to add conditional index and unique on task_instance table for mssql/mysql

@norm
Copy link
Contributor

norm commented Jun 7, 2022

This is not a prompt to change anything, merely an observation — it feels weird to create an instance of the webserver to init the DB.

@ephraimbuddy
Copy link
Contributor Author

This is not a prompt to change anything, merely an observation — it feels weird to create an instance of the webserver to init the DB.

I have moved the creation of the session into the webserver. This means DB initialization would not create the session table but starting up the webserver would create the session table. This makes sense since the session table is from flask_session and not part of airflow.

@ephraimbuddy ephraimbuddy marked this pull request as ready for review June 26, 2022 22:27
@ephraimbuddy ephraimbuddy added the full tests needed We need to run full set of tests for this PR to merge label Jun 26, 2022
@ephraimbuddy ephraimbuddy force-pushed the create-new-db-orm branch 5 times, most recently from 204cf7e to df04aae Compare June 27, 2022 08:04
@uranusjr
Copy link
Member

SQLite is failing with

  File "/opt/airflow/airflow/migrations/versions/0106_2_3_0_update_migration_for_fab_tables_to_add_missing_constraints.py", line 82, in downgrade
    batch_op.drop_constraint('ab_view_menu_name_uq', type_='unique')
  File "/usr/local/lib/python3.7/contextlib.py", line 119, in __exit__
    next(self.gen)
  File "/usr/local/lib/python3.7/site-packages/alembic/operations/base.py", line 376, in batch_alter_table
    impl.flush()
  File "/usr/local/lib/python3.7/site-packages/alembic/operations/batch.py", line 142, in flush
    fn(*arg, **kw)
  File "/usr/local/lib/python3.7/site-packages/alembic/operations/batch.py", line 680, in drop_constraint
    raise ValueError("No such constraint: '%s'" % const.name)
ValueError: No such constraint: 'ab_view_menu_name_uq'

@ephraimbuddy ephraimbuddy force-pushed the create-new-db-orm branch 5 times, most recently from 00dcb3e to 783a71e Compare July 4, 2022 13:47
airflow/models/base.py Outdated Show resolved Hide resolved
Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

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

Given the conditionals we need to have in initdb it might be worth it if we re-factor it to be:

@provide_session
def initdb(session: Session = NEW_SESSION):
    """Initialize Airflow database."""
    if db_exists:
        _create_db_from_orm(session=session)
    else:
        upgradedb(session=session)

    if conf.getboolean('database', 'LOAD_DEFAULT_CONNECTIONS'):
        create_default_connections(session=session)

And that way upgradedb can call this new function.

airflow/utils/db.py Show resolved Hide resolved
airflow/utils/db.py Outdated Show resolved Hide resolved
airflow/utils/db.py Outdated Show resolved Hide resolved
airflow/utils/db.py Outdated Show resolved Hide resolved
Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

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

Generally lgtm, just one small-ish thing

@ephraimbuddy ephraimbuddy merged commit 5588c3f into apache:main Aug 1, 2022
@ephraimbuddy ephraimbuddy deleted the create-new-db-orm branch August 1, 2022 12:01
@ephraimbuddy ephraimbuddy added the type:new-feature Changelog: New Features label Aug 15, 2022
@ephraimbuddy ephraimbuddy added this to the Airflow 2.4.0 milestone Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:webserver Webserver related Issues full tests needed We need to run full set of tests for this PR to merge type:new-feature Changelog: New Features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants