Multiple entity managers support #38

Open
dinamic opened this Issue Jul 4, 2012 · 47 comments

Comments

Projects
None yet

dinamic commented Jul 4, 2012

Inside our ERP system we have calls to multiple database storages. To use them with Symfony2, we've defined them as different connections each one attached to a different entity manager.

If we make some change to the database structure there is no way to specify on which entity manager this change is going to be performed.

This can be controlled via the command line, but doing so is not reliable, unless we cherry-pick the doctrine migrations on deployment and specify different "--em" argument depending on the current migration.

@pvanliefland pvanliefland added a commit to pvanliefland/DoctrineMigrationsBundle that referenced this issue Jan 26, 2013

@pvanliefland pvanliefland Issue #38 Multiple entity managers support
Added the possibility to configure migrations depending on
the entity manager
76b6ce1

+1

zodeus commented Mar 1, 2013

+1

+1

Awesome!

Contributor

hason commented Apr 18, 2013

+1

aclarkd commented Jun 13, 2013

+1

dinamic commented Jun 14, 2013

Currently this can be achieved by using Container Aware Migrations. If one can have the service container injected, he can obtain an instance of some entity manager and its connection.

Here's a reference.

By the time of posting this issue there was no such functionality.
It is, however, considered as a workaround. It does not seem right to have the container on your disposal.

Member

odino commented Jul 8, 2013

+1

newneon commented Jul 30, 2013

Another workaround would be to implement the --configuration option properly. Currently the $this->configuration (AbstractCommand) gets overwritten with the default and therefore does not do what it was supposed to do: run migrations from a specific folder, using a specific migrations table. Changing the if statement in getMigrationConfiguration (if (! $this->configuration) to always pass (true || ! $this... ) allowed me to use a different folder, a different table and (in combination with --em) a different entity manager.

EDIT: just saw there is a fix... #18

@pvanliefland pvanliefland added a commit to pvanliefland/DoctrineMigrationsBundle that referenced this issue Sep 2, 2013

@pvanliefland pvanliefland Issue #38 Multiple entity managers support
Added the possibility to configure migrations depending on
the entity manager
e86c137

👍

But seriously: What happened to this ticket?

dinamic commented Jul 17, 2014

Two years later nothing has changed.

I am still going to recommend the Container Aware Migrations as a workaround.

Max101 commented Aug 6, 2014

👍 +1
Hope to see this someday...

For me, ContainerAwareInterface approach is completely sufficient and working.
I suggest closing, since 2 years passed with no PR.

odino referenced this issue Sep 1, 2014

Closed

Release of 1.0.0 #80

arnaugm commented Oct 28, 2014

@dinamic how do you use the new entity manager to execute the migrations? From the container you can get the proper $em but then, the addSql() calls continue using the other connection.

class Version20141028122535 extends AbstractMigration implements ContainerAwareInterface
{
    private $container;

    /**
     * @var EntityManager
     */
    private $em;

    public function setContainer(ContainerInterface $container = null)
    {
        $this->container = $container;
    }

    public function preUp(Schema $schema)
    {
        $this->em = $this->container->get('doctrine.orm.shared_entity_manager');
    }

    public function preDown(Schema $schema)
    {
        $this->em = $this->container->get('doctrine.orm.shared_entity_manager');
    }

    public function up(Schema $schema)
    {
        // this up() migration is auto-generated, please modify it to your needs
        $this->abortIf($this->em->getConnection()->getDatabasePlatform()->getName() != 'mysql', 'Migration can only be executed safely on \'mysql\'.');

        $this->addSql('CREATE TABLE new_table (id INT AUTO_INCREMENT NOT NULL, PRIMARY KEY(id)) DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci ENGINE = InnoDB');
    }

    public function down(Schema $schema)
    {
        // this down() migration is auto-generated, please modify it to your needs
        $this->abortIf($this->connection->getDatabasePlatform()->getName() != 'mysql', 'Migration can only be executed safely on \'mysql\'.');

        $this->addSql('DROP TABLE new_table');
    }
}

dinamic commented Nov 4, 2014

Yea, that was a problem I noticed when I tried it later on. I ended up getting the connection from the entity manager and executing the queries towards it. It's not ideal, but it works and we don't really have another way around.

raistlin commented Dec 5, 2014

ContainerAwareInterface has nothing to do with this issue.
With ContainerAwareInterface you can get any entity manager you want, but you don't know what entity manager was selected in command line.

dinamic commented Dec 5, 2014

That's correct. You don't know and you don't actually care what's selected on the command line.

The ContainerAwareInterface solves a problem here, it's not the cause of it.

We are still waiting for a proper fix to be applied for this problem. Until then, ContainerAwareInterface is the necessary evil.

ryall commented Jan 28, 2015

+1

+1

eggyal commented May 21, 2015

+1

@paulchubatyy paulchubatyy added a commit to paulchubatyy/DoctrineMigrationsBundle that referenced this issue Jul 4, 2015

@pvanliefland @paulchubatyy pvanliefland + paulchubatyy Issue #38 Multiple entity managers support
Added the possibility to configure migrations depending on
the entity manager
5173b0c

A few downsides I found to the proposed container workaround:

  • It's ugly as hell.
  • You get a console warning for not executing any SQL.
  • The queries get executed even if you do a dry-run.

spinx commented Aug 13, 2015

@stof
We have a fork where we added support for this. Kinda necessary with 16 EMs on the app.

Couple of things:

If we clean the code up a bit would you be in a position to merge a PR in on both this repo and doctrine/migrations ?

Member

stof commented Aug 17, 2015

@spinx does it really need a change in doctrine/migrations itself ? what kind of change ?

spinx commented Aug 17, 2015

@stof Mostly changes to commands. To allow diffing on all EMs, generating migrations for EMs. As I mentioned the only downside is that you need to track which EM the migration is for so you need to append EM name or similar to the migration version.

Member

mikeSimonson commented Aug 17, 2015

@spinx or you just use different configuration for different em and your problem is already solved.

spinx commented Aug 18, 2015

@mikeSimonson I'm not sure I understand what you mean by that. Can you elaborate ?

Member

mikeSimonson commented Aug 18, 2015

@spinx
For each migration command you can pass an entity manager and a migration configuration.
In the migration configuration you can set the migration folder and the sql table that will hold the version migrated.
Then you create an em configuration for all your different em.

The result will be that the migration on your different em are going to be independent of each other.

spinx commented Aug 18, 2015

Ah, I see. Might be a way to go, not sure.

@stof Is adding em to migration version acceptable ? Or maybe a column of 'manager' that defaults to 'default' instead, to have more backwards compatibility. People are probably relying on versions to be integers, right?

Member

mikeSimonson commented Aug 18, 2015

That would be a huge BC break and honestly you shouldn't use the em for migrations in the first place.
It's only going to bring you performance issues and troubles.

spinx commented Aug 18, 2015

Wouldn't use the EM but you somehow need to know against which EM you want to execute. Also if you are diffing to generate migrations files, the way it works now, having unix time as version, you can't really generate migrations for multiple EMs. Well you can do sleep(1) which is a bas solution as well.

Member

mikeSimonson commented Aug 18, 2015

You don't need to know against which em you are executing the migrations and there are no collision possible for the migrations version name as they are completely separate in different configurations / folders.

spinx commented Aug 18, 2015

ok, so just add support for multiple dirs here:
https://github.com/doctrine/DoctrineMigrationsBundle/blob/master/DependencyInjection/Configuration.php#L39

and update code to process that correctly ?

that might work, what I like about our solution though is that it doesn't need any configuration.

Member

mikeSimonson commented Aug 19, 2015

The difference between you solution and mine is that in yours you need to hardcode that multiple folder logic in the code and add a million of tests to account for all the weird edge cases that your are going to hit.
In mine it works now with the current code and you won't have any surprise because 2 migrations in 2 differents folders have the same name and collision in the migration_versions table.

dinamic commented Aug 19, 2015

IMO the most elegant solution would be to be able to provide migration configuration within the migration itself. This way you will be able to select entity manager or connection to execute the migration towards. Also we will have the flexibility of adding new features in the future without too much hassle.

Thoughts?

Member

stof commented Aug 19, 2015

@dinamic this does not make sense. the configuration is responsible for finding migrations, so the migration cannot be providing a configuration.

There should be strictly nothing to change in the library for that, as it is perfectly logical for a migration configuration to operate on a single connection.
Supporting multiple configuration because of a multi-em setup should be a job for the bundle only, and its proxied command settin gthe appropriate helper will be the place to switch between entity managers (as done in DoctrineBundle)

spinx commented Aug 19, 2015

ok, I'll play around a bit keeping in mind your feedback and try to find an elegant solution

Member

mikeSimonson commented Aug 19, 2015

@spinx I want to re add that using the em in the first place is a bad idea (unless you have to support multiple databases). You will have issues when you change an entity and you will have an super slow migration.

In general you should only use plain sql.

flohw commented Apr 6, 2016

Solution proposed by @mikeSimonson works very well. Maybe we can find something easier to use but in my opinion the feature is already here.

succinct commented May 9, 2016

I've run into the same problem. I did find that I can specify --db=<database_name> to run only the ones that are tied to that DB - but then it tries to run them against the default EM, and you can't specify both --em and --db at the same time.

angelsk commented Jun 27, 2016

Yup, DB option finally ran the correct migration - but into the wrong DB :(

So, is there working solution?

andrzejdziekonski commented Sep 21, 2016 edited

My workaround was:

  • add prefix to the 2nd connection tables
  • in the 1st connection configuration adding schema_filter exluding above prefix
  • in the migration class where i modify prefix tables i add skipIf condition on the database name to be sure that the migration goes for correct db (you can use ContainerAwareInterface to grab dbname from parameters)
  • then i execute up for above migration with --em parameter

Result is that i can use two em's and migrating databases with no issues. As the 2nd db won't change ofter (if ever) i am kinda satisfied with that ignore filter solution.

timiTao commented Oct 19, 2016

👎 after few years, still no good official solution?

There are some really glaring omissions with Symfony & Doctrine and this is one of them. Unbelievable that this hasn't been addressed by now, over 4 years since it was first logged.

We use multiple entity managers to talk to different databases, without this we basically can't use migrations without hacking the source.

spinx commented Nov 8, 2016

We have our own wrapper commands that do diffs and migrations across 25 entity managers, and it works great. Our solution generates migration files in separate directories, which is fine for us, because each bundle has its own migrations dir. I agree this is not a good approach for everyone.

I'm still willing to work on this but we need to agree on an approach.

Ability to use multiple databases seems so basic to me, even small projects have more than one database in our case. Why is this not wanted @stof @mikeSimonson ?

@Dens49 Dens49 added a commit to Dens49/DoctrineMigrationsBundle that referenced this issue Jan 20, 2017

@pvanliefland @Dens49 pvanliefland + Dens49 Tried to merge pvanliefland's fork into the current version
Issue #38 Multiple entity managers support

Added the possibility to configure migrations depending on
the entity manager

Conflicts:
	Command/DoctrineCommand.php
	Command/MigrationsExecuteDoctrineCommand.php
	Command/MigrationsGenerateDoctrineCommand.php
	Command/MigrationsStatusDoctrineCommand.php
5678b24

Wirone commented Feb 6, 2017

I, too, can't believe this is not resolved. Like Doctrine's ORM configuration can have multiple entity managers set and each manager can have its own database connection, DoctrineMigrationBundle's config should support multiple entity managers in order to migrate different databases. This is so natural and basic for me - it's unbelievable that it's not possible.

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