Skip to content

Commit

Permalink
fix: delete configured computers (#5397)
Browse files Browse the repository at this point in the history
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 now tested, as well as that removing a Computer
results in removal of the corresponding Authinfo.

Since deleting users from the frontend is currently not possible,
we do not add a test to check that removing a User results in
removal of their Authinfos.

Co-authored-by: Chris Sewell <chrisj_sewell@hotmail.com>
  • Loading branch information
ltalirz and chrisjsewell authored Feb 27, 2022
1 parent 7211f84 commit 3fa81cf
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 11 deletions.
6 changes: 3 additions & 3 deletions aiida/storage/psql_dos/models/authinfo.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"""Module to manage authentification information for the SQLA backend."""
from sqlalchemy import ForeignKey
from sqlalchemy.dialects.postgresql import JSONB
from sqlalchemy.orm import relationship
from sqlalchemy.orm import backref, relationship
from sqlalchemy.schema import Column, UniqueConstraint
from sqlalchemy.types import Boolean, Integer

Expand Down Expand Up @@ -44,8 +44,8 @@ class DbAuthInfo(Base):
auth_params = Column(JSONB, default=dict, nullable=False)
enabled = Column(Boolean, default=True, nullable=False)

aiidauser = relationship('DbUser', backref='authinfos')
dbcomputer = relationship('DbComputer', backref='authinfos')
aiidauser = relationship('DbUser', backref=backref('authinfos', passive_deletes=True, cascade='all, delete'))
dbcomputer = relationship('DbComputer', backref=backref('authinfos', passive_deletes=True, cascade='all, delete'))

__table_args__ = (UniqueConstraint('aiidauser_id', 'dbcomputer_id'),)

Expand Down
14 changes: 8 additions & 6 deletions tests/cmdline/commands/test_computer.py
Original file line number Diff line number Diff line change
Expand Up @@ -666,24 +666,26 @@ def test_computer_delete(self):
from aiida.common.exceptions import NotExistent

# Setup a computer to delete during the test
label = 'computer_for_test_label'
orm.Computer(
label='computer_for_test_delete',
label=label,
hostname='localhost',
transport_type='core.local',
scheduler_type='core.direct',
workdir='/tmp/aiida'
).store()
# and configure it
options = ['core.local', label, '--non-interactive', '--safe-interval', '0']
self.cli_runner(computer_configure, options, catch_exceptions=False)

# See if the command complains about not getting an invalid computer
options = ['non_existent_computer_name']
self.cli_runner(computer_delete, options, raises=True)
self.cli_runner(computer_delete, ['computer_that_does_not_exist'], raises=True)

# Delete a computer name successully.
options = ['computer_for_test_delete']
self.cli_runner(computer_delete, options)
self.cli_runner(computer_delete, [label])
# Check that the computer really was deleted
with pytest.raises(NotExistent):
orm.Computer.objects.get(label='computer_for_test_delete')
orm.Computer.objects.get(label=label)


@pytest.mark.usefixtures('aiida_profile_clean')
Expand Down
12 changes: 10 additions & 2 deletions tests/orm/test_authinfos.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import pytest

from aiida.common import exceptions
from aiida.orm import authinfos
from aiida.orm import authinfos, computers


class TestAuthinfo:
Expand All @@ -32,7 +32,7 @@ def test_set_auth_params(self):
self.auth_info.set_auth_params(auth_params)
assert self.auth_info.get_auth_params() == auth_params

def test_delete(self):
def test_delete_authinfo(self):
"""Test deleting a single AuthInfo."""
pk = self.auth_info.pk

Expand All @@ -42,3 +42,11 @@ def test_delete(self):

with pytest.raises(exceptions.NotExistent):
authinfos.AuthInfo.objects.delete(pk)

def test_delete_computer(self):
"""Test that deleting a computer also deletes the associated Authinfo."""
pk = self.auth_info.pk

computers.Computer.objects.delete(self.computer.pk)
with pytest.raises(exceptions.NotExistent):
authinfos.AuthInfo.objects.delete(pk)

0 comments on commit 3fa81cf

Please sign in to comment.