Skip to content
This repository has been archived by the owner on Nov 11, 2020. It is now read-only.

Convert timeout options for driver >= 1.5.0 #175

Closed
wants to merge 1 commit into from

Conversation

rjelierse
Copy link
Contributor

This transforms the deprecated timeout and wtimeout options to their respective counterparts (socketTimeoutMS and wTimeoutMS) whenever this is applicable.

This change is necessary because as of version 1.5.0 of the mongo driver, E_DEPRECATED errors are triggered when the old options are given.


services: mongodb

before_script:
- pecl -q install -f mongo-${MONGO_VERSION} && echo "extension=mongo.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"`
- printf "\n" | pecl -q install -f mongo-${MONGO_VERSION} && echo "extension=mongo.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"`
Copy link
Member

Choose a reason for hiding this comment

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

Likewise, please amend the commit to remove this change. I'll implement this separately in a fashion similar to what I did for ODM: doctrine/mongodb-odm@4fe7d41

Copy link
Member

Choose a reason for hiding this comment

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

Taking care of this and the driver version addition in #176.

@jmikola
Copy link
Member

jmikola commented Apr 18, 2014

This PR made me aware that we were missing some driver documentation. That should now be fixed with PHP-1076.

@rjelierse
Copy link
Contributor Author

Fixed your suggestions, everything alright now?

@@ -281,6 +281,7 @@ public function drop()
public function ensureIndex(array $keys, array $options = array())
{
$options = isset($options['safe']) ? $this->convertWriteConcern($options) : $options;
$options = isset($options['timeout']) ? $this->convertSocketTimeout($options) : $options;
Copy link
Member

Choose a reason for hiding this comment

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

This actually needs wtimeout handling as well. On MongoDB 2.4, ensureIndex() will perform and insert operation on system.indexes. In 2.6+, the createIndexes command is used. I've updated the driver documentation to clarify this in http://svn.php.net/viewvc?view=revision&revision=333382

@jmikola
Copy link
Member

jmikola commented Apr 21, 2014

LGTM once wtimeout is added to ensureIndex().

Is it possible to add a couple of tests modeled after testWriteConcernOptionIsConverted and testWriteConcernOptionIsNotConvertedForOlderDrivers in Collection test? You can use ensureIndex as the guinea pig.

@jmikola jmikola added this to the 1.1.6 milestone Apr 21, 2014
@jmikola jmikola added task and removed task labels Apr 21, 2014
@rjelierse
Copy link
Contributor Author

Added wtimteout to ensureIndex(). Also wrote tests for both options, but chose to test against insert(), since ensureIndex() is also marked as deprecated for 1.5.0

This transforms the deprecated "timeout" and "wtimeout" options to their
respective counterparts ("socketTimeoutMS" and "wTimeoutMS") whenever this
is applicable.
jmikola added a commit that referenced this pull request Apr 22, 2014
@jmikola
Copy link
Member

jmikola commented Apr 22, 2014

Manually merged in 9550db5. I also tacked on an extra commit to make the methods consistent with the existing convertWriteConcern() method in 0e30158. I thought I had left that as a todo comment in the PR, but it looks like I missed it.

Thanks for the work on this!

@jmikola jmikola closed this Apr 22, 2014
@jmikola jmikola changed the title Convert options for PHP Mongo driver >= 1.5.0 Convert timeout options for PHP Mongo driver >= 1.5.0 Apr 29, 2014
@jmikola jmikola changed the title Convert timeout options for PHP Mongo driver >= 1.5.0 Convert timeout options for driver >= 1.5.0 Apr 29, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants