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.

Sign up for GitHub

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

Jump to bottom

handle migration failure gracefully #14

Closed
wants to merge 1 commit into from
Closed

handle migration failure gracefully #14

wants to merge 1 commit into from

Conversation

mickyarun
Copy link

@mickyarun mickyarun commented Oct 18, 2023 •
edited by nfriedly

Migration was failing if we use typeorm, so handled it gracefully and migration I am now handling it in by base project

Error: Migration failed. Reason: Hashes don't match for migrations '0_create-migrations-table.sql,1-init.sql,2-add-db-functions-agg.sql,3-add-db-functions-ind.sql,4-add-db-functions-sessions.sql,5-hotfix-update-constraints.sql'.
This means that the scripts have changed since it was applied.
    at C:\Users\shnat\Desktop\atoa3\ATOACore\node_modules\postgres-migrations\dist\migrate.js:100:27
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at C:\Users\shnat\Desktop\atoa3\ATOACore\node_modules\postgres-migrations\dist\with-lock.js:25:28
    at C:\Users\shnat\Desktop\atoa3\ATOACore\node_modules\postgres-migrations\dist\with-connection.js:16:28
    at applyMigrations (C:\Users\shnat\Desktop\atoa3\ATOACore\node_modules\@acpr\rate-limit-postgresql\dist\index.cjs:78:3)
[ERROR] 13:44:52 Error: Migration failed. Reason: Hashes don't match for migrations '0_create-migrations-table.sql,1-init.sql,2-add-db-functions-agg.sql,3-add-db-functions-ind.sql,4-add-db-functions-sessions.sql,5-hotfix-update-constraints.sql'.

This means that the scripts have changed since it was applied.

Fixes #13 (sort of)

@mickyarun mickyarun mentioned this pull request Oct 18, 2023
Open
nfriedly
nfriedly reviewed Oct 19, 2023
View reviewed changes
Copy link
Member

@nfriedly nfriedly left a comment •
edited

Choose a reason for hiding this comment

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

Hey @mickyarun,

Thanks for this, and sorry we didn't get back to you sooner.

The change seems reasonable to me, although I could imagine a situation where you'd prefer to have it throw an exception rather than log and continue. @adrianprelipcean what do you think?

Also, @mickyarun, you should run npm run lint-autofix to fix the formatting.

@adrianprelipcean
Copy link
Member

I am not used to ORMs, but in general it's bad practice to surround a migration process in a try / catch block, since there needs to be a guarantee that the migrations are applied.

I think there's also something odd with regards to the migrations applied since you have an extra migration 0_create-migrations-table.sql in your error message, which could break the process. Can you remove that file or move it to a different folder so it is not considered by the postgres-migrations instance used in this project?

@adrianprelipcean
Copy link
Member

Closing down this PR, the problem here is that migrations are hardcoded to run in public.migrations from postgres-migrations, which probably overlaps with your other ORMs. Unfortunately, since that project is not maintained anymore the relevant issues that are related to this topic are not considered. We could use a fork of postgres-migrations that supports changing the schema, although I'm not sure how well that is maintained yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nfriedly nfriedly nfriedly left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migration failing if project already have migration setup using typeorm
3 participants
@mickyarun @adrianprelipcean @nfriedly