Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redefine the structure of the CalcInfo.local_copy_list #2581

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Mar 6, 2019

Fixes #2573

With the upcoming change of the node repository, files within it can no
longer be assumed to necessarily be stored on the local file system. It
is therefore impossible to address files within it through an absolute
file path, however, that is exactly what the local_copy_list expects
as the first item of its tuples.

Now, the Node class only exposes methods to get either the content of
a repository file or an filelike object. To comply with this change in
design the structure of the local_copy_list tuples is changed to
tuples of length three where each element represents:

  • UUID of the node
  • Relative key of the file within the node repository
  • Relative path within the working directory where to copy the file

The upload_calculation function of the execmanager has been updated to
this interface change, but because our Transport interface does not
yet provide a method to put an object from a filelike object, we have to
create a temporary file on the local file system whose absolute filepath
can be passed to the Transport.put call. Once the transport is updated
to provide a put_object_from_filelike, this inefficiency can be removed.

@sphuber sphuber requested a review from giovannipizzi March 6, 2019 18:01
@sphuber
Copy link
Contributor Author

sphuber commented Mar 6, 2019

@ltalirz and @yakutovicha if this PR gets merged, we need to update the guide for the implementation of CalcJob classes. After this fix, for a given node, to copy one of the files in its repositories, one can do:

folder = self.inputs.folder # FolderData
singlefile = self.inputs.single # SinglefileData
local_copy_list = [
    (folder.uuid, 'relative/path.txt', 'relative/destination.txt')
    (single.uuid, single.filename, 'input_file.upf')
]

With the upcoming change of the node repository, files within it can no
longer be assumed to necessarily be stored on the local file system. It
is therefore impossible to address files within it through an absolute
file path, however, that is exactly what the `local_copy_list` expects
as the first item of its tuples.

Now, the `Node` class only exposes methods to get either the content of
a repository file or an filelike object. To comply with this change in
design the structure of the `local_copy_list` tuples is changed to
tuples of length three where each element represents:

 * UUID of the node
 * Relative key of the file within the node repository
 * Relative path within the working directory where to copy the file

The `upload_calculation` function of the execmanager has been updated to
this interface change, but because our `Transport` interface does not
yet provide a method to put an object from a filelike object, we have to
create a temporary file on the local file system whose absolute filepath
can be passed to the `Transport.put` call. Once the transport is updated
to provide a `put_object_from_filelike`, this inefficiency can be removed.
@sphuber sphuber force-pushed the fix_2573_redefine_local_copy_list branch from 6722444 to 4976712 Compare March 6, 2019 18:18
@sphuber
Copy link
Contributor Author

sphuber commented Mar 6, 2019

The failing test on Jenkins is a known bug that Casper is addressing in PR #2576

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 70.051% when pulling 4976712 on sphuber:fix_2573_redefine_local_copy_list into 277a4d8 on aiidateam:provenance_redesign.

@sphuber sphuber merged commit 838c1dd into aiidateam:provenance_redesign Mar 7, 2019
@sphuber sphuber deleted the fix_2573_redefine_local_copy_list branch March 12, 2019 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants