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.

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

fix: delete configured computers #5397

Merged
merged 3 commits into from
Feb 27, 2022

Conversation

ltalirz
Copy link
Member

@ltalirz ltalirz commented Feb 25, 2022

fix #5396

Deleting a configured computer resulted in an error, since the
associated Authinfo object was not deleted (instead, the "dbcomputer_id"
was set to null, which violated a not-null constraint).

Deleting computers was already tested, but not deleting a configured
computer. This is fixed as well.

Deleting a configured computer resulted in an error, since the
associated Authinfo object was not deleted (instead, the "dbcomputer_id"
was set to null, which violated a not-null constraint).

Deleting computers was already tested, but not deleting a configured
computer. This is fixed as well.
@ltalirz
Copy link
Member Author

ltalirz commented Feb 25, 2022

@chrisjsewell Seems like the deletion test passes now.

Do I need to write a migration here?
Is there an automatic way of getting it?

@chrisjsewell
Copy link
Member

chrisjsewell commented Feb 25, 2022

Do I need to write a migration here?

No, relationships are at the ORM level (client-side), i.e. transient and not part of the database schema

Might be good to add two tests to tests/orm/test_authinfos.py, to specifically test that deleting a computer or user also removes any associated authinfos
(but don't try to delete the default user, because that may mess up subsequent tests)

@ltalirz
Copy link
Member Author

ltalirz commented Feb 25, 2022

While looking into this, I realized that there actually is no User.objects.delete - is there any way to delete a user?

If not should I be adding it in this PR?

@chrisjsewell
Copy link
Member

is there any way to delete a user?

Not on the front-end (i.e. in aiida/orm/implementation/users.py)

I guess thats a bit of a wider discussion than this PR 😬
As mentioned above, we should probably not allow for the deletion of the default user for a profile, since that is a bit of a special case

@ltalirz
Copy link
Member Author

ltalirz commented Feb 27, 2022

is there any way to delete a user?

Not on the front-end (i.e. in aiida/orm/implementation/users.py)

I guess thats a bit of a wider discussion than this PR 😬 As mentioned above, we should probably not allow for the deletion of the default user for a profile, since that is a bit of a special case

Ok, then I guess it's enough if I add a test that deleting a computer also deletes the authinfo and leave the test deleting a user for another time.

@ltalirz ltalirz requested a review from chrisjsewell February 27, 2022 11:45
@chrisjsewell
Copy link
Member

👍

Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cheers

@ltalirz ltalirz merged commit 3fa81cf into aiidateam:develop Feb 27, 2022
@sphuber sphuber deleted the issue-5396-computer-delete branch February 27, 2022 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to delete computer - null value in column "dbcomputer_id" violates not-null constraint
2 participants