Skip to content

Commit

Permalink
Fix bug in aiida.engine.daemon.execmanager.retrieve_files_from_list
Browse files Browse the repository at this point in the history
The `retrieve_files_from_list` would loop over the instructions of the
`retrieve_list` attribute of a calculation job and for each entry define
the variables `remote_names` and `local_names` which contain the
filenames of the remote files that are to be retrieved and with what
name locally.

However, for the code path where the element of `retrieve_list` is a
list or tuple and the first element contains no wildcard characters, the
`remote_names` variable is not defined, meaning that the value of the
previous iteration would be used. This was never detected because this
code path was not actually tested.

This bug would only affect `CalcJob`s that specified a `retrieve_list`
that contained an entry of the form:

    ['some/path', 'some/path', 0]

where the entry is a list and the first element does not contain a
wildcard.
  • Loading branch information
sphuber committed Jul 21, 2020
1 parent 0d7baa5 commit 135caf9
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 4 deletions.
9 changes: 5 additions & 4 deletions aiida/engine/daemon/execmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -546,12 +546,12 @@ def retrieve_files_from_list(calculation, transport, folder, retrieve_list):
treated as the work directory of the folder and the depth integer determines
upto what level of the original remotepath nesting the files will be copied.
:param transport: the Transport instance
:param folder: an absolute path to a folder to copy files in
:param retrieve_list: the list of files to retrieve
:param transport: the Transport instance.
:param folder: an absolute path to a folder that contains the files to copy.
:param retrieve_list: the list of files to retrieve.
"""
for item in retrieve_list:
if isinstance(item, list):
if isinstance(item, (list, tuple)):
tmp_rname, tmp_lname, depth = item
# if there are more than one file I do something differently
if transport.has_magic(tmp_rname):
Expand All @@ -561,6 +561,7 @@ def retrieve_files_from_list(calculation, transport, folder, retrieve_list):
to_append = rem.split(os.path.sep)[-depth:] if depth > 0 else []
local_names.append(os.path.sep.join([tmp_lname] + to_append))
else:
remote_names = [tmp_rname]
to_append = tmp_rname.split(os.path.sep)[-depth:] if depth > 0 else []
local_names = [os.path.sep.join([tmp_lname] + to_append)]
if depth > 1: # create directories in the folder, if needed
Expand Down
56 changes: 56 additions & 0 deletions tests/engine/daemon/test_execmanager.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# -*- coding: utf-8 -*-
###########################################################################
# Copyright (c), The AiiDA team. All rights reserved. #
# This file is part of the AiiDA code. #
# #
# The code is hosted on GitHub at https://github.com/aiidateam/aiida-core #
# For further information on the license, see the LICENSE.txt file #
# For further information please visit http://www.aiida.net #
###########################################################################
"""Tests for the :mod:`aiida.engine.daemon.execmanager` module."""
import os
import pytest

from aiida.engine.daemon import execmanager
from aiida.transports.plugins.local import LocalTransport


@pytest.mark.usefixtures('clear_database_before_test')
def test_retrieve_files_from_list(tmp_path_factory, generate_calculation_node):
"""Test the `retrieve_files_from_list` function."""
node = generate_calculation_node()

retrieve_list = [
'file_a.txt',
('sub/folder', 'sub/folder', 0),
]

source = tmp_path_factory.mktemp('source')
target = tmp_path_factory.mktemp('target')

content_a = b'content_a'
content_b = b'content_b'

with open(str(source / 'file_a.txt'), 'wb') as handle:
handle.write(content_a)
handle.flush()

os.makedirs(str(source / 'sub' / 'folder'))

with open(str(source / 'sub' / 'folder' / 'file_b.txt'), 'wb') as handle:
handle.write(content_b)
handle.flush()

with LocalTransport() as transport:
transport.chdir(str(source))
execmanager.retrieve_files_from_list(node, transport, str(target), retrieve_list)

assert sorted(os.listdir(str(target))) == sorted(['file_a.txt', 'sub'])
assert os.listdir(str(target / 'sub')) == ['folder']
assert os.listdir(str(target / 'sub' / 'folder')) == ['file_b.txt']

with open(str(target / 'sub' / 'folder' / 'file_b.txt'), 'rb') as handle:
assert handle.read() == content_b

with open(str(target / 'file_a.txt'), 'rb') as handle:
assert handle.read() == content_a

0 comments on commit 135caf9

Please sign in to comment.