-
-
Notifications
You must be signed in to change notification settings - Fork 455
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
Fix #1230 - Console commands do not respect driverOptions when master/slave configuration is used #1240
Conversation
…/slave configuration is used
@@ -126,6 +126,7 @@ public function testDbalLoadSingleMasterSlaveConnection() | |||
'dbname' => 'mysql_db', | |||
'host' => 'localhost', | |||
'unix_socket' => '/path/to/mysqld.sock', | |||
'driverOptions' => [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the file on line 114 be modified to add options in both the master and the slave, to make things more interesting? Without your change, what happens? Are the master's options copied to the slave?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is actually no such option as "master" configuration key/option. What's defined one level above slaves is what's master. And yes that's how this patch works. It copies everything from master to slaves except the things in the list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, I read the issue backwards. What if there is already an option at the slave level? Will it be overwritten? Should the configuration at
Lines 2 to 15 in 6f3f4d9
dbal: | |
dbname: mysql_db | |
user: mysql_user | |
password: mysql_s3cr3t | |
unix_socket: /path/to/mysqld.sock | |
keep_slave: true | |
default_table_options: | |
engine: InnoDB | |
slaves: | |
slave1: | |
user: slave_user | |
dbname: slave_db | |
password: slave_s3cr3t | |
unix_socket: /path/to/mysqld_slave.sock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that's necessary. I didn't change merging logic. Whatever slave specifies is kept for that slave.
…verOptions Reverts the changes made in doctrine#1240, and applies the passing down of `driverOptions` manually in `CreateDatabaseDoctrineCommand` and `DropDatabaseDoctrineCommand` Resolves doctrine#1252
@stof could you please check this, you were the one adding driverOptions to this blacklist, so there is a tiny chance you would know if that was on purpose