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.

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

Different mongo driver versions in mquery vs mongoose #25

Closed
ashtuchkin opened this issue Jan 15, 2014 · 3 comments
Closed

Different mongo driver versions in mquery vs mongoose #25

ashtuchkin opened this issue Jan 15, 2014 · 3 comments

Comments

@ashtuchkin
Copy link
Contributor

As of now, we have the following versions:

mongoose@3.8.4 node_modules/mongoose
...
├── mongodb@1.3.23 (kerberos@0.0.3, bson@0.2.5)
└── mquery@0.4.1 (debug@0.7.0, mongodb@1.3.19)

As you can see, mongoose@3.8.4 requires mongodb@1.3.23, and mquery@0.4.1 which itself requires mongodb@1.3.19. Two versions of mongodb driver are loaded, which is +150ms to startup time and a source of magic incompatibility errors.

Keeping versions in sync would help, but is prone to forgetting, etc. Maybe it's better to provide external mongodb driver reference to the mquery before use, i.e.:

mongodb = require('mongodb')
mquery = require('mquery')(mongodb);
@aheckmann
Copy link
Collaborator

yeah I think there's another issue open related to this. might be nice to support passing in an ObjectId constructor and optionally a ReadPref constructor. then we don't have any direct dependency

@aheckmann
Copy link
Collaborator

pull request?

ashtuchkin added a commit to ashtuchkin/mquery that referenced this issue Jan 16, 2014
@ashtuchkin
Remove mongodb driver dependency.
595377e 
See more discussion in mongoosejs#25.

mongodb.ObjectId class was needed to clone existing ObjectId. Now we just use
obj.constructor of the old ObjectId.

mongodb.ReadPreference is more difficult. It's used to create a readPreference
option in queries.

Our options are:
 1. We could try to keep the same semantics and save this class on mquery
    initialization, something like `mquery = require('mquery')(mongodb)`.
    This is deemed too serious change for such a small cause.
 2. We could try to get it from a collection, but 1) it seems that this class
    is not exported by collection and 2) this would violate the collection
    abstraction infrastructure of mquery (lib/collection folder).
 3. As mongo driver accepts both string representation of read preference and
    an instance of ReadPreference class, we could work with just strings. This
    way, user can keep using short string representation when the tags are not
    needed (in majority of cases), and provide an instance of ReadPreference
    class when tags are required. It's also future-proof as mongo driver can
    add other options to ReadPreference class and users will be able to use
    them right away. This last option is implemented.
@ashtuchkin ashtuchkin mentioned this issue Jan 16, 2014
Merged
ashtuchkin added a commit to ashtuchkin/mquery that referenced this issue Jan 16, 2014
@ashtuchkin
Remove mongodb driver dependency.
2a9108d 
See more discussion in mongoosejs#25.

mongodb.ObjectId class was needed to clone existing ObjectId. Now we just use
obj.constructor of the old ObjectId.

mongodb.ReadPreference is more difficult. It's used to create a readPreference
option in queries.

Our options are:
 1. We could try to keep the same semantics and save this class on mquery
    initialization, something like `mquery = require('mquery')(mongodb)`.
    This is deemed too serious change for such a small cause.
 2. We could try to get it from a collection, but 1) it seems that this class
    is not exported by collection and 2) this would violate the collection
    abstraction infrastructure of mquery (lib/collection folder).
 3. As mongo driver accepts both string representation of read preference and
    an instance of ReadPreference class, we could work with just strings. This
    way, user can keep using short string representation when the tags are not
    needed (in majority of cases), and provide an instance of ReadPreference
    class when tags are required. It's also future-proof as mongo driver can
    add other options to ReadPreference class and users will be able to use
    them right away. This last option is implemented.
ashtuchkin added a commit to ashtuchkin/mquery that referenced this issue Jan 16, 2014
@ashtuchkin
Remove mongodb driver dependency.
aa3c17f 
See more discussion in mongoosejs#25.

mongodb.ObjectId class was needed to clone existing ObjectId. Now we just use
obj.constructor of the old ObjectId.

mongodb.ReadPreference is more difficult. It's used to create a readPreference
option in queries.

Our options are:
 1. We could try to keep the same semantics and save this class on mquery
    initialization, something like `mquery = require('mquery')(mongodb)`.
    This is deemed too serious change for such a small cause.
 2. We could try to get it from a collection, but 1) it seems that this class
    is not exported by collection and 2) this would violate the collection
    abstraction infrastructure of mquery (lib/collection folder).
 3. As mongo driver accepts both string representation of read preference and
    an instance of ReadPreference class, we could work with just strings. This
    way, user can keep using short string representation when the tags are not
    needed (in majority of cases), and provide an instance of ReadPreference
    class when tags are required. It's also future-proof as mongo driver can
    add other options to ReadPreference class and users will be able to use
    them right away. This last option is implemented.
@ashtuchkin
Copy link
Contributor Author

Fixed in #26. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@aheckmann @ashtuchkin