Skip to content

Commit

Permalink
CLI: Check user execute in verdi code test for InstalledCode (#6597)
Browse files Browse the repository at this point in the history
An additional test is added for user execute permissions in the `validate_filepath_executable` method of `InstalledCode` called by `verdi code test`.

This can help debugging (or even avoiding) the issue of `touch`ing a dummy code, registering it in AiiDA, and then not being able to use it due to the script not being executable (as default Linux file permissions are `rw-rw-r--`). Some of us have actually encountered this while developing, and the resulting issue was difficult to debug. With this change, one should be able to find it quicker using `verdi code test`.
  • Loading branch information
GeigerJ2 authored Nov 5, 2024
1 parent 0fa9582 commit 8350df0
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
14 changes: 14 additions & 0 deletions src/aiida/orm/nodes/data/code/installed.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,13 @@ def validate_filepath_executable(self):
with override_log_level(): # Temporarily suppress noisy logging
with self.computer.get_transport() as transport:
file_exists = transport.isfile(str(self.filepath_executable))
if file_exists:
mode = transport.get_mode(str(self.filepath_executable))
# `format(mode, 'b')` with default permissions
# gives 110110100, representing rw-rw-r--
# Check on index 2 if user has execute
user_has_execute = format(mode, 'b')[2] == '1'

except Exception as exception:
raise exceptions.ValidationError(
'Could not connect to the configured computer to determine whether the specified executable exists.'
Expand All @@ -147,6 +154,13 @@ def validate_filepath_executable(self):
f'The provided remote absolute path `{self.filepath_executable}` does not exist on the computer.'
)

if not user_has_execute:
execute_msg = (
f'The file at the remote absolute path `{self.filepath_executable}` exists, '
'but might not actually be executable. Check the permissions.'
)
raise exceptions.ValidationError(execute_msg)

def can_run_on_computer(self, computer: Computer) -> bool:
"""Return whether the code can run on a given computer.
Expand Down
14 changes: 13 additions & 1 deletion tests/orm/data/code/test_installed.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,15 @@ def computer(request, aiida_computer_local, aiida_computer_ssh):

@pytest.mark.usefixtures('aiida_profile_clean')
@pytest.mark.parametrize('computer', ('core.local', 'core.ssh'), indirect=True)
def test_validate_filepath_executable(ssh_key, computer, bash_path):
def test_validate_filepath_executable(ssh_key, computer, bash_path, tmp_path):
"""Test the :meth:`aiida.orm.nodes.data.code.installed.InstalledCode.validate_filepath_executable` method."""

filepath_executable = '/usr/bin/not-existing'
dummy_executable = tmp_path / 'dummy.sh'
# Default Linux permissions are 664, so file is not executable
dummy_executable.touch()
code = InstalledCode(computer=computer, filepath_executable=filepath_executable)
dummy_code = InstalledCode(computer=computer, filepath_executable=str(dummy_executable))

with pytest.raises(ValidationError, match=r'Could not connect to the configured computer.*'):
code.validate_filepath_executable()
Expand All @@ -117,6 +122,13 @@ def test_validate_filepath_executable(ssh_key, computer, bash_path):
with pytest.raises(ValidationError, match=r'The provided remote absolute path .* does not exist on the computer\.'):
code.validate_filepath_executable()

with pytest.raises(
ValidationError,
match=r'The file at the remote absolute path .* exists, but might not actually be executable\.',
):
# Didn't put this in the if, using transport.put, as we anyway only connect to localhost via SSH in this test
dummy_code.validate_filepath_executable()

code.filepath_executable = str(bash_path.absolute())
code.validate_filepath_executable()

Expand Down

0 comments on commit 8350df0

Please sign in to comment.