Allow schema specification when migrating#611
Conversation
The upstream copy of our postgres-migrations project has an issue where it does not support postgres schemas, and if ANY schema has a migrations table then migrations will fail. This was frustrating for running tests, but recently while looking into developing a job queue which would require a separate schema it became a blocking issue. Upstream also appears to no longer be maintained (the most recent commit as of this writing was about 2 years ago, and the PR we opened to fix the schema limitation has been open for around 18 months). Fortunately there is a fork of the library designed specifically for adding schema support. This replaces the library with the modified version and also updates our migration code to utilize the schema functionality. Issue #609 Consider finding a new tool for migrations
| import { db } from './db'; | ||
|
|
||
| export const migrate = async (): Promise<void> => { | ||
| export const migrate = async (schema = 'public'): Promise<void> => { |
There was a problem hiding this comment.
Someone with stronger DB opinions should let me know if it would be better to use a pdc schema as our default, rather than public.
There was a problem hiding this comment.
I think it's OK to default to public. My sense is that more often than not a dedicated database instance gets created for each separate application.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #611 +/- ##
=======================================
Coverage 95.44% 95.44%
=======================================
Files 70 70
Lines 1009 1009
Branches 163 164 +1
=======================================
Hits 963 963
Misses 43 43
Partials 3 3 ☔ View full report in Codecov by Sentry. |
bickelj
left a comment
bickelj
left a comment
There was a problem hiding this comment.
LGTM. I didn't try it locally, but I see the checks passed and no red flags stand out.
| "pino": "^8.16.2", | ||
| "pino-http": "^8.5.1", | ||
| "postgres-migrations": "^5.3.0", | ||
| "postgres-schema-migrations": "^6.1.0", |
There was a problem hiding this comment.
Nice find! You might want to mention this package in ThomWright/postgres-migrations#93 so others can find it too.
There was a problem hiding this comment.
I actually learned about it from a comment on the issue that PR addresses (ThomWright/postgres-migrations#92 (comment))
jasonaowen
left a comment
jasonaowen
left a comment
There was a problem hiding this comment.
Oh perfect - that fork is on npm after all! I'd missed that it got renamed.
I was able to run this (well, main, since this is a post-hoc review) locally without issue.
+1! Great work, @slifty. 💙
3 participants
This PR replaces our migration tool with one that supports multiple schemas, and utilizes that tool when running tests.
Resolves #609