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.

Sign up for GitHub

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

Jump to bottom

fix: handle input dependencies in workflow graph evaluation #360

Merged
merged 2 commits into from
Mar 19, 2025
Merged

fix: handle input dependencies in workflow graph evaluation #360

merged 2 commits into from
Mar 19, 2025
+153 −7

Conversation

dead8309
Copy link
Contributor

@dead8309 dead8309 commented Mar 18, 2025 •
edited
Loading

This pull request fixes an issue in wdl-engine where workflow input dependencies were not being handled correctly during graph evaluation.

Before submitting this PR, please make sure:

  • You have added a few sentences describing the PR here.
  • You have added yourself or the appropriate individual as the assignee.
  • You have added at least one relevant code reviewer to the PR.
  • Your code builds clean without any errors or warnings.
  • You have added tests (when appropriate).
  • You have updated the README or other documentation to account for these
    changes (when appropriate).
  • You have added an entry to the relevant CHANGELOG.md (see
    "keep a changelog" for more information).
  • Your commit messages follow the conventional commit style.
  • Your PR title follows the conventional commit style.

Closes: #356

Sorry, something went wrong.

@dead8309
Copy link
Contributor Author

squashing..

@dead8309 dead8309 force-pushed the fix/workflow-input-deps branch from c59d51e to 9285eb3 Compare March 18, 2025 23:31
@peterhuene peterhuene changed the title fix(wdl-engine): handle input dependencies in workflow graph evaluation fix: handle input dependencies in workflow graph evaluation Mar 19, 2025
@peterhuene peterhuene self-requested a review March 19, 2025 02:39
@peterhuene peterhuene self-assigned this Mar 19, 2025
peterhuene
peterhuene approved these changes Mar 19, 2025
View reviewed changes
Copy link
Collaborator

@peterhuene peterhuene left a comment

Choose a reason for hiding this comment

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

@dead8309 thank you very much for this contribution!

The fix looks good and I tested it out locally to run and as expected the tasks run in series when the input is not supplied and parallel when supplied 👍

@peterhuene
Copy link
Collaborator

peterhuene commented Mar 19, 2025 •
edited
Loading

Looks like you'll also need to run cargo +nightly fmt (you'll need a nightly toolset installed if you don't have one).

@dead8309 dead8309 force-pushed the fix/workflow-input-deps branch from 9285eb3 to 275c795 Compare March 19, 2025 04:13
@dead8309
Copy link
Contributor Author

done!!

dead8309 added 2 commits March 19, 2025 09:54
@dead8309
fix(wdl-engine): handle input dependencies in workflow graph evaluation
6ef392e 
- Skip dependencies for provided inputs
- Ensure tasks can run in parallel when inputs are provided
@dead8309
test(wdl-engine): add unit test for workflow graph input dependencies
Loading
Loading status checks…
5927e6a 
- Add sample test from stjude-rust-labs#356
- fix typo
- remove debugging logs
- update changelog
- format files
- remove unncecessary to_string() calls
@dead8309 dead8309 force-pushed the fix/workflow-input-deps branch from 275c795 to 5927e6a Compare March 19, 2025 04:26
@dead8309
Copy link
Contributor Author

rebased

peterhuene
peterhuene approved these changes Mar 19, 2025
View reviewed changes
Copy link
Collaborator

@peterhuene peterhuene left a comment

Choose a reason for hiding this comment

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

@dead8309 Thanks again for fixing this!

@peterhuene peterhuene merged commit d8950c1 into stjude-rust-labs:main Mar 19, 2025
17 checks passed
Gyan-max pushed a commit to Gyan-max/wdl that referenced this pull request Mar 22, 2025
@dead8309 @Gyan-max
fix: handle input dependencies in workflow graph evaluation (stjude-r…
885d0e3 
…ust-labs#360)
a-frantz pushed a commit to Gyan-max/wdl that referenced this pull request Mar 22, 2025
@dead8309 @a-frantz
fix: handle input dependencies in workflow graph evaluation (stjude-r…
c6ef656 
…ust-labs#360)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peterhuene peterhuene

Assignees

@peterhuene peterhuene

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Workflow evaluation graphs do not take into account workflow inputs
2 participants
@dead8309 @peterhuene