Skip to content

Commit

Permalink
Engine: Fix bug introduced when refactoring upload_calculation
Browse files Browse the repository at this point in the history
In 6898ff4 the implementation of the
processing of the `local_copy_list` in the `upload_calculation` method
was changed. Originally, the files specified by the `local_copy_list`
were first copied into the `SandboxFolder` before copying its contents
to the working directory using the transport. The commit allowed the
order in which the local and sandbox files were copied, so the local
files were now no longer copied through the sandbox. Rather, they were
copied to a temporary directory on disk, which was then copied over
using the transport. The problem is that if the latter would copy over a
directory that was already created by the copying of the sandbox, an
exception would be raised.

For example, if the sandbox contained the directory `folder` and the
`local_copy_list` contained the items `(_, 'file', './folder/file')`
this would work just fine in the original implementation as the `file`
would be written to the `folder` on the remote folder. The current
implementation, however, would write the file content to `folder/file`
in a local temporary directory, and then iterate over the directories
and copy them over. Essentially it would be doing:

    Transport.put('/tmpdir/folder', 'folder')

but since `folder` would already exist on the remote working directory
the local folder would be _nested_ and so the final path on the remote
would be `/workingdir/folder/folder/file`.

The correct approach is to copy each item of the `local_copy_list` from
the local temporary directory _individually_ using the `Transport.put`
interface and not iterate over the top-level entries of the temporary
directory at the end.
  • Loading branch information
sphuber committed May 5, 2024
1 parent 4934c20 commit db3c9f1
Showing 1 changed file with 18 additions and 18 deletions.
36 changes: 18 additions & 18 deletions src/aiida/engine/daemon/execmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,23 +331,24 @@ def _copy_remote_files(logger, node, computer, transport, remote_copy_list, remo


def _copy_local_files(logger, node, transport, inputs, local_copy_list):
"""Perform the copy instrctions of the ``local_copy_list``."""
with TemporaryDirectory() as tmpdir:
dirpath = pathlib.Path(tmpdir)
"""Perform the copy instructions of the ``local_copy_list``."""

# The transport class can only copy files directly from the file system, so the files in the source node's repo
# have to first be copied to a temporary directory on disk.
for uuid, filename, target in local_copy_list:
logger.debug(f'[submission of calculation {node.uuid}] copying local file/folder to {target}')
# The transport class can only copy files directly from the file system, so the files in the source node's repo
# have to first be copied to a temporary directory on disk.
for uuid, filename, target in local_copy_list:
logger.debug(f'[submission of calculation {node.uuid}] copying local file/folder to {target}')

try:
data_node = load_node(uuid=uuid)
except exceptions.NotExistent:
data_node = _find_data_node(inputs, uuid) if inputs else None
try:
data_node = load_node(uuid=uuid)
except exceptions.NotExistent:
data_node = _find_data_node(inputs, uuid) if inputs else None

if data_node is None:
logger.warning(f'failed to load Node<{uuid}> specified in the `local_copy_list`')
continue
if data_node is None:
logger.warning(f'failed to load Node<{uuid}> specified in the `local_copy_list`')
continue

with TemporaryDirectory() as tmpdir:
dirpath = pathlib.Path(tmpdir)

# If no explicit source filename is defined, we assume the top-level directory
filename_source = filename or '.'
Expand All @@ -360,15 +361,14 @@ def _copy_local_files(logger, node, transport, inputs, local_copy_list):
if data_node.base.repository.get_object(filename_source).file_type == FileType.DIRECTORY:
# If the source object is a directory, we copy its entire contents
data_node.base.repository.copy_tree(filepath_target, filename_source)
transport.put(f'{dirpath}/*', target or '.')
else:
# Otherwise, simply copy the file
with filepath_target.open('wb') as handle:
with data_node.base.repository.open(filename_source, 'rb') as source:
shutil.copyfileobj(source, handle)

# Now copy the contents of the temporary folder to the remote working directory using the transport
for filepath in dirpath.iterdir():
transport.put(str(filepath), filepath.name)
transport.makedirs(str(pathlib.Path(target).parent), ignore_existing=True)
transport.put(str(filepath_target), target)


def _copy_sandbox_files(logger, node, transport, folder):
Expand Down

0 comments on commit db3c9f1

Please sign in to comment.