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.

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

replace schemamama with sqlx migrations #2327

Merged
merged 3 commits into from
Nov 22, 2023
Merged

Conversation

syphar
Copy link
Member

@syphar syphar commented Nov 19, 2023

This drops our schemamama migrations and replaces them with sqlx migrations, providing an initial migration file that represents just the current database schema.

Just running database migrate will also migrate the migration state, so updates should be seamless.

I removed running the reverse migrations from the teardown for every test case, I assume it's good enough to clean the whole schema. I added a separate check in CI to test the full setup & teardown only once.

There is one case where this setup would break: when a developer has set DATABASE_URL and wants to run cargo run -- database migrate it won't compile. sqlx migrate run should work then. I wonder if we should update the documentation to use sqlx migrate run, though this would mean that we need to install the sqlx CLI in more places.

Also I would love some feedback on what documentation should be updated?

@github-actions github-actions bot added the S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed label Nov 19, 2023
@Nemo157
Copy link
Member

Nemo157 commented Nov 21, 2023

I removed running the reverse migrations from the teardown for every test case, I assume it's good enough to clean the whole schema. I added a separate check in CI to test the full setup & teardown only once.

Running the down migrations after each test was added to try and get a variety of data in the database that might cause issues in the down migration logic. Otherwise we're only really checking the down migration syntax, not what it does to data.

src/db/mod.rs Outdated Show resolved Hide resolved
@syphar
Copy link
Member Author

syphar commented Nov 22, 2023

I removed running the reverse migrations from the teardown for every test case, I assume it's good enough to clean the whole schema. I added a separate check in CI to test the full setup & teardown only once.

Running the down migrations after each test was added to try and get a variety of data in the database that might cause issues in the down migration logic. Otherwise we're only really checking the down migration syntax, not what it does to data.

Thanks for the pointer, I added it back.

@syphar syphar merged commit 7f551f6 into rust-lang:master Nov 22, 2023
9 checks passed
@syphar syphar deleted the sqlx-migrate branch November 22, 2023 12:00
@github-actions github-actions bot added S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Nov 22, 2023
@syphar syphar removed the S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it label Nov 22, 2023
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