Skip to content

EntityManager connection setter#430

Closed
albertofem wants to merge 2 commits intodoctrine:masterfrom
albertofem:master
Closed

EntityManager connection setter#430
albertofem wants to merge 2 commits intodoctrine:masterfrom
albertofem:master

Conversation

@albertofem
Copy link

I found myself wanting to set the connection in runtime for an EntityManager. I had to use reflection to achieve that. I would be great to have this setter.

@travisbot
Copy link

This pull request passes (merged 11738d5 into ece6a00).

@stof
Copy link
Member

stof commented Aug 23, 2012

I don't think this is a good idea. Changing the connection in the middle of the logic is a very good way to break things and once you add a setter, you cannot ensure it is not done.

what was your use-case for setting the connection at runtime ?

@stof
Copy link
Member

stof commented Aug 23, 2012

Btw, some internal classes are keeping a reference to the connection (some persisters) or to the database platform (hydrators or the metadata factory) for performance reasons, meaning that your setter will break things by making Doctrine use 2 different connection at the same time depending of the part of the code needing it.

@albertofem
Copy link
Author

We are changing the connection at runtime in order to provide cross-database joins in our project (MySQL based). To do that without changing too much code, the best way is to loop the connections in the schema commands. You can see the implementation here: https://github.com/Arulu/DoctrineBundle/blob/master/Command/Proxy/UpdateSchemaDoctrineCommand.php. Note the use of reflection to change the property. If not a setter, moving the property from private to protected would do it, as we can easily extend the class and add the setter.

@beberlei
Copy link
Member

This can break the use of the EntityManager, so its a no go to add this.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants