Skip to content
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

Add Migrations middleware #774

Open
wants to merge 5 commits into
base: 4.x
Choose a base branch
from
Open

Add Migrations middleware #774

wants to merge 5 commits into from

Conversation

dereuromark
Copy link
Member

@dereuromark dereuromark commented Nov 26, 2024

Resolves #317

This does require a bit more manual work.
Also, this wouldn't yet allow including plugin migrations

My idea here would be to look for all *_phinxlog tables in the DB and reverse engineer the plugins from them
Alternatively, we could provide a config here to inject: 'plugins' => ...

Then we could also iterate over them in the middleware.

Furthermore, we could check on a query string skip-migration-check = 1 and if so, also behave like non debug mode.
This can be useful if you need to check something prior to actually running it locally.

@dereuromark
Copy link
Member Author

dereuromark commented Nov 29, 2024

pending_migrations

Open tasks

  • Make connection table more dynamic (and get default one from connection manager)
  • Make phinxlog table more dynamic
  • tests
  • ... ?

@dereuromark dereuromark changed the title Add Migrations middleware WIP: Add Migrations middleware Nov 29, 2024
@LordSimal
Copy link
Contributor

I can think of plugins which do contain migrations which I don't want/need to run in my app because I just use parts of that plugin (e.g. this one).
Therefore it would be good to ignore either a whole plugin from this logic or specific migrations.

docs/en/index.rst Outdated Show resolved Hide resolved
docs/en/index.rst Outdated Show resolved Hide resolved
docs/en/index.rst Outdated Show resolved Hide resolved
src/Middleware/PendingMigrationsMiddleware.php Outdated Show resolved Hide resolved
@dereuromark
Copy link
Member Author

Therefore it would be good to ignore either a whole plugin from this logic or specific migrations.

The plugins list is already opt-in by default, so if you dont specify a plugin it would not run the check on it.
If you skip specific migrations, that is out of scope here.

@dereuromark
Copy link
Member Author

dereuromark commented Dec 2, 2024

It works now for app and plugins, you can provide a list of plugins, otherwise only app.
You can provide a different connection string, otherwise defaults to default.

Anything else? Or does this only need tests etc to get finished up?

Also removed some dead dormant code that wasnt been used for a while.

@dereuromark dereuromark changed the title WIP: Add Migrations middleware Add Migrations middleware Dec 2, 2024
@dereuromark
Copy link
Member Author

I might need a bit of help with the tests, also because they seem to leak to stdout

@dereuromark
Copy link
Member Author

dereuromark commented Dec 2, 2024

Ideas that could also be done as part of a follow up PR

  • 'app' false/true to only allow plugins to be checked (e.g. if you are working alone, and only want to check vendor plugins)
  • 'cache' false/true to allow caching based on the hashsum of all migration files (app+plugin), this would then safe a bit of queries maybe? on the other side this needs still the file lookups.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Raise exception when there are migrations yet to be run
3 participants