Improve db/migrations misconfigurations

[cllns]

Came to fix one thing and found a few more :)

1. Fix the docker-compose configuration: use correct port, provide the password, add a note to the readme about docker compose up (successor to docker-compose)

2. Re-word the warning when the user is missing config/db/. I found this from upgrading to 2.2.0.beta1 and the message I got was WARNING: Database ./db/bookshelf_development.sqlite has no config/db/ directory which doesn't make a ton of sense. Now it'll say WARNING: Database ./db/bookshelf_development.sqlite expects the folder config/db/ to exist but it does not.

3. Add a specific warning when the user is missing config/db/migrate/ (but has config/db)

4. Add a specific warning when the user has config/db/migrate/ but it's empty (they might have this folder but then have their migrations in a different folder, e.g. with the name 'migrations' which is incorrect)

I also added the line "No database migrations can be run for this database." to the last two, just to be really explicit. I'm not 100% sure it's true, though, if they have migrations in a slice instead of the main app. Happy to get rid of it, or re-word it so that it's always accurate.

After these changes, if they're missing config/db/ this is what they see when they run hanami db prepare

WARNING: Database ./db/bookshelf_development.sqlite expects the folder config/db/ to exist but it does not.

WARNING: Database ./db/bookshelf_development.sqlite expects migrations to be located within config/db/migrate/ but that folder does not exist.

No database migrations can be run for this database.
sh: ~/hanami/bookshelf/config/db/structure.sql: No such file or directory
!!! => "./db/bookshelf_development.sqlite structure dumped to config/db/structure.sql" FAILED

Which is pretty redundant but I'd rather give more info than too little. We can make it more concise later, and also skip attempting to dump the structure if we can know it'll fail beforehand.

[timriley]

Thanks @cllns! Appreciate you taking the opportunity to tighten up and reconsider things :)

So I have feedback about one of the two new warnings, but everything else aside from that looks good. So if you're up for my suggestion to consider that second warning separately, the rest of this PR looks good to merge :)

[timriley]

I'm been thinking about this case. An empty migrations folder IMO doesn't necessarily indicate something has gone wrong.

Such an empty folder can be expected:

• In brand new apps
• And in apps where a user has collapsed historical migrations by ensuring the schema dump is up to date and deleting the old migration files.

In these cases, nothing is really wrong (as opposed to the entire migrations dir being missing, which is wrong), it's just that we don't have any migrations to apply, and I think we could consider that routine.

So, a couple options here:

1. Remove this warning, or
2. Tone down the language so it's more an "informative" message than a "WARNING" message

What do you think?

Another thing I'd like to consider is being smarter with our output when there are migration files present but we're already up to date. Right now we just print the same output each time:

=> database db/my_app.sqlite migrated in 0.0002s

In this case, this is kind of a lie: the migrations are already up to date and we haven't actually "migrated" anything.

So perhaps we have the chance to unify the output across both the the "files absent" and "files present, but already up to date" cases.

If I could suggest something here, it'd be to remove this extra warning for now, and then give ourselves a ticket to consider making it nicer/more unified as a follow-up effort (and I'd probably mark it slightly lower priority than everything else we already have queued up before the 2.2 release).

[cllns]

Gotcha! Yea it's definitely not a problem to have an empty migrations folder, I mostly just wanted the output to reflect that, especially to help people who might have the migrations in the wrong place (we generate them correctly, but maybe someone will manually want to add migrations to a slice after the fact).

I softened the WARNING to a NOTE. We can clean that up once we come back to it, to fix the issue you raised.

Do you think a comment on #177 is an appropriate place for that better output, or a new issue?

[timriley]

@cllns Let's make a separate issue for this one. #177 is mostly about reviewing everything that already exists, rather than adding a specific new thing.

[timriley]

I created #225 for this :)

[timriley]

Thanks again for this one, @cllns. Merging this one now so that I can have a better docker-compose config for adding MySQL support into.