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

Handle missing schemas in ensure_up! #53

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

maltoe
Copy link
Contributor

@maltoe maltoe commented Dec 6, 2023

We noticed that c:Migrator.ensure_up!/0 does not check for existence of schemas before calling Ecto.Migrator.migrations/3, which unfortunately terminates with a rather cryptic error.

This PR fixes that by explicitly checking schema existence before calling migrations/3. Opted for only checking (instead of creating it) to keep ensure_up!/0's behaviour "read-only".


Unfortunately no test coverage as Migrator isn't tested at all, but I promise I've tested it locally.

image

We noticed that `c:Migrator.ensure_up!/0` does not check for existence
of schemas before calling `Ecto.Migrator.migrations/3`, which
unfortunately terminates with a rather cryptic error.

This PR fixes that by explicitly checking schema existence before
calling `migrations/3`. Opted for only checking (instead of creating it)
to keep `ensure_up!/0`'s behaviour "read-only".
@maltoe maltoe force-pushed the handle-missing-schemas-in-ensure-up branch from 9a66c18 to 0e1482c Compare December 6, 2023 12:37
@maltoe maltoe requested a review from hannahbit December 6, 2023 12:37
Copy link

@hannahbit hannahbit left a comment

Choose a reason for hiding this comment

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

Nice 🍨

@maltoe maltoe merged commit b0d3b4b into main Dec 6, 2023
1 check passed
@maltoe maltoe deleted the handle-missing-schemas-in-ensure-up branch December 6, 2023 14:13
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