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

[supported] Add opt-in functionality to apply out-of-order migrations #262

Closed
mfridman opened this issue Aug 4, 2021 · 7 comments
Closed
Assignees

Comments

@mfridman
Copy link
Collaborator

mfridman commented Aug 4, 2021

This is a placeholder issue, will update the description.

tl;dr, over the past few years most of the issues reported have been around goose's opinionated way of dealing with out-of-order migrations.

I believe we can address many of those issues by allowing an opt-in mechanism, such as an explicit flag, to "disable" the strict behaviour of goose.

Although we can document and suggest best practices, at the end of the day these worked for us and may not be applicable for all other teams.

We should meet users in the middle (lots of great feedback from the community) and give them the flexibility to use goose as they see fit. The responsibility will be shifted from the tool itself, to the end user.

This can be implemented in a backwards-compatible way.

cc @VojtechVitek we've gone back and forth on this for a while. Do you have any objections to introduce this functionality?

@mfridman mfridman self-assigned this Aug 4, 2021
@mfridman mfridman pinned this issue Aug 4, 2021
@mfridman mfridman mentioned this issue Aug 26, 2021
@VojtechVitek
Copy link
Collaborator

@mfridman Explicit CLI flag would work for me.

@archseer
Copy link

We recently migrated to goose and got hit by this. I implemented a quick workaround here: e8cd8fc6

"strict" mode should also detect pending out-of-order migrations and raise an error. Current behaviour was a bit surprising because it silently skipped those migrations and we only caught it because a subsequent migration was relying on that missing state.

@devenbhooshan
Copy link

devenbhooshan commented Sep 23, 2021

for out-of-order issues if anyone is looking for adding a check in their CI systems.

the below script(when run from your project root) checks whether a new migration file is added at the last in the current branch(won't work if a file is directly added to the master branch)

#!/bin/bash

MIGRATION_FOLDER=storage/postgres/migration/

NEW_FILES=`git diff  --name-only --diff-filter=A origin/master | grep -i $MIGRATION_FOLDER`

declare -A NEW_FILES_FOUND

for filename in $NEW_FILES; do
  NEW_FILES_FOUND[$filename]=1
done

for filename in $(ls -1 $MIGRATION_FOLDER* | sort -r); do 
    if [ ${#NEW_FILES_FOUND[@]} -gt 0 ]; then
      if [ ! -v NEW_FILES_FOUND[$filename] ]; then
        echo "new migration file is not in the correct order"
        exit 1;
      fi
      unset NEW_FILES_FOUND[$filename]
    fi
done

echo "linting finished"

@shlima
Copy link

shlima commented Sep 27, 2021

+1

@mfridman
Copy link
Collaborator Author

"strict" mode should also detect pending out-of-order migrations and raise an error. Current behaviour was a bit surprising because it silently skipped those migrations and we only caught it because a subsequent migration was relying on that missing state.

This is a vlid point, skipping migrations silently should be opt-in behaviour instead of the default. Although modifying this behaviour is a breaking change, I'd argue this would be the correct behaviour for "strict" mode. From all the feedback we've received it appears this would be acceptable. I can't think of a time when we would have wanted a migration to be skipped. (also we do the hybrid approach, so we don't even get a chance to encounter this).

In "non-strict" mode goose applies all migrations even if they are out-of-order, and in this mode there isn't a chance to skip migrations since we're applying them all.

Before implementing this, I've kept this issue open to gather feedback. So thank you all for contributing your thoughts and use-cases.

@mfridman
Copy link
Collaborator Author

mfridman commented Oct 16, 2021

Suggestions welcome, this is a first-pass at an implementation in #280 for Up, UpByOne and UpTo

What do people like, out-of-order or allow-missing ?

tl;dr

binary

Adds -allow-missing or -out-of-order to allow applying those migrations. (name undecided)

library

Implemented with functional options.

goose.Up(
	db, 
	migrationsDir, 
	goose.WithAllowMissing(), // <- apply missing migrations before applying new migrations
)

Suppose migration 1 and 4 are applied and then version 2, 3, 5 are introduced. Today goose will ignore version 2 and 3 and apply 5.

In the new implementation goose will error (because 2 and 3 are lower than the current max version: 4), unless you explicitly set the flag (CLI) or option (library).

With this goose detects migrations 2, 3 and applies them, followed by migration 5. Which means the order in the database will be: 1, 4, 2, 3, 5

A missing migration is one that has not been applied and is lower that the current max version_id in the database.

@mfridman
Copy link
Collaborator Author

mfridman commented Oct 24, 2021

Marking fixed by #280

Notable change is goose will now raise an error (by default) if it detects missing migrations.

If you want to apply these missing migrations run goose with the -allow-missing flag, or if using as a library supply the functional option goose.WithAllowMissing() to Up, UpTo or UpByOne.

More info on the blog post.

@mfridman mfridman changed the title Add opt-in functionality to apply out-of-order migrations [supported] Add opt-in functionality to apply out-of-order migrations Nov 26, 2021
@mfridman mfridman unpinned this issue Apr 21, 2022
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

No branches or pull requests

5 participants