Add the retrieve_temporary_list member to the CalcInfo datastructure #892
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For some calculation one wants to retrieve certain files for parsing but does not want to store them permanently in the repository because for example they may take up a lot of space. To facilitate for this use case we introduce the concept of the retrieve_temporary_list member in the CalcInfo datastructure. Implementations of a Calculation can opt to provide a list of directives, identical to that of the original retrieve_list member, that will instruct the exectution manager to retrieve those files to a local folder for parsing. The difference is that the files listed in the retrieve_temporary_list member will be stored in a FolderData node that will not be stored in the database and which will only be alive for the duration of the parsing. After parsing is completed the retrieved files are garbage collected.
giovannipizzi
requested changes
Nov 6, 2017
aiida/common/datastructures.py
Outdated
| # ['remotepath','localpath',depth] ] | ||
| 'retrieve_temporary_list', # a list of files or patters to retrieve that will only be kept during parsing and will be deleted after parsing is completed | ||
| # possible formats: ['remotepath', # just the name of the file to retrieve. Will be put in '.' of the repository with name os.path.split(item)[1] | ||
| # ['remotepath','localpath', depth]] | ||
| # second format will copy the remotepath file/folder to localpath. |
There was a problem hiding this comment.
This comment belongs still to 'retrieve_list', whose description is not 'cut'
aiida/daemon/execmanager.py
Outdated
| @@ -806,6 +771,7 @@ def retrieve_computed_for_authinfo(authinfo): | |||
| calc.pk, retrieved_files.dbnode.pk), | |||
| extra=logger_extra) | |||
| retrieved_files.store() | |||
|
|
|||
There was a problem hiding this comment.
Add a comment that we are not storing the _temp files
There was a problem hiding this comment.
Thanks, fine for me. I'll approve this and let you merge or decide to put an explicit clear of the SandboxFolder for the temporary FolderData
giovannipizzi
previously approved these changes
Nov 6, 2017
Also deleted the link that was added to the retrieved_temporary_folder which was a left over from testing phase. Since the FolderData is never going to be stored, it also shouldn't have any links
The parser will not do anything, but if the 'retrieve_temporary_files' key was set in the 'template' input node, then it is expected that those files will be temporarily stored in an unstored FolderData node which should be passed in the 'retrieved' argument of the parse_with_retrieved call of the Parser class. In that case, the arguments are checked for the presence of the unstored FolderData node
sphuber
force-pushed
the
fix_886_retrieve_temporary_list
branch
7 times, most recently
from
634e4ba to
bc6bdcf
Compare
The daemon integration test leverages the TemplatereplacerCalculation to launch some trivial calculations. We add the output filepath to the 'retrieve_temporary_files' entry of the template dictionary which will cause the TemplatereplacerParser class to check for an unstored FolderData node containing those files, which should be passed to the parse_with_retrieved function called by the execmanager
sphuber
force-pushed
the
fix_886_retrieve_temporary_list
branch
from
bc6bdcf to
1ea8548
Compare
|
I have added a |
Also enhance the parser by explicitly verifying that all the files that are listed in the 'template' input node in the retrieve_temp_list key are present in the temporary folder
sphuber
force-pushed
the
fix_886_retrieve_temporary_list
branch
3 times, most recently
from
8f7e1d5 to
fad81b4
Compare
This means that the parsing of the output file can be moved from the daemon test to the actual parser itself
sphuber
force-pushed
the
fix_886_retrieve_temporary_list
branch
from
fad81b4 to
53c3479
Compare
giovannipizzi
approved these changes
Nov 7, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #886