Add DagsFolderDagBundle to db during migration/init #44742
Conversation
boring-cyborg
bot
added
area:db-migrations
There was a problem hiding this comment.
I'll just mention some concerns here for posterity while approving.
I think it would be better if setting up bundles could at least optionally be config driven and not just through database because that would be a lot simpler for CI processes. Beyond it simply being easier, it seems there is maybe a bit of a deploy atomicity problem, in the sense that, with it being purely db-driven, then we sort of cannot atomically upgrade the image and the configured bundle version together.
My other thought is that, I sorta think it would be better if we did not have to create a db entry for the default bundle, that it could just be a default that is provided from the code, because there's nothing really that we can configure on it, and the user did not create it, to me it would make a little more sense to just not create it an orm object for it. But this does not seem to be of great consequence so 🤷
| @@ -58,6 +60,10 @@ def upgrade(): | |||
| batch_op.add_column(sa.Column("latest_bundle_version", sa.String(length=200), nullable=True)) | |||
| batch_op.create_foreign_key(batch_op.f("dag_bundle_id_fkey"), "dag_bundle", ["bundle_id"], ["id"]) | |||
|
|
|||
| with Session(bind=op.get_bind()) as session: | |||
| add_dags_folder_dag_bundle(session=session) | |||
| session.commit() | |||
There was a problem hiding this comment.
This will break offline SQL generation of migrations
|
Replaced by #44924, which moves this into config. |
This adds the DagsFolderDagBundle (aka the bundle exposing the folder configured in
[core] DAGS_FOLDER) as a bundle in the db.It will be added during migrations, and when you init a fresh db from the ORM models. It will not, however, recreate it you've chosen to remove it from an existing instance when that instance starts.