Engine: Fix bug in upload calculation for PortableCode with SSH (#6519

)

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.

src/aiida/engine/daemon/execmanager.py

@@ -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`
@@ -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()

tests/engine/daemon/test_execmanager.py

@@ -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
 
@@ -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,
+        )