-
Notifications
You must be signed in to change notification settings - Fork 530
Fixing bootstrapping issues with migrations. #2047
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
base: master
Are you sure you want to change the base?
Conversation
@kangmingtay can you please review this so it can get merged? It's an ongoing issue for a lot for people. |
Signed-off-by: Sienna Satterwhite <[email protected]>
ca5c731
to
25697bc
Compare
+1 |
I guess supabase really doesn't care about OSS contributions |
6f2c5c9
to
5f2b660
Compare
Hi @siennathesane @willnode — First I wanted to say we do care and appreciate both your contributions very much. Sorry for the delay on these reviews, I'll bring this up with the team to brainstorm how to improve. I reviewed all the migration related PRs/issues and wanted to ensure the original contributions from @willnode (#1983, #2040) and the (id::text = user_id::text fix) in #2047 were preserved correctly so I was going to create a new PR with everything cherry picked. I mistakingly force pushed to the wrong upstream after cherry picking 1983 & 2040, which lost the ID change. To try to fix this I reintroduced your id migration fix as a separate commit using your previous git log message and commit. However, I noticed that GitHub now marks your commit as unverified since it didn’t come from your GPG key, and I want to respect your authorship standards. If you would like to force push a signed commit and tag me I'll make sure this gets reviewed. Sorry for the trouble but looking forward to getting these migration pulls and issues closed out. |
You can force push again to the original commit 25697bc which should restore this PR |
5072c4d
to
25697bc
Compare
@willnode Since you are okay with losing authorship I've restored the prior commit. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@willnode when I wrote this, I had included your fixes plus my own. Didn't mean to erase your contribution at all! 🫶 |
LGTM |
Pull Request Test Coverage Report for Build 16780325165Details
💛 - Coveralls |
What kind of change does this PR introduce?
Big fix for #1729, #1848, #1983, and #2040 with an additional type fix.
What is the current behavior?
The auth service cannot be deployed in a net new environment on PostgreSQL 17.
What is the new behavior?
The service is running properly with PostgreSQL 17 in a cleanroom environment.
Additional context
Here is a redacted version of the terraform I used to deploy it with. I used my own container build with these fixes,
ghcr.io/siennathesane/auth:v2.175.0
, that you can use to verify the fix is valid, if you want.Closes #1729
Closes #1848
Closes #1983
Closes #2040