Skip to content

Conversation

@fureszpeter
Copy link
Contributor

Changes proposed in this pull request:

Important Notes

Since Laravel support only one slave (read-only) connection, not completely compatible with Laravel database.php

Differences:

_Doctrine version:_

            'read' => [
                [ //<==Please note, we need to add multiple arrays here!
                    'host' => env('DB_HOST_R', 'localhost'),
                    'database' => env('DB_DATABASE_R', ''),
                    'username' => env('DB_USERNAME_R', ''),
                    'password' => env('DB_PASSWORD_R', ''),
                ],
            ],

_Laravel version:_

            'read' => [
                    'host' => env('DB_HOST_R', 'localhost'),
                    'database' => env('DB_DATABASE_R', ''),
                    'username' => env('DB_USERNAME_R', ''),
                    'password' => env('DB_PASSWORD_R', ''),
            ],

@patrickbrouwers
Copy link
Contributor

Can you retarget your work to 1.2? Note that 1.2 shares PDO connections with Laravel. I don't know how much that will change for your work?


use UnexpectedValueException;

class MasterSlaveConfigParser
Copy link
Contributor

@guiwoda guiwoda Sep 9, 2016

Choose a reason for hiding this comment

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

@fureszpeter I don't think this should be extracted to a whole class.
This could live inside the EntityManagerFactory, as no other code will need to call it. Plus, a big, big, BIG 👎 to utility (static) classes.

@patrickbrouwers
Copy link
Contributor

Replaced by #214

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.

3 participants