search

davedevelopment / phpmig

Simple migrations system for php
Other
568 stars 92 forks source link

Method of working out which migrations to run does not support team working / typical real life environments, needs to inspect previously ran migrations #148

Open PaulMcRoar opened 4 years ago

PaulMcRoar commented 4 years ago

The method of working out which migrations to run, going off the highest timestamp in the database is too basic for real development scenarios.

Typically you will have different developers working on feature branches, and adding migrations as they go along with their development. Whilst a development branch is worked on, other branches may be merged and released. This means that when a branch is merged in it will likely have migrations which have an earlier timestamp than those that have already been ran, but those migrations still need to run to support that feature.

The method of working out which migrations to run should inspect the files in the migrations directory(s) and check them against the ran migrations to see which still need to run. It should not go off a latest timestamp / version alone.

As it is, with this system, we need to rename all our migrations post merge and before rollout to give them a later timestamp / version that what is current. When you could possibly have dozens of migration files, this is onerous.

davedevelopment commented 4 years ago

Hi Paul

The method of working out which migrations to run, going off the highest timestamp in the database is too basic for real development scenarios.

It's been working for real development scenarios for 9 years.

As it is, with this system, we need to rename all our migrations post merge and before rollout to give them a later timestamp / version that what is current. When you could possibly have dozens of migration files, this is onerous.

You should not have to do that, phpmig migrate should run all migrations that have not been run as of yet, in the order of the timestamps. If it's not doing that, there is a bug somewhere.

Edit: should not...

PaulMcRoar commented 4 years ago

the 'getMigrations' method in Phpmig/Api/PhpmigApplication.php takes a 'from', and an optional 'to', and then checks each migration's timestamp to see if it is greater than the 'from' and then adds that to the migrations to run list. It doesn't check against previously ran migrations to see if they have ran. 'getMigrations' is called in 'up' with the from value of 'getVersion' which is just the last ran migration.

( Slightly worried that fixing this would start running migrations on people's platforms that hadn't yet ran, so would possibly need some kind of configuration option to change the behaviour. )

davedevelopment commented 4 years ago

Ah, my apologies, I think you'd do best to ignore that whole namespace. It was added in #65 as someone wanted to run some migrations from their UI, but it wasn't complete and by the looks of things, not all that sensible.

This is the main "migrate" method, which will run all migrations that haven't been run before https://github.com/davedevelopment/phpmig/blob/b85793c49b2a5fef381ae336f5defd027ea42b62/src/Phpmig/Console/Command/MigrateCommand.php#L52

PaulMcRoar commented 4 years ago

The application I've inherited has about 100 different client databases that need to be kept in sync, and so we've written our own wrapper which spins through them and uses the API.

Would you be happy for me to submit a patch so that the API behaviour mirrors that of the Command line?

davedevelopment commented 4 years ago

Of course :+1:, refactor, extract, rewrite at will.