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

Fix issue 477 #557

Merged
merged 9 commits into from Mar 30, 2021
Merged

Fix issue 477 #557

merged 9 commits into from Mar 30, 2021

Conversation

iparask
Copy link
Contributor

@iparask iparask commented Mar 2, 2021

Hello,

this PR fixes #477. It resolves any shared_data directive based on a > exists or not and pushes them to the pilot via pilot's stage_in method.

This is unit tested as well.

@codecov
Copy link

codecov bot commented Mar 2, 2021

Codecov Report

Merging #557 (91ad1f9) into devel (c1cc422) will increase coverage by 0.27%.
The diff coverage is 78.37%.

Impacted file tree graph

@@            Coverage Diff             @@
##            devel     #557      +/-   ##
==========================================
+ Coverage   63.11%   63.39%   +0.27%     
==========================================
  Files          21       21              
  Lines        2359     2371      +12     
==========================================
+ Hits         1489     1503      +14     
+ Misses        870      868       -2     
Impacted Files Coverage Δ
src/radical/entk/execman/rp/task_processor.py 84.79% <75.00%> (+1.22%) ⬆️
src/radical/entk/execman/rp/resource_manager.py 62.24% <88.88%> (+3.35%) ⬆️

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 c1cc422...91ad1f9. Read the comment docs.

@lee212
Copy link
Contributor

lee212 commented Mar 8, 2021

Are these affected?

td.input_staging = get_input_list_from_task(task, placeholders)
td.output_staging = get_output_list_from_task(task, placeholders)

@iparask
Copy link
Contributor Author

iparask commented Mar 10, 2021

You mean because of $SHARED. Thank you! I'll check and get back to you

@iparask
Copy link
Contributor Author

iparask commented Mar 10, 2021

Your comment made me think how placeholders are resolved when there are in both sides of >. This triggered some extra changes.

However, the user needs to keep in mind that EnTK does not have any knowledge of folder structure under $SHARED. For example, a user sets shared_data as:

appman.shared_data = ['folder1/file1 > folder1/file1']

and want to use file as input staging for a task. The task definition has to include the folder tree after $SHARED:

td.input_staging = ['$SHARED/folder1/file1']

@iparask
Copy link
Contributor Author

iparask commented Mar 19, 2021

ping @lee212

Copy link
Member

@andre-merzky andre-merzky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@iparask iparask merged commit 2b42c9a into devel Mar 30, 2021
@iparask iparask deleted the fix/issue_512 branch March 30, 2021 13:40
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.

Keeping directory structure in shared_data?
3 participants