Skip to content

PHPLIB-254 Support maxTimeMS option for CreateIndexes operation #431

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Nov 14, 2017
Merged

PHPLIB-254 Support maxTimeMS option for CreateIndexes operation #431

merged 4 commits into from
Nov 14, 2017

Conversation

kvwalker
Copy link
Contributor

@kvwalker kvwalker requested a review from jmikola November 10, 2017 18:10
derickr
derickr reviewed Nov 13, 2017
View reviewed changes
src/Collection.php
$indexOptions = array_diff_key($options, ['writeConcern' => 1]);
$commandOptions = array_intersect_key($options, ['writeConcern' => 1]);
$indexOptions = array_diff_key($options, ['maxTimeMS' => 1, 'writeConcern' => 1]);
$commandOptions = array_intersect_key($options, ['maxTimeMS' => 1, 'writeConcern' => 1]);
Copy link
Contributor

Choose a reason for hiding this comment

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

In #432 you've also added a test case to test for invalid values for the new option. Can you add one for this new option to provideInvalidConstructorOptions in CreateIndexesTest.php too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks!

@jmikola
Copy link
Member

jmikola commented Nov 14, 2017 •
edited
Loading

Code changes LGTM.

Can you update docs in the RST manual and PHP doc blocks for CreateIndexes operation class when you get a chance? Same as was requested in #432.

For the RST manual, maxTimeMS is already defined in apiargs-common-option.yaml so you should just need to add the following to apiargs-MongoDBCollection-method-createIndex-option.yaml and apiargs-MongoDBCollection-method-createIndexes-option.yaml.

source:
  file: apiargs-common-option.yaml
  ref: maxTimeMS

Note that Collection::createIndexes() didn't need any code changes here as it already takes command options separately from any index options (no splitting is necessary).

jmikola
jmikola reviewed Nov 14, 2017
View reviewed changes
Copy link
Member

@jmikola jmikola left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Good to merge!

@kvwalker kvwalker merged commit 1af82a4 into mongodb:master Nov 14, 2017
@kvwalker kvwalker deleted the PHPLIB-254 branch November 14, 2017 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derickr derickr derickr left review comments

@jmikola jmikola jmikola left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kvwalker @jmikola @derickr