Skip to content
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

Bump mongodb driver to 2.2.24 #8453

Merged
merged 3 commits into from Mar 9, 2017
Merged

Conversation

laosb
Copy link
Contributor

@laosb laosb commented Mar 4, 2017

Fixes #8449.

@benjamn benjamn force-pushed the laosb-bump-mongodb-driver-2-2-24 branch from 6fc6e00 to 6f864c8 Compare March 8, 2017 17:38
@benjamn benjamn added this to the Release 1.4.3.x milestone Mar 8, 2017
Warning text: "the server/replset/mongos options are deprecated, all their
options are supported at the top level of the options object"
@benjamn benjamn force-pushed the laosb-bump-mongodb-driver-2-2-24 branch from 6f864c8 to ac8fa03 Compare March 8, 2017 19:02
@benjamn benjamn self-assigned this Mar 8, 2017
@benjamn benjamn requested a review from abernix March 8, 2017 20:01
@@ -21,7 +21,7 @@ Npm.strip({
});

Package.onUse(function (api) {
api.use('npm-mongo', 'server');
api.use('npm-mongo@2.2.24', 'server');
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems important because the new options format won't work with older versions of npm-mongo.

Copy link
Contributor

@abernix abernix left a comment

Choose a reason for hiding this comment

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

LGTM, with one "FYI" blurb I've included for posterity's sake.

@@ -128,17 +128,12 @@ MongoConnection = function (url, options) {
self._observeMultiplexers = {};
self._onFailoverHook = new Hook;

var mongoOptions = _.extend({
db: { safe: true },
Copy link
Contributor

Choose a reason for hiding this comment

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

Took some work to track this down but I believe the omission of db: { safe: true } here is, well, safe.

safe was originally added to Meteor in eee0f74 which hints that it was a supported option by mongodb@1.1.11 (the version in use at the same time).

The Node Driver HISTORY.md doesn't really mention that safe was ever removed. Suffice to say, it hasn't existed in the MongoClient Db documentation in a long, long time (see 1.4 version of the docs).

It is not listed in the current version of the docs but it does seem to be somewhat-expected in the code for URI connection strings, but not documented and the option should never make it to a server right now as it's being parsed in all three Mongo Server, Mongos and ReplSet.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be replaced with w and j, no? Allowing one to set this could address: meteor/meteor-feature-requests#17

@abernix abernix merged commit b92c88f into devel Mar 9, 2017
@abernix abernix deleted the laosb-bump-mongodb-driver-2-2-24 branch March 9, 2017 14:45
@abernix abernix modified the milestones: Release 1.4.3.x, Release 1.4.3.2 Mar 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants