Skip to content

Commit

Permalink
InstalledCode: Fix bug in validate_filepath_executable for SSH (#…
Browse files Browse the repository at this point in the history
…5787)

If the code was attached to a `Computer` with `core.ssh` transport, the
`validate_filepath_executable` would always return that the computer
could not be connected to. This is because the actual exception that was
caught does not come from `Computer.get_transport` but is a `TypeError`
raised by `transport.isfile` since it receives a `pathlib.Path` were it
expects a `str`.

The problem went undetected since it only tested against a computer with
`core.local` transport. A new test is added to also test against a
computer with `core.ssh` transport`.
  • Loading branch information
sphuber authored Nov 24, 2022
1 parent 6469e23 commit 3cc681c
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 7 deletions.
6 changes: 3 additions & 3 deletions aiida/orm/nodes/data/code/installed.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,11 @@ def validate_filepath_executable(self):
try:
with override_log_level(): # Temporarily suppress noisy logging
with self.computer.get_transport() as transport:
file_exists = transport.isfile(self.filepath_executable)
except Exception: # pylint: disable=broad-except
file_exists = transport.isfile(str(self.filepath_executable))
except Exception as exception: # pylint: disable=broad-except
raise exceptions.ValidationError(
'Could not connect to the configured computer to determine whether the specified executable exists.'
)
) from exception

if not file_exists:
raise exceptions.ValidationError(
Expand Down
22 changes: 18 additions & 4 deletions tests/orm/data/code/test_installed.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
# pylint: disable=redefined-outer-name
"""Tests for the :class:`aiida.orm.nodes.data.code.installed.InstalledCode` class."""
import pathlib
import uuid

import pytest

Expand Down Expand Up @@ -89,16 +88,31 @@ def test_filepath_executable(aiida_localhost):
code.filepath_executable = filepath_executable


def test_validate_filepath_executable():
@pytest.fixture
def computer(request, aiida_computer_local, aiida_computer_ssh):
"""Return a computer configured for ``core.local`` and ``core.ssh`` transport."""
if request.param == 'core.local':
return aiida_computer_local(configure=False)

if request.param == 'core.ssh':
return aiida_computer_ssh(configure=False)

raise ValueError(f'unsupported request parameter: {request.param}')


@pytest.mark.parametrize('computer', ('core.local', 'core.ssh'), indirect=True)
def test_validate_filepath_executable(ssh_key, computer):
"""Test the :meth:`aiida.orm.nodes.data.code.installed.InstalledCode.validate_filepath_executable` method."""
filepath_executable = '/usr/bin/not-existing'
computer = Computer(label=str(uuid.uuid4()), transport_type='core.local')
code = InstalledCode(computer=computer, filepath_executable=filepath_executable)

with pytest.raises(ValidationError, match=r'Could not connect to the configured computer.*'):
code.validate_filepath_executable()

computer.configure()
if computer.transport_type == 'core.ssh':
computer.configure(key_filename=str(ssh_key), key_policy='AutoAddPolicy')
else:
computer.configure()

with pytest.raises(ValidationError, match=r'The provided remote absolute path .* does not exist on the computer\.'):
code.validate_filepath_executable()
Expand Down

0 comments on commit 3cc681c

Please sign in to comment.