feat(hub-projects): generate and apply datatable migrations on project publish/install#9977
feat(hub-projects): generate and apply datatable migrations on project publish/install#9977diegoimbert wants to merge 127 commits into
Conversation
The merge command upserted datatable_migration definitions into the target workspace and reported the item as successfully deployed, but never ran the migrations. For forked datatables backed by separate databases, this left the target schema unchanged until someone manually ran `wmill datatable migrate up`, while the CLI reported a successful merge. Collect the datatable migrations deployed (not deleted) into the target and, after the deploy loop, offer to run them via the existing offerToRunNewMigrations helper — the same post-deploy run prompt the push/sync path uses (interactive only; `--yes`/non-TTY skip the mutating run, matching push behavior). Export parseDatatableMigrationDeployPath so the merge path can parse the deployed items. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n lock A migration run snapshots a migration's code_up from datatable_migrations and only records its version in the data table's _wm_migrations after the job succeeds. upsert_datatable_migration checked _wm_migrations before allowing an edit but took no lock, so a concurrent edit could read "not applied yet", rewrite code_up/code_down, and then the in-flight run would record the version for the old SQL — leaving _wm_migrations pointing at SQL that was never applied (migrate up then skips it; rollback runs a down that doesn't match). Serialize definition rewrites and deletes with the same per-database advisory lock the run/rollback paths use: - Factor the connect+advisory-lock into lock_datatable_migration_runs and the applied-versions read into read_applied_versions_on_client. - run_datatable_migrations now snapshots the definitions AFTER taking the lock, so code_up can't change between snapshot and version-record. - upsert (when changing an existing def) and delete take the lock across the applied-check and the write; delete now rejects deleting an already-applied migration (would orphan its _wm_migrations record), symmetric with upsert. Both fail closed if the data table database is unreachable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…editor preview Creating a table on a migrations-enabled data table opened the DB table editor's "Confirm running the following" preview modal, whose confirm triggers applyDdl, which then asks for out-of-order confirmation. Both are ConfirmationModals with a hardcoded z-[9999]; the out-of-order one lives in DBManagerContent (mounted before the editor), so it rendered behind the still-open preview modal. Add an optional zIndexClass prop to ConfirmationModal (default z-[9999], backward-compatible) and give the DB-manager out-of-order confirm z-[10000] so it stacks on top. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Detect datatable assets in a project's scripts/flows/raw apps when publishing to the Hub, generate a best-effort CREATE TABLE migration per data table from the source workspace's live schema, and let the publisher edit/toggle them in the bundle drawer. On import, offer to run the shipped migrations: recorded (datatable_migrations + _wm_migrations) when the target data table opted into migrations, otherwise as a one-off preview job. Missing target data tables are surfaced and skipped. - backend: POST /hub/migrations proxy forwarding to the Hub - frontend publish: projectMigrations.ts detection + generation, new "Data table migrations" section in DeployToHub - frontend import: run/skip modal + missing-datatable confirmation - extract pure SQL-gen from DatatableSchemaDiff.svelte into datatableSchemaSql.ts so plain .ts modules can import it Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…keys Pull a referenced table's FK targets into the generated migration transitively, so it creates every table it references (ordered by FK dependency), and drop any FK whose target still isn't in the set so the generated SQL always runs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deploying windmill with
|
| Latest commit: |
232002b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://67039902.windmill.pages.dev |
| Branch Preview URL: | https://di-hub-projects-datatable.windmill.pages.dev |
Detect data table usage off the predeploy bundle preview and surface it as a "Data table dependencies" summary right after "Resource dependencies", mirroring how resource types and triggers are shown. The editable migration itself stays in the bundle drawer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When a table can't be found in the schema, a data table is referenced as a whole, or the schema can't be loaded, write a `--` comment describing the problem into the migration instead of leaving it blank. Partial migrations keep the CREATE TABLEs that did generate and comment the rest; comment-only migrations stay disabled. The bundle drawer now always shows the SQL box so those comments are visible and editable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…migration Replace the plain "run migrations?" confirmation with a review drawer that previews each runnable migration, lets the user edit the SQL and toggle which to run, before the import proceeds. When recording an imported migration, also record a down migration (DROP TABLE of the created tables, in reverse order) derived from the up SQL, so it can be rolled back; the derived rollback is previewed in both the publish bundle drawer and the import review drawer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the plain textarea with a Monaco SQL editor split into Up/Down tabs. The down migration is now generated once as best-effort (DROP TABLE in reverse creation order) and is fully editable — no longer parsed back out of the up SQL. The down is threaded through publish → Hub → import (new project_migration.sql_down) and recorded as code_down when an imported migration is applied. - projectMigrations: GeneratedMigration.sql_down generated from the table set - MigrationSqlEditor.svelte: shared Up/Down tabbed Monaco editor (re-keyed on regeneration since Monaco ignores external code changes) - DeployToHub + install review drawer use it; sql_down pushed/applied - backend: PublishMigrationBody carries sql_down Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
/ai review this PR |
|
Claude finished @diegoimbert's task in 3m 49s —— View job Review: datatable migrations on project publish/installRead through all 8 files and traced the generation/install logic. Overall this is well-structured and thoroughly commented — the FK-closure + prune + topological-order pipeline is sound and the test coverage on the generation side is good. A few correctness/robustness points, none blocking: 1. Up migration is not idempotent, but FK closure makes collisions likely (medium)
2.
|
…grations The FK closure pulls a referenced table's parents into the same transaction (e.g. `orders` drags in `customers`); those shared parents often already exist in the target, so a plain CREATE TABLE aborted the whole migration on the first collision. Emit CREATE TABLE IF NOT EXISTS for project migrations (via a new opt-in flag on generateMigrationSql, leaving the schema-diff behavior unchanged) so a pre-existing parent is skipped instead of failing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
orderByFkDependency keyed its dependency graph by bare table name (and
resolved FK targets with .split('.').pop()), so two same-named tables in
different schemas collapsed and one was dropped from the ordered set and
never created. Key by schema.table like the rest of the pipeline, resolving
FK targets through resolveTable. Also let resolveTable fall back to the bare
table name when a schema-qualified ref's schema doesn't match.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The generated down migration listed DROP TABLE for every table in the FK closure, including shared parent tables that may have pre-existed in the target — a rollback could drop a table the project never created (data loss). Emit all DROP statements commented out with a note, so nothing is dropped by default; the publisher uncomments the tables this migration actually owns. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
planMigrations awaits the review / missing-datatable modals before setting installing = true, so the Import button stayed enabled during review and a second click launched a concurrent install() (second review drawer, duplicated item creation). Track a planningMigrations flag, disable the button on it, and early-return install() if already installing or planning. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
regenerateMigrations cleared the drafts on error, showing "No data table usage detected" — indistinguishable from a genuine schema-load failure. Add a toast on the catch so the publisher can tell the two apart. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
/ai review this PR |
|
Claude finished @diegoimbert's task in 3m 14s —— View job Review: datatable migrations on project publish/installRe-reviewed the full diff against 1. The FK
|
planMigrations awaited missingDatatableModal.ask() but ignored its boolean, so cancelling the "some data tables are missing" warning still proceeded with the import — the cancel affordance did nothing. Show the warning first and abort the whole import when the user cancels (planMigrations returns null; install() early-returns), so they can create the data table(s) and re-run. Confirming still imports without the missing migrations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Low-code apps don't carry a persisted asset list, but the DB-table component declares its data table and table explicitly: a `oneOf` `type` config with `selected === 'datatable'` holding `datatable://<name>` and the table. Walk the app value for those configs so an app that reads a data table is picked up by the Data table dependencies detection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…able config" This reverts commit 9c43ebd.
Full-code (raw) apps explicitly declare the data tables/tables they use in value.data.tables (refs like main/customers or main/schema:table), which the "Data table dependencies" detection missed — it only looked at inline-script assets. Read the declaration via extractDataConfig/parseDataTableRef. The bundler previously dropped value.data (kept only files + runnables); include it so detection sees it and the imported app keeps its declaration, and pass it through on import. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Integrates datatable migrations into the Hub projects feature. When a project is published to the Hub, its scripts/flows/raw apps are scanned for
datatableassets (datatable_name/table_name[/column_name]), and a best-effortCREATE TABLEmigration is generated per referenced data table from the source workspace's live schema. On import, the shipped migrations can be run against the target workspace.Changes
Detection & generation (publish side)
projectMigrations.ts: detect datatable assets across a project's selected scripts/flows/raw apps and generate aCREATE TABLEmigration per data table from the source workspace's live schema.DeployToHub— the publisher can edit/toggle the generated migrations in the bundle drawer.DatatableSchemaDiff.svelteintodatatableSchemaSql.tsso plain.tsmodules can import it.Import side
projects/install: run/skip modal for the shipped migrations. When the target data table opted into migrations they're applied as recorded migrations (datatable_migrations+_wm_migrations); otherwise as a one-off preview job.Backend
POST /hub/migrationsproxy inhub_publish.rsforwarding to the Hub (consistent with the existing thin-proxy pattern).Fix
ConfirmationModalgains an optionalzIndexClassprop (defaultz-[9999], backward-compatible); the DB-manager out-of-order confirm usesz-[10000]so it stacks above the DB table editor's still-open preview modal.Test plan
projectMigrations.test.ts— asset detection, migration generation, FK closure orderingmain/tableAandmain/tableB→ confirm one migration per table is generated with correct columns and FK-orderedCREATE TABLEsdatatable_migrations+_wm_migrationsScreenshots
Deferred. Exercising the publish→install datatable-migration flow requires the full backend + Hub running; screenshots to be added once verified against a running environment.
🤖 Generated with Claude Code