Skip to content

Commit

Permalink
Engine: Fix bug in upload calculation for PortableCode with SSH
Browse files Browse the repository at this point in the history
When a `CalcJob` would be run with a `PortableCode` using a computer
configured with the `core.ssh` transport plugin, the upload task would
except. The `aiida.engine.daemon.execmanager.upload_calculation` method
is passing `pathlib.Path` objects to the transport interface which is
not supported. By chance this does not raise an exception when using the
`LocalTransport`, but the `SshTransport` passes these values to the
paramiko library which does choke on anything but strings.

The use of a `PortableCode` was tested for in the unit test
`tests/engine/processes/calcjobs/test_calc_job.py:test_portable_code`
but this would only use a local transport and thus the bug would not
appear. Parametrizing it to also use the `SshTransport` wouldn't help
since the test uses `metadata.dry_run = True`, whose implementation will
always swap the transport to a local one, still avoiding the bugged
code pathway.

Instead a test is added that directly calls `upload_calculation` which
parametrizes over all installed transport plugins and uses a
`PortableCode`. This confirmed the bug. The `upload_calculation` method
is updated to ensure casting all `pathlib.Path` objects to `str` before
passing it to the transport.
  • Loading branch information
sphuber committed Jul 4, 2024
1 parent 6d2edc9 commit e738c1c
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 5 deletions.
8 changes: 4 additions & 4 deletions src/aiida/engine/daemon/execmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,11 @@ def upload_calculation(
# Note: this will possibly overwrite files
for root, dirnames, filenames in code.base.repository.walk():
# mkdir of root
transport.makedirs(root, ignore_existing=True)
transport.makedirs(str(root), ignore_existing=True)

# remotely mkdir first
for dirname in dirnames:
transport.makedirs((root / dirname), ignore_existing=True)
transport.makedirs(str(root / dirname), ignore_existing=True)

# Note, once #2579 is implemented, use the `node.open` method instead of the named temporary file in
# combination with the new `Transport.put_object_from_filelike`
Expand All @@ -192,8 +192,8 @@ def upload_calculation(
content = code.base.repository.get_object_content((pathlib.Path(root) / filename), mode='rb')
handle.write(content)
handle.flush()
transport.put(handle.name, (root / filename))
transport.chmod(code.filepath_executable, 0o755) # rwxr-xr-x
transport.put(handle.name, str(root / filename))
transport.chmod(str(code.filepath_executable), 0o755) # rwxr-xr-x

# local_copy_list is a list of tuples, each with (uuid, dest_path, rel_path)
# NOTE: validation of these lists are done inside calculation.presubmit()
Expand Down
31 changes: 30 additions & 1 deletion tests/engine/daemon/test_execmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from aiida.common.datastructures import CalcInfo, CodeInfo, FileCopyOperation
from aiida.common.folders import SandboxFolder
from aiida.engine.daemon import execmanager
from aiida.orm import CalcJobNode, FolderData, RemoteData, SinglefileData
from aiida.orm import CalcJobNode, FolderData, PortableCode, RemoteData, SinglefileData
from aiida.plugins import entry_point
from aiida.transports.plugins.local import LocalTransport

Expand Down Expand Up @@ -585,3 +585,32 @@ def test_upload_combinations(
filepath_workdir = pathlib.Path(node.get_remote_workdir())

assert serialize_file_hierarchy(filepath_workdir, read_bytes=False) == expected_hierarchy


def test_upload_calculation_portable_code(fixture_sandbox, node_and_calc_info, tmp_path):
"""Test ``upload_calculation`` with a ``PortableCode`` for different transports.
Regression test for https://github.com/aiidateam/aiida-core/issues/6518
"""
subdir = tmp_path / 'sub'
subdir.mkdir()
(subdir / 'some-file').write_bytes(b'sub dummy')
(tmp_path / 'bash').write_bytes(b'bash implementation')

code = PortableCode(
filepath_executable='bash',
filepath_files=tmp_path,
).store()

node, calc_info = node_and_calc_info
code_info = CodeInfo()
code_info.code_uuid = code.uuid
calc_info.codes_info = [code_info]

with node.computer.get_transport() as transport:
execmanager.upload_calculation(
node,
transport,
calc_info,
fixture_sandbox,
)

0 comments on commit e738c1c

Please sign in to comment.