retryConnect & retryQuery values configurable rather than hard coded. #117

Closed
wants to merge 3 commits into from

2 participants

@butter5

We needed the retry connect and retry query options configurable rather than hard coded, so I've modified the bundle to allow retry connect and retry query to be configured per connection.

Example config:

doctrine_mongodb:
connections:
default:
server: mongodb://localhost:27017
options:
connect: true
retry_connect: 3
retry_query: 3
default_database: test_database
document_managers:
default:
auto_mapping: true

Phil Butterw... added some commits Jul 6, 2012
Phil Butterworth Allowing the retry connect and retry queries options to be configured…
… on a connection by connection basis using the config files, rather than hardcoding them.
e0e8d88
Phil Butterworth Fixed a few issues with getting this to work properly. a195509
@jmikola jmikola and 1 other commented on an outdated diff Jul 9, 2012
Resources/config/mongodb.xml
@@ -59,7 +59,7 @@
<services>
<!-- defaults -->
<service id="doctrine.odm.mongodb.cache" alias="doctrine.odm.mongodb.cache.array" />
-
+ <service id="doctrine.odm.mongodb.document_manager" class="%doctrine.odm.mongodb.document_manager.class%"/>
@jmikola
Doctrine member
jmikola added a note Jul 9, 2012

This shouldn't be here. The extension class already aliases the default DM to this service ID near the end of loadDocumentManager().

@butter5
butter5 added a note Jul 11, 2012

Sure. I had to add this because I was getting a few issues while trying to get the bundle working. Will be removed on next push.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jmikola jmikola commented on an outdated diff Jul 9, 2012
DependencyInjection/Configuration.php
@@ -141,6 +141,8 @@ private function addConnectionsSection(ArrayNodeDefinition $rootNode)
->scalarNode('password')->end()
->end()
->end()
+ ->scalarNode('retry_connect')->end()
+ ->scalarNode('retry_query')->end()
@jmikola
Doctrine member
jmikola added a note Jul 9, 2012

These should have default values, so we don't have to do isset() checks later in the extension class.

->scalarNode('retry_connect')->defaultValue(0)->end()
->scalarNode('retry_query')->defaultValue(0)->end()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jmikola jmikola commented on an outdated diff Jul 9, 2012
DependencyInjection/DoctrineMongoDBExtension.php
{
$defaultDatabase = isset($documentManager['database']) ? $documentManager['database'] : $defaultDB;
$configServiceName = sprintf('doctrine.odm.mongodb.%s_configuration', $documentManager['name']);
+ $retryConnect = isset($config['retry_connect']) ? $config['retry_connect'] : 0;
+ $retryQuery = isset($config['retry_query']) ? $config['retry_query'] : 0;
@jmikola
Doctrine member
jmikola added a note Jul 9, 2012

See my above comments about specifying default values in the Configuration class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jmikola jmikola commented on an outdated diff Jul 9, 2012
DependencyInjection/DoctrineMongoDBExtension.php
@@ -160,6 +164,8 @@ protected function loadDocumentManager(array $documentManager, $defaultDM, $defa
'setHydratorNamespace' => '%doctrine.odm.mongodb.hydrator_namespace%',
'setAutoGenerateHydratorClasses' => '%doctrine.odm.mongodb.auto_generate_hydrator_classes%',
'setDefaultDB' => $defaultDatabase,
+ 'setRetryConnect' => $retryConnect,
+ 'setRetryQuery' => $retryQuery
@jmikola
Doctrine member
jmikola added a note Jul 9, 2012

If these two options only end up being set on the ODM's Configuration object, why bother specifying them on the connection block of the config structure? We can simply define them within Configuration::addDocumentManagersSection(), no?

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

Jeremy,

I've reworked based on your comments. I would appreciate any further feedback.

Thanks
Phil.

@jmikola
Doctrine member

I squashed the PR as 0a69f0b and merged it into 2.0 and master. Thanks!

@jmikola jmikola closed this Jul 17, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment