Helper Methods #111

Closed ankane closed 4 years ago

I think helper methods can be very powerful for Strong Migrations (shout-out to @fatkodima for lots of work on this and GitLab for popularizing the concept), but it's a fundamental change from the approach Strong Migrations has taken in the past, so I wanted to discuss it here and get feedback from the community.

Trade-offs

Currently, Strong Migrations is focused on keeping the migration experience consistent with vanilla Rails and educating users on what's not safe. The advantages are:

Users don't need to learn new methods
Users get to learn a bit about databases

Helper methods change this by introducing new methods that make it less obvious about what's going on behind-the-scenes. The advantages are:

Simpler code - the new methods don't require users to jump through hoops to make migrations safe
Less to think about - many users may not care about what's going on behind-the-scenes

Choice

The good news is we can give teams a choice.

StrongMigrations.enable_helpers

This will:

Add the helper methods to migrations
Change the messages users see about how to make migrations safe

If one is overwhelmingly preferred, we can consider removing the other in the future.

The drawbacks of a choice are:

Increased maintenance
Increased complexity / possible confusion with docs, issues, etc

Implementation

Two options that come to mind are:

Prefix or suffix methods

add_foreign_key_safely # safe method
add_foreign_key        # unsafe method with safety checks

Override existing methods (eliminates need for safety_assured)

add_foreign_key  # safe method
add_foreign_key! # original method, no safety checks

This doesn't require users to learn new methods, but it's more invasive, as it changes the behavior of existing Rails methods.

Would love to hear everyone's thoughts.

Personally, I'd prefer suffix methods as it is obvious what is the difference, while I will always forget what is the difference between bang/nobang methods. And this won't change behavior of existing Rails methods.

As for StrongMigrations.enable_helpers, I think it will be better to not have any options to enable/disable it and always have those methods available to use.

To solve mentioned trade-offs, maybe it will be better to expand a little existing warning messages, something like this:

Changing the type of an existing column requires the entire
table and indexes to be rewritten. A safer approach is to:
1. ...
2. ...
3. ...
...

You can achieve this using change_column_safely ...

So, having 2 logical blocks: 1) general theory and 2) some notes on how process can be simplified by available methods, if available.

Any interest in doing it in a separate gem? I feel like that would keep the focus of this one small and clear. It also makes it easy to opt-in or out via Bundler rather than gem configuration.

Hey @Skipants, that sounds like the best approach. The gem could include the helpers as well as override Strong Migration's error messages to use the helpers.

@fatkodima Is this something you're interested in creating and running?

Hey. I'm more on the side of mine approach. I'm not interested in that, at least right now.

No worries. However, I think it's better as a separate gem at this point. Going to close out the existing PRs, but I appreciate the contributions.

Has anyone started on this separate gem, @ankane ?

Hey @kkumler, not that I'm aware of.

@kkumler There's https://github.com/doctolib/safe-pg-migrations that provides some helper methods.

@pirj Thanks for the consideration to mention me after so long. That project looks interesting, though I think the safe_by_default methods added into strong_migrations may satisfy the majority of user needs, but it's still definitely worth a look!

@mvz You requested a safe rename_table helper. Fresh from the oven.

Thanks, @pirj!

Beware, safe-pg-migrations does disable_ddl_transaction! for ALL migrations.

@mvz @pirj @Skipants @kkumler I created a separate gem with helpers 🎉 Aka strong_migrations on steroids.

It has helpers for renaming tables/columns, changing columns types (including changing primary keys from integer to bigint), adding columns with default values, adding different constraints, and many more, and be able to do data migrations on very large tables in background. Please, check out! https://github.com/fatkodima/online_migrations

Just need to make a release.

ankane / strong_migrations