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

WX-757 Fix stdout, stderr in workflow body causing crashes #7386

Merged
merged 9 commits into from
Mar 14, 2024
Merged

WX-757 Fix stdout, stderr in workflow body causing crashes #7386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5fe626a
Fix `stdout`, `stderr`
aednichols Mar 12, 2024
b627704
Import niceties
aednichols Mar 12, 2024
9ba6d85
Add test
aednichols Mar 12, 2024
1e49941
Merge branch 'develop' into aen_wx_757_2
aednichols Mar 12, 2024
c7f4018
Deflake test?
aednichols Mar 13, 2024
483c15f
Merge remote-tracking branch 'origin/aen_wx_757_2' into aen_wx_757_2
aednichols Mar 13, 2024
4303a3b
Merge branch 'develop' into aen_wx_757_2
aednichols Mar 13, 2024
71a4938
Merge branch 'develop' into aen_wx_757_2
aednichols Mar 14, 2024
39502dc
Merge branch 'develop' into aen_wx_757_2
aednichols Mar 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions ...es/standardTestCases/failures/stderr_stdout_workflow_body/stderr_stdout_workflow_body.wdl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
version 1.0

workflow break_with_stderr {

output {
File load_data_csv = select_first([stdout(), stderr()])
}
}
13 changes: 13 additions & 0 deletions centaur/src/main/resources/standardTestCases/stderr_stdout_workflow_body.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
name: stderr_stdout_workflow_body
testFormat: workflowfailure

files {
workflow: failures/stderr_stdout_workflow_body/stderr_stdout_workflow_body.wdl
}

metadata {
workflowName: break_with_stderr
status: Failed
"failures.0.message": "Workflow failed"
"failures.0.causedBy.0.message": "Failed to evaluate 'break_with_stderr.load_data_csv' (reason 1 of 2): Evaluating select_first([stdout(), stderr()]) failed: stdout is not implemented at the workflow level, Failed to evaluate 'break_with_stderr.load_data_csv' (reason 2 of 2): Evaluating select_first([stdout(), stderr()]) failed: stderr is not implemented at the workflow level"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I really appreciate how our error handling reports both errors on the same line!

}
4 changes: 2 additions & 2 deletions ...c/main/scala/wdl/transforms/base/linking/expression/values/EngineFunctionEvaluators.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
ioFunctionSet: IoFunctionSet,
forCommandInstantiationOptions: Option[ForCommandInstantiationOptions]
)(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomSingleFile]] =
EvaluatedValue(WomSingleFile(ioFunctionSet.pathFunctions.stdout), Seq.empty).validNel
ErrorOr(EvaluatedValue(WomSingleFile(ioFunctionSet.pathFunctions.stdout), Seq.empty))

Check warning on line 54 in wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/values/EngineFunctionEvaluators.scala

View check run for this annotation

Codecov / codecov/patch

wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/values/EngineFunctionEvaluators.scala#L54 

Added line #L54 was not covered by tests
}

implicit val stderrFunctionEvaluator: ValueEvaluator[StderrElement.type] = new ValueEvaluator[StderrElement.type] {
Expand All @@ -60,7 +60,7 @@
ioFunctionSet: IoFunctionSet,
forCommandInstantiationOptions: Option[ForCommandInstantiationOptions]
)(implicit expressionValueEvaluator: ValueEvaluator[ExpressionElement]): ErrorOr[EvaluatedValue[WomSingleFile]] =
EvaluatedValue(WomSingleFile(ioFunctionSet.pathFunctions.stderr), Seq.empty).validNel
ErrorOr(EvaluatedValue(WomSingleFile(ioFunctionSet.pathFunctions.stderr), Seq.empty))

Check warning on line 63 in wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/values/EngineFunctionEvaluators.scala

View check run for this annotation

Codecov / codecov/patch

wdl/transforms/new-base/src/main/scala/wdl/transforms/base/linking/expression/values/EngineFunctionEvaluators.scala#L63 

Added line #L63 was not covered by tests
}

private val ReadWaitTimeout = 60.seconds
Expand Down
Loading