-
-
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
Add --shard option to doctrine commands #456
Conversation
} | ||
foreach ($options['shards'] as $i => $shard) { | ||
if (!isset($shard['driver'])) { | ||
$options['shards'][$i]['driver'] = $options['driver']; |
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.
why is that ? the PoolingShardConnection does not need to specify the driver for each shard (it does not make any sense to have a different driver per shard)
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.
Cause it will throw an exception to require driver
or driverOptions
in shard connection if this parameter is missing
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.
where would it throw an exception ?
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.
@stof In DriverManager _checkParams method
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.
For example when executing a doctrine:database:create
command
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.
well, then doctrine:database:create
should be fixed instead when it builds params in a special way instead of using the configured params.
Adding useless params is the wrong way (the DriverManager will never read $params['shard'][$i]['driver']
but always $params['driver']
)
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.
👍
--shard
option to doctrine commands--shard
option to doctrine commands
--shard
option to doctrine commands
|
||
if ($shardId) { | ||
if (!$manager->getConnection() instanceof PoolingShardConnection) { | ||
throw new \LogicException(sprintf("Connection of EntityManager '%s' must implements shards configuration.", $name)); |
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.
"must implement"
👍 |
1 similar comment
👍 |
@deeky666 Can you also have a look at it please ? :) |
👍 In another PR it would be nice to refactor the create and the drop commands: they share a lot of common code and code duplication is bad :-) |
Thanks @vincentchalamon |
I need to use shard connections in my project, but it seems that some code is not up to date with sharding. This PR add some compatibility & options with shard connections.
--shard
option todoctrine:database
commandsdoctrine.<connection name>_shard_manager
using PoolingShardManager--shard
option todoctrine:migrations
commands => Add --shard option to migrations command DoctrineMigrationsBundle#132--shard
option todoctrine:fixtures
commands => Add --shard option to doctrine command DoctrineFixturesBundle#145