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
Implement remove
with tests and doc example
#147
Conversation
delete_many
with tests and doc example
@art049 please review at your earliest convenience. |
Codecov Report
@@ Coverage Diff @@
## master #147 +/- ##
===========================================
+ Coverage 99.60% 100.00% +0.39%
===========================================
Files 38 38
Lines 2775 2723 -52
Branches 495 178 -317
===========================================
- Hits 2764 2723 -41
+ Misses 8 0 -8
+ Partials 3 0 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Hey @joeriddles thanks for the PR ! I had a look to the code and it looks good ;) ! The last thing is about the naming of this method. With the current API it might look confusing as we will have:
One possibility would be to rename the new method to If you have other ideas, don't hesitate. As well @adriencaccia ideas are welcomed :) |
It would be great if you can incorporate this :) |
Hey @art049 thanks for the reply and review!
Will do 👍
I think that's a great call out. I will update the PR accordingly.
@Kromtar thanks for the reminder to wrap this PR up! I'll work on it tomorrow. |
@art049 Please review the updated PR when you can. |
delete_many
with tests and doc exampleremove
with tests and doc example
e70dcd3
to
1bf579a
Compare
Closing in favor of #237. Implementing remove directly with |
Closes Issue #121
Notes
DocumentsNotFoundError
to aggregate multiple documents not being found.delete_many
should typically raiseDocumentsNotFoundError
, unless another thread deletes one of the instances returned fromfind
before it is deleted from withindelete_many
. This should be a rare scenario typically.