Infer migrations_paths from database name

[seejohnrun]

Summary

Similar to what we do for schema files, we now infer the default migrations_paths for a database configuration using the database specification name.

We still allow overriding these defaults with a migrations_paths key in the database configuration file.

Additionally, we've updated some documentation accordingly in the guides.

We've also removed Migrator.migrations_paths since we should be taking the migrations_paths off of the config instead.

cc / @eileencodes

[rails-bot[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[eileencodes]

So we don't forget again in the future:

The problem we hit with this PR is that we don't know which entry is the default and therefore should get db/migrate as its migrations paths and which entries should be inferred as their own name. We originally considered making primary required but eventually decided against that. I think there are a few ways to solve this but they all have caveats:

1. Require that apps using inferred paths add db/migrate to at least one of the configurations so we know which one the default is. This is probably quite error prone, and confusing for users.
2. Require that apps mark a configuration as default. This would help us in other places but getting this message out to apps may be difficult.
3. Assume the first configuration is the default. We do this in other places, it could make a lot of sense, but it's a bit implicit and not really explicit. Potentially we could combine 2 and 3 - if there's a default defined, we use that for db/migrate, if not we assume it's the first one.

[jasonkarns]

This (or the lack of this) behavior just bit us. The docs indicate (as of 7.0.3.1) that option 3 above is followed

If a primary configuration is provided, it will be used as the "default" configuration. If there is no configuration named "primary", Rails will use the first configuration as default for each environment. The default configurations will use the default Rails filenames. For example, primary configurations will use schema.rb for the schema file, whereas all the other entries will use [CONFIGURATION_NAMESPACE]_schema.rb for the filename.

This phrasing (specifically: "will use the default Rails filenames") makes it seemingly reasonable to assume that the migration path would also be inferred as db/[CONFIGURATION_NAMESPACE]_migrate.

Weirdly enough some additional impact of migrations_paths defaulting that surprised us:

If a non-primary database does not need migrations (and therefore theoretically need not have migrations_path config), then a bunch of db:* rake tasks start failing. To whit:

1. Rails defaults the secondary db to also use db/migrate by default
2. So then rails sees the primary's migrations as pending for the secondary db
3. Which triggers the "abort-if-pending-migrations" pre-req for many rake tasks.

This is especially odd when running rake tasks scoped to the primary db. (Which seems to indicate that the "abort-if-pending-migrations" check is not scoped to the db whose tasks are being requested.)

Example:

defaults:
  shared: &shared
    adapter: mysql2
    encoding: utf8mb4
    username: root
    password: pass
    host: 127.0.0.1
    port: 3306

  the_main: &the_main_defaults
    <<: *shared

  the_second: &the_second_defaults
    <<: *shared
    use_metadata_table: false

development:
  the_main:
    <<: *the_main_defaults
    database: main
  the_second:
    <<: *the_second_defaults
    database: second

rake db:reset:the_main some_other_task

drops the_main development db and then promptly fails/exits with: "You have N pending migrations:" (the_main's test db is not reset, nor is some_other_task run)

While it seems clear there are a handful of interrelated quirks with these db tasks; at least one prevention mechanism is for migrations_paths to be inferred just as the _schema.rb path is. (which would ensure pending migrations are not erroneously reported)

Some other strange behavior: setting migrations_paths: nil works around the issue for us; however, migrations_paths: false does not! Of course, the "clean" solution is to set path to a legitimate value, even if the directory doesn't exist or is empty.

[eileencodes]

If a non-primary database does not need migrations (and therefore theoretically need not have migrations_path config)

Will database_tasks: false not work for this? That will ignore tasks for the db you don't want migrations for.

It doesn't sound like the bugs you're discussing are directly related to this PR and unfortunately there are too many problems we hit to move forward at this time. If there are areas for improvement a new issue should be opened with a reproduction.