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

AWS-EFS postmapping fix #5522

Merged
merged 4 commits into from
Aug 21, 2020
Merged

AWS-EFS postmapping fix #5522

merged 4 commits into from
Aug 21, 2020

Conversation

nvanaja
Copy link
Collaborator

@nvanaja nvanaja commented May 26, 2020

Added AWS-EFS expression post mapping function so the output expressions with files with relative paths get mapped to the correct path.

@nvanaja
Copy link
Collaborator Author

nvanaja commented Jun 5, 2020

Hi,
Pl let me know how long it will take for this pull request to be reviewed?
Thanks,
Vanaja

@nvanaja
Copy link
Collaborator Author

nvanaja commented Jun 11, 2020

Hi,
Could some one please review this change? We need this fix.
Also, BUILD_TYPE=horicromtalDeadlock seems to fail. How do I fix that?
Thanks,
Vanaja

@cjllanwarne
Copy link
Contributor

hi @nvanaja - we discussed this internally and there was some thought that this PR might possibly be unnecessary now with the changes in Cromwell 52 (especially PRs #5468 and #5554).

Would you mind confirming that this change is still needed - and if so we'll look to have it reviewed as part of our current sprint?

Thanks!

@nvanaja
Copy link
Collaborator Author

nvanaja commented Jul 28, 2020

Hi @cjllanwarne Thanks for the update. I looked at #5468 and #5554 changes made. They all S3 related changes/enghancements. This fix is EFS specific and hence not addressed by the above two. So this change is needed.
Thanks,
Vanaja

Copy link
Contributor

@markjschreiber markjschreiber left a comment

Choose a reason for hiding this comment

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

Looks good from a code perspective. Would be good to see if the Cromwell team can support Centaur tests with this configuration. This would presumably require a different environment from the current AWS test environment which uses S3 rather than EFS.

@grsterin grsterin self-requested a review August 10, 2020 14:50
Copy link
Contributor

@grsterin grsterin left a comment

Choose a reason for hiding this comment

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

Hi @nvanaja. Please revert file permissions change for backend/src/main/scala/cromwell/backend/standard/StandardAsyncExecutionActor.scala

image

nvanaja pushed a commit to nvanaja/cromwell that referenced this pull request Aug 12, 2020
nvanaja pushed a commit to nvanaja/cromwell that referenced this pull request Aug 12, 2020
@nvanaja
Copy link
Collaborator Author

nvanaja commented Aug 12, 2020

Hi @nvanaja. Please revert file permissions change for backend/src/main/scala/cromwell/backend/standard/StandardAsyncExecutionActor.scala

image

Done.

@grsterin grsterin merged commit 894a075 into broadinstitute:develop Aug 21, 2020
mcovarr pushed a commit that referenced this pull request Aug 24, 2020
nvanaja pushed a commit to nvanaja/cromwell that referenced this pull request Nov 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants