Add update and delete functionality #8
Conversation
Added an enum for allowed operation types and set up the basic mechanism for running different operations.
Tests that pass on master now pass with the modified API in place.
update operation now passes a simple test.
Added a few more basic tests for the update operation
Tests are passing.
Cleaned up the code somewhat. Renamed some of the variables in index_spec.js to avoid shadowing.
…er-Van-Drunen/node-stream-to-mongo-db Merged new operation features with master
|
@Peter-Van-Drunen I haven't forgotten this PR but I have some work to do on my own. And this PR is quite big 😅 |
| }; | ||
|
|
||
| // Utility for writing to the db with the correct operation | ||
| const writeToDB = async () => { |
There was a problem hiding this comment.
I think we could cache this function result before calling it.
Here you call this function every time we need to write to db (line 84).
There was a problem hiding this comment.
Plus the error about invalid operation will be triggered before the stream is used.
There was a problem hiding this comment.
I speak about which operation to choose (which callback)
| } | ||
| } | ||
| }); | ||
|
|
||
| writable.on('finish', async () => { | ||
| try { | ||
| if (records.length > 0) await insert(); | ||
| if (records.length > 0) await writeToDB(); |
There was a problem hiding this comment.
I'll fell more confortable if you reset records here too.
That's why I put it to insert at first place :)
| @@ -7,50 +7,95 @@ module.exports = { | |||
| // default config | |||
| { | |||
| batchSize: 1, | |||
| insertOptions: { w: 1 }, | |||
| operationType: 'insert' | |||
There was a problem hiding this comment.
Here I have the feeling you break the API for some users.
Maybe we should keep insertOptions for now, and use it into operation.insert?
|
@fabienjuif Thanks for the feedback, I will resolve these problems when I get the chance. |
|
Yeah sure @Peter-Van-Drunen you did a great job already 👍 If you don't have time since then, I'll try to make the patch needed ;) |
|
Any news about merging this PR? |
This PR aims to add update and delete functionality to the module. See issue #7.
There may be room for improvement before this should be merged. Let me know what you think @AbdullahAli @fabienjuif