-
Notifications
You must be signed in to change notification settings - Fork 176
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
Backfilling data in the same migration as adding a column not being caught? #228
Comments
Hi @Rockster160, Strong Migrations won't catch this (and hasn't in the past). You could use a custom check to track that |
That's unfortunate. 😞 For now, we added a very hacky solution where we just parse the file and look for a few blacklisted key words. StrongMigrations.add_check do |method, args|
filename = Dir["db/migrate/#{version}_*.rb"].first
contents = File.read(filename)
blacklist_updates = [:update, :save, :create]
blacklist_updates.each do |word|
# Check if any uncommented lines contain any of the blacklisted words
blacklist_regex = /^(?!.*#.*#{word})(?!.*#{word}.*#).*#{word}.*?/
next unless contents.match?(blacklist_regex)
message = "Don't backfill data in the same migration as adding columns!"
stop!(message, header: "Unsafe Data Migration")
end
end This looks for |
@Rockster160 thanks for this example! I'm adapting it for our own use. In walking through the regex, I noticed that it passes the following example with a comment after the keyword, when I think one might want it to call def change
User.update_all("full_name = 'oops'") # this should be stopped, but it is not
end It's the highlighted portion here: Thanks again - very helpful. |
You're right! I'm unsure what the reasoning behind that was! blacklist_updates = [:update, :save, "create\b"] |
Using the example provided by the documentation as an example of a bad migration:
strong_migrations
does not catch this and allows it to run.We accidentally let one of these slip through and it caused the DB to lock while running the backfill.
I thought this used to be caught- we moved to
strong_migrations
fromzero_downtime_migrations
which definitely caught this.If it's not built-in, is there a a way to set up a custom check to raise an error when these are detected?
Rails 6.1.7.3
ruby 3.2.2
strong_migrations-1.4.1
The text was updated successfully, but these errors were encountered: