Skip to content

Commit

Permalink
Fix bug in upload_calculation for CalcJobs with local codes
Browse files Browse the repository at this point in the history
This code path was not tested at all and so the remaining occurrence of
`get_folder_list` which is part of the old repository interface that was
removed in `v1.0.0` went unnoticed.

The fix was forced to write files to temporary files and flush them
instead of using a filelike object, because `Transport.put` only accepts
absolute filepaths for the moment.
  • Loading branch information
sphuber committed Jan 6, 2020
1 parent 982b089 commit 422c0ff
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 5 deletions.
20 changes: 19 additions & 1 deletion aiida/backends/tests/engine/test_calc_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"""Test for the `CalcJob` process sub class."""
from copy import deepcopy
from functools import partial
import os
from unittest.mock import patch

import pytest
Expand Down Expand Up @@ -38,9 +39,11 @@ class TestCalcJob(AiidaTestCase):
@classmethod
def setUpClass(cls, *args, **kwargs):
super().setUpClass(*args, **kwargs)
import aiida
files = [os.path.join(os.path.dirname(aiida.__file__), os.pardir, '.ci', 'add.sh')]
cls.computer.configure() # pylint: disable=no-member
cls.remote_code = orm.Code(remote_computer_exec=(cls.computer, '/bin/true')).store()
cls.local_code = orm.Code(local_executable='true', files=['/bin/true']).store()
cls.local_code = orm.Code(local_executable='add.sh', files=files).store()
cls.inputs = {
'x': orm.Int(1),
'y': orm.Int(2),
Expand Down Expand Up @@ -213,3 +216,18 @@ def test_exception_presubmit(self):
launch.run(ArithmeticAddCalculation, code=self.remote_code, **self.inputs)

self.assertIn('exception occurred in presubmit call', str(context.exception))

def test_run_local_code(self):
"""Run a dry-run with local code."""
inputs = deepcopy(self.inputs)
inputs['code'] = self.local_code
inputs['metadata']['computer'] = self.computer
inputs['metadata']['dry_run'] = True

# The following will run `upload_calculation` which will test that uploading files works correctly
_, node = launch.run_get_node(ArithmeticAddCalculation, **inputs)
uploaded_files = os.listdir(node.dry_run_info['folder'])

# Since the repository will only contain files on the top-level due to `Code.set_files` we only check those
for filename in self.local_code.list_object_names():
self.assertTrue(filename in uploaded_files)
13 changes: 9 additions & 4 deletions aiida/engine/daemon/execmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def upload_calculation(node, transport, calc_info, folder, inputs=None, dry_run=
link_label = 'remote_folder'
if node.get_outgoing(RemoteData, link_label_filter=link_label).first():
execlogger.warning('CalcJobNode<{}> already has a `{}` output: skipping upload'.format(node.pk, link_label))
return calc_info, script_filename
return calc_info

computer = node.computer

Expand Down Expand Up @@ -139,8 +139,14 @@ def upload_calculation(node, transport, calc_info, folder, inputs=None, dry_run=
for code in input_codes:
if code.is_local():
# Note: this will possibly overwrite files
for f in code.get_folder_list():
transport.put(code.get_abs_path(f), f)
for f in code.list_object_names():
# 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`
# Since the content of the node could potentially be binary, we read the raw bytes and pass them on
with NamedTemporaryFile(mode='wb+') as handle:
handle.write(code.get_object_content(f, mode='rb'))
handle.flush()
transport.put(handle.name, f)
transport.chmod(code.get_local_executable(), 0o755) # rwxr-xr-x

# In a dry_run, the working directory is the raw input folder, which will already contain these resources
Expand Down Expand Up @@ -192,7 +198,6 @@ def find_data_node(inputs, uuid):
with NamedTemporaryFile(mode='wb+') as handle:
handle.write(data_node.get_object_content(filename, mode='rb'))
handle.flush()
handle.seek(0)
transport.put(handle.name, target)

if dry_run:
Expand Down

0 comments on commit 422c0ff

Please sign in to comment.