Skip to content

Conversation

cray0000
Copy link

The latest version of mongodb throws error on unknown options. In a simple ShareDB usecase (when a separate mongoPoll database is not used) mongo gets its options from the mutual pool of options for mongo and for ShareDB. This leads to errors like allowAllQueries options is not supported in mongodb.

This pull request fixes this issue by adding sanitization of options (removes all sharedb-specific options) before passing them to mongo.

Suggestion: Passing mongo options implicitly should be deprecated. We should always get mongo options from mongoOptions field.

@coveralls
Copy link

coveralls commented Feb 13, 2017

Coverage Status

Coverage increased (+0.1%) to 93.01% when pulling 3a05d8c on dmapper:sanitize-mongo-options into 260a6c7 on share:master.

// TODO: Deprecate the ability to pass mongo options implicitly
var mongoOptions;
if (options.mongoOptions) {
mongoOptions = options.mongoOptions;
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this change of being explicit about which options to pass to mongo in a separate object, but let's not do the second part where we try and make the old options style backward compatible. I think it is reasonable to expect people to update their connection instantiation code as they switch drivers.

var mongoOptions;
if (options.mongoOptions) {
mongoOptions = options.mongoOptions;
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd remove this part where we delete all the sharedb-mongo options and instead just require people to update their options if they want to use the new driver

@nateps
Copy link
Contributor

nateps commented Nov 13, 2019

Thanks for the help! I'm closing this, since it is being addressed slightly differently with #83 instead

@nateps nateps closed this Nov 13, 2019
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.

3 participants