CalcJob: allow nested target paths for local_copy_list

[sphuber]

Fixes #4350

If a CalcJob would specify a local_copy_list containing an entry
where the target remote path contains nested subdirectories, the
upload_calculation would except unless all subdirectories would
already exist. To solve this, one could have added a transport call that
would create the directories if the target path is nested. However, this
would risk being very inefficient if there are many local copy list
instructions with relative path, where each would incurr a command over
the transport.

Instead, we change the design and simply apply the local copy list
instructions to the sandbox folder on the local file system. This also
at the same time allows us to get rid of the inefficient workaround of
writing the file to a temporary file, because the transport interface
doesn't accept filelike objects and the file repository does not expose
filepaths on the local file system.

The only additional thing to take care of is to make sure the files from
the local copy list do not end up in the repository of the node, which
was the whole point of the local_copy_list's existence in the first
place. But this is solved by simply adding each file, that is added to
the sandbox, also to the provenance_exclude_list.

[codecov]

Codecov Report

Merging #4373 into develop will increase coverage by 0.01%.
The diff coverage is 71.43%.

@@             Coverage Diff             @@
##           develop    #4373      +/-   ##
===========================================
+ Coverage    79.32%   79.33%   +0.01%     
===========================================
  Files          468      468              
  Lines        34752    34755       +3     
===========================================
+ Hits         27564    27569       +5     
+ Misses        7188     7186       -2     

Flag	Coverage Δ
#django	72.96% <71.43%> (+0.01%)	⬆️
#sqlalchemy	72.16% <71.43%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
aiida/engine/daemon/execmanager.py	61.97% <71.43%> (+0.50%)	⬆️
aiida/transports/plugins/local.py	81.54% <0.00%> (+0.26%)	⬆️
aiida/engine/daemon/runner.py	82.76% <0.00%> (+3.45%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff7b9e6...3641fcd. Read the comment docs.

[ramirezfranciscof]

So, apparently it all worked basically with the modifications we discusses as you had anticipated! I only have 2 comments: one I already did in the code, the second is if it would be possible to add a test that checks this now allows to generate directories by providing full paths in the local_copy_file.

[ramirezfranciscof]

So, you moved this here and now you have a dry_run check right next to another; would it perhaps be better to unify these two conditional branches? (the first is just if not dry_run, but the second has an else clause in line 225).

[sphuber]

Done and also added a test for the nested paths

[ramirezfranciscof]

So, I've also noticed that in the if section I mentioned in the review there is a part where if you are doing a dry run, it would copy the files requested from the remote machine locally to your sandbox folder. It would seem to me that this part would be easy to modify if we wanted to allow a list for copying files from different machines (it would just do this code even if it was not a dry run and the machine uuid was different, and then it would add the files to the provenance exlude list as we are doing in this PR).

aiida-core/aiida/engine/daemon/execmanager.py

Lines 226 to 248 in b002a9a

	else:
	for (remote_computer_uuid, remote_abs_path, dest_rel_path) in remote_copy_list:
	if remote_computer_uuid == computer.uuid:
	logger.debug(
	'[submission of calculation {}] copying {} remotely, directly on the machine {}'.format(
	node.pk, dest_rel_path, computer.label
	)
	)
	try:
	transport.copy(remote_abs_path, dest_rel_path)
	except (IOError, OSError):
	logger.warning(
	'[submission of calculation {}] Unable to copy remote resource from {} to {}! '
	'Stopping.'.format(node.pk, remote_abs_path, dest_rel_path)
	)
	raise
	else:
	raise NotImplementedError(
	'[submission of calculation {}] Remote copy between two different machines is '
	'not implemented yet'.format(node.pk)
	)

[sphuber]

@ramirezfranciscof thanks for the review. I addressed your comments and this should be ready for review

[ramirezfranciscof]

Cool stuff; good to go for me! I'll see if I can rebase transport after you merge and start finishing that up.