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

APPS-1149 optional elements in structs #218

Merged
merged 12 commits into from
Apr 14, 2022

Conversation

YuxinShi0423
Copy link
Collaborator

@YuxinShi0423 YuxinShi0423 commented Apr 8, 2022

This PR is to fix the evaluation of structs as input parameters of workflow/scatter:

  • hash inputs should be coerced from object to struct

  • their optional elements should be assigned to Null if not specified in job inputs.

  • add tests

    • task struct input
    • workflow struct input
    • workflow struct input to scatter input

@YuxinShi0423
Copy link
Collaborator Author

All test WDL files I added can be compiled and executed successfully. Also all WDL integration tests with this version of wdlTools are passed. (CWL is not affected so I did not run the tests.)

Copy link
Contributor

@commandlinegirl commandlinegirl left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for working on that!

src/test/scala/wdlTools/exec/ExecTest.scala Outdated Show resolved Hide resolved
Copy link
Contributor

@commandlinegirl commandlinegirl left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for adding a release note!

@commandlinegirl
Copy link
Contributor

@YuxinShi0423 Are you planning to release a snapshot for wdlTools once this this merged and create a PR in dxCompiler to run integration tests on this?

@YuxinShi0423 YuxinShi0423 merged commit 18496e8 into develop Apr 14, 2022
@YuxinShi0423
Copy link
Collaborator Author

@commandlinegirl I have run the integration test with this snapshot. Shall I release a new version of wdlTools? Or is there a way to build a snapshot (but not release it) which dxCompiler can use?

@commandlinegirl
Copy link
Contributor

@commandlinegirl I have run the integration test with this snapshot. Shall I release a new version of wdlTools? Or is there a way to build a snapshot (but not release it) which dxCompiler can use?

You don't have to create a new full wdlTools release. You can run the "Publish snapshot" workflow from the Actions tab in the wdlTools repo:
image

And then use the released version (it will be 0.17.9-SNAPSHOT) in dxCompiler (it's already set to 0.17.9 as shown here).

@commandlinegirl
Copy link
Contributor

@YuxinShi0423 Oh, re my previous comment - I can see you already ran it!:) So now you'd just need to create PR in dxCompiler to run integration tests.

@commandlinegirl commandlinegirl deleted the APPS-1149-optional-elements-in-structs branch April 19, 2022 00:36
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.

None yet

2 participants