Skip to content

Auto migration #526

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

Open
wants to merge 115 commits into
base: dev
Choose a base branch
from
Open

Auto migration #526

wants to merge 115 commits into from

Conversation

fomalhautb
Copy link
Contributor

@fomalhautb fomalhautb commented Mar 10, 2025


Important

Replaces Prisma's migration commands with custom scripts, updating workflows, scripts, and documentation to support new database migration strategy.

  • Behavior:
    • Replaces prisma migrate commands with custom migration scripts in db-migrations.ts.
    • Introduces db:init, db:reset, db:seed, and db:migration-gen commands in package.json and apps/backend/package.json.
    • Updates CI workflows (check-prisma-migrations.yaml, e2e-api-tests.yaml) to use new migration commands.
  • Scripts:
    • Adds db-migrations.ts for handling database migrations, including reset, init, and seed operations.
    • Adds generate-migration-imports.ts to generate migration imports.
  • Migrations:
    • Modifies SQL migration files to remove transaction blocks and add custom sentinels for statement splitting.
    • Updates migration files to align with new migration handling.
  • Testing:
    • Adds auto-migration.tests.ts to test new migration logic.
  • Documentation:
    • Updates README.md and self-host.mdx to reflect new migration commands.
  • Misc:
    • Updates turbo.json to include new migration tasks.
    • Adds dependencies chokidar-cli, dotenv, and postgres to apps/backend/package.json.

This description was created by Ellipsis for bfc09f3. You can customize this summary. It will automatically update as commits are pushed.

Copy link

vercel bot commented Mar 10, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
stack-backend ❌ Failed (Inspect) Jul 18, 2025 11:28pm
stack-dashboard ❌ Failed (Inspect) Jul 18, 2025 11:28pm
stack-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 18, 2025 11:28pm
stack-docs ❌ Failed (Inspect) Jul 18, 2025 11:28pm

Copy link

recurseml bot commented Mar 10, 2025

😱 Found 8 issues. Time to roll up your sleeves! 😱

🗒️ View all ignored comments in this repo
  • The constraint 'TokenStoreType extends string' is too restrictive. It should likely be 'TokenStoreType extends string | object' to match the condition check in line 113 where TokenStoreType is checked against {}
  • Return type mismatch - the interface declares useUsers() returning ServerUser[] but the Team interface that this extends declares useUsers() returning TeamUser[]
  • There is a syntax error in the super constructor call due to the ellipsis operator used incorrectly. Objects aren't being merged correctly. This syntax usage can lead to runtime errors when trying to pass the merged object to 'super()'. Verify that the intended alterations to the object occur before or outside of the super() call if needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

how will the new scripts work with source-of-truth? assumably we also have to update the new github workflows when we merge the branch? (just putting here as a reminder for when you merge branches)

@@ -36,3 +36,5 @@ yarn-error.log*
# typescript
*.tsbuildinfo
next-env.d.ts

src/auto-migrations/migration-files.tsx
Copy link
Contributor

Choose a reason for hiding this comment

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

can we put this in src/generated/auto-migrations or so?

Copy link
Contributor

Choose a reason for hiding this comment

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

and then the .gitignore can be in src/generated like in the client library

Comment on lines +52 to +58
await runQueryAndMigrateIfNeeded({
prismaClient,
fn: async () => {
await tx.$queryRaw(getMigrationCheckQuery());
},
});

Copy link
Contributor

Choose a reason for hiding this comment

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

run when the prismaclient is created instead

}> {
const migrationFiles = options.migrationFiles ?? MIGRATION_FILES;

await acquireMigrationLock({ prismaClient: options.prismaClient });
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
await acquireMigrationLock({ prismaClient: options.prismaClient });
// TODO we should not acquire a lock if the migrations are already up-to-date, and at most once for every migration that is being applied. (right now, in a long-running migration which will cause many servers to restart, all those servers after the first one would queue up and lock one after the other just to see that we're already up-to-date and release the lock again. in the future, we could use a mix of shared and exclusive locks to make sure that only one client at a time attempts to get an exclusive lock, and if it fails to get that, goes back to acquiring a shared lock instead)
await acquireMigrationLock({ prismaClient: options.prismaClient });

would be better if we don't acquire a lock when all migrations are up-to-date, but I think we can just leave a todo here and worry later

Comment on lines 106 to 112
-- Check if started_at exists and is within 10 seconds
IF started_at_value IS NOT NULL AND started_at_value >= clock_timestamp() - INTERVAL '10 seconds' THEN
RAISE EXCEPTION 'MIGRATION_IN_PROGRESS';
ELSIF started_at_value IS NOT NULL AND started_at_value < clock_timestamp() - INTERVAL '10 seconds' THEN
-- Update the startedAt to current timestamp since it's older than 10 seconds and start this migration
UPDATE "SchemaMigrationLock" SET "startedAt" = clock_timestamp() WHERE id = 1;
END IF;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will still potentially cause multiple migrations at the same time? the transactions are not cancelled if the lock is given to someone else after 10 seconds

@N2D4 N2D4 assigned fomalhautb and unassigned N2D4 Jul 18, 2025
fomalhautb and others added 7 commits July 18, 2025 14:15
This reverts commit b41155d.
Co-authored-by: Konsti Wohlwend <n2d4xc@gmail.com>
Co-authored-by: Konsti Wohlwend <n2d4xc@gmail.com>
- Update import path for migration files to point to the new generated location.
- Rename function `isMigrationNeeded` to `isMigrationNeededError` for clarity.
- Remove obsolete entry from .gitignore related to migration files.

Co-authored-by: Konsti Wohlwend <n2d4xc@gmail.com>
- Introduced a call to the `seed()` function in the main migration process to ensure the database is populated with initial data after migration.
- Added a call to the `seed()` function back into the main migration flow to ensure the database is populated with initial data after executing migrations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants