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

Change adapter in migration #1064

Conversation

t-moennich
Copy link

This PR implements a new setAdapterByAlias($alias) method in the AbstractMigration class. This allows to change the used adapter during the migration process.

To keep the adapter selection handy, I've also created a configuration extensions to provide a list of databases which may be choosen by an alias.

Specific test for the new configuration section and setAdapterByAlias method will be pushed asap.

@t-moennich
Copy link
Author

Would be great to know if this PR has a future (as we need this in our company).
If so, I'll push detailed unit tests of course.

*/
public function getAvailableAdapter($name)
{
$adapters = $this->getAvailableAdapters();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would the list of adapters change during the lifetime of running phinx? If not, should $adapters be cached?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory no, as they are persistent per configuration. So yes, caching seems to be an good idea!

@@ -675,7 +675,7 @@ public function getMigrations()
}

// instantiate it
$migration = new $class($version, $this->getInput(), $this->getOutput());
$migration = new $class($this, $version, $this->getInput(), $this->getOutput());
Copy link
Collaborator

@rquadling rquadling Mar 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OOI, as $this is being supplied, why bother supplying the input and output when they can be extracted from $manager within AbstractMigration::__construct(Manager $manager, $version)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, will be changed

@rquadling
Copy link
Collaborator

What is the use case you have for changing the adapter from the configuration? (Purely for interest).

@t-moennich
Copy link
Author

@rquadling some of our migrations in a single project have to access multiple databases.
Therefore we have to change the adapter.

We tried also to change the name option in the PdoAdapter options but this would into uncontrollable migrations which could write into everything.
So we decided to provide a list of database connections and allow to refer on them by the alias.

@rquadling
Copy link
Collaborator

Some thoughts.

Is this really choosing an environment (from Phinx's terminology)?

'Adapter' is the term related to the database driver layer, not a specific connection or credential set.

Whilst commonly, people have 'development', 'staging'/'test', 'production', each of those 'environments' can have a different adapter and credential set.

For you, I guess you have something like 'prod_1', 'prod_2', 'prod_3' and you are wanting to run the migrations against these environments (either all against all, or some against some).

Where does the phinxlog table exist? I would again guess that you have a central server (one you consider primary in some regard) and that's the default environment?

What I'm concerned with is calling this 'adapter' when that has no direct bearing on credentials or specific server. Just which driver to use.

Given that this looks to be an environment switch, you'd have to guarantee that the environment is switched back to the default one in case someone doesn't set it in the migration. So cloning the manager for a migration rather than passing it as a reference. This would mean each migration gets a clean copy of the manager to do what it wants with, but it isn't persisted to the next migration.

@t-moennich
Copy link
Author

Is this really choosing an environment (from Phinx's terminology)?

Nope it isn't. It's realy the change of the database connection/the adapter.
We have some old projects there tables are logically split among different databases.
Therefore we had to switch them either in a second migration or also in the same migration multiple times.

Where does the phinxlog table exist? I would again guess that you have a central server (one you consider primary in some regard) and that's the default environment?

The phinxlog is stored at the environment 'adapter' connection, which is given in the environment configuration itself.
But this is - our advantage - stored in Manager\Environment and dosen't change while changing the migration 'adapter'. :)

'Adapter' is the term related to the database driver layer, not a specific connection or credential set.
What I'm concerned with is calling this 'adapter' when that has no direct bearing on credentials or specific server. Just which driver to use.

I'm with you! I've also thought about naming the required method setConnectionByDatabaseAlias rather than setAdapterByAlias but as the AbstractMigration already uses the setAdapter term I just adopted that.

But in the end WE just switch the database name. A full switch of the adapter brings more features but on the other side, another adapter for the same environment shouldn't be necessary at all..

@dereuromark
Copy link
Member

This should be re-done on current master - otherwise closed as outdated.

@dereuromark
Copy link
Member

Requires a new PR

refs

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.

None yet

3 participants