Add MongoCollection wrapper and move few basic uses of collection to it. #752
Conversation
| //TODO: condiser moving index creation logic into Schema.js | ||
| return this._mongoCollection.createIndex(index) | ||
| // Retry, but just once. | ||
| .then(() => this.find(query, { skip, limit, sort })); |
There was a problem hiding this comment.
Unlike the old implementation, this doesn't seem to have any logic to avoid retrying infinitely if you get the same error over and over. I don't see how that could happen if the index build succeeded, but maybe it's worth it to keep that logic around anyway?
There was a problem hiding this comment.
Good call actually, let me change this a tiny bit then to accommodate for this.
nlutsenko
force-pushed
the
nlutsenko.adapter.collection
branch
from
a622a18 to
9538a7d
Compare
| //TODO: condiser moving index creation logic into Schema.js | ||
| return this._mongoCollection.createIndex(index) | ||
| // Retry, but just once. | ||
| .then(() => this._rawFind(query, { skip, limit, sort })); |
There was a problem hiding this comment.
Done. Everything going to go through a private method now, which will retry find just once.
|
@nlutsenko updated the pull request. |
| return req.config.database.adaptiveCollection('_SCHEMA') | ||
| .then(collection => collection.find({})) | ||
| .then(schemas => schemas.map(mongoSchemaToSchemaAPIResponse)) | ||
| .then(schemas => ({ response: { results: schemas }})); |
There was a problem hiding this comment.
Putting this in a separate then is actually marginally slower than not doing so, because each then callback happens in a separate turn of the event loop. Very minor difference, but I thought you might want to know (if you end up contributing to the dashboard at all, avoiding extraneous thens is a bigger deal because doing too many can cause a re-render on each turn of the event loop)
There was a problem hiding this comment.
(y) That makes sense.
Do you want me to change it here? It is soo beautiful with an extra then :)
There was a problem hiding this comment.
Here it is a very small perf impact, so I don't think it really matters. In the dashboard I would ask you to change it :p
|
A couple optional things. Feel free to ignore them. |
Add MongoCollection wrapper and move few basic uses of collection to it.
Adds
MongoCollectionwrapper class that will be used as a replacement for direct mongo drive collection access.Also, moved few basic things that interact with the collection directly to this new API.
As you can see - we can already move custom indexing logic into MongoCollection, since it's super specific to mongo.
The next steps:
database.collection()todatabase.adaptiveCollection()adding the APIs where we need