-
Notifications
You must be signed in to change notification settings - Fork 214
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
bug fix for verdi node delete
#2545
Conversation
Fix a bug such that if no argument is passed to `verdi node delete`. It will try to delete all notes in the database. This is because `QueryBuilder().append(Node, filters={'or': []}, project='id').all()` match to every single node and projects its id. Perhaps this behaviour of `QueryBuilder` is not as expected?
@zhubonan thanks for the bug report & the fix!
As a general rule, I think it makes sense if an "empty" or condition is true. I can see why it may look a bit confusing, but I think it will also be rare that a user writes The bug here is that not providing a node PK results in no filter being applied (rather than doing nothing because of the missing command line arguments). |
Co-Authored-By: zhubonan <33688599+zhubonan@users.noreply.github.com>
I see. Thanks. |
Just checked that |
Fix a bug such that if no argument is passed to
verdi node delete
. It will try to delete ALL notes in the database.This is because
QueryBuilder().append(Node, filters={'or': []}, project='id').all()
match to every single node and projects its id.Perhaps this behaviour of
QueryBuilder
is not as expected? It makes more sense if an exception is raised when the'or'
list contains no element at all. I am not sure if it is still the case in 1.0.0 versions.