Skip to content

Shift async_driver gear into park if exception from workflow graph#269

Merged
knoepfel merged 2 commits intoFramework-R-D:mainfrom
knoepfel:stop-driver
Jan 28, 2026
Merged

Shift async_driver gear into park if exception from workflow graph#269
knoepfel merged 2 commits intoFramework-R-D:mainfrom
knoepfel:stop-driver

Conversation

@knoepfel
Copy link
Copy Markdown
Member

This PR removes a stall that could occur if an exception is triggered in the workflow portion of the framework graph.

Copy link
Copy Markdown
Contributor

Is the FIXME something that should be fixed here, or have an issue for addressing a little later?

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

❌ Your project status has failed because the head coverage (74.93%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

@@            Coverage Diff             @@
##             main     #269      +/-   ##
==========================================
- Coverage   75.87%   74.93%   -0.95%     
==========================================
  Files         124      124              
  Lines        2898     2916      +18     
  Branches      506      513       +7     
==========================================
- Hits         2199     2185      -14     
- Misses        483      508      +25     
- Partials      216      223       +7     
Flag Coverage Δ
unittests 74.93% <100.00%> (-0.95%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
phlex/core/framework_graph.cpp 89.24% <100.00%> (+0.87%) ⬆️
phlex/core/framework_graph.hpp 100.00% <ø> (ø)
phlex/driver.hpp 100.00% <ø> (ø)
phlex/utilities/async_driver.hpp 100.00% <100.00%> (ø)

... and 17 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a46fa3...cdb6cfb. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@knoepfel
Copy link
Copy Markdown
Member Author

Is the FIXME something that should be fixed here, or have an issue for addressing a little later?

It's not clear enough to me what the ramifications are of short-circuiting the logic or leaving it. The FIXME implies that it should be fixed...but I actually don't know if it should. Maybe I'll just adjust the comment...?

@greenc-FNAL
Copy link
Copy Markdown
Contributor

Does this resolve the remaining segfault issue in #213 or is that still not understood?

@knoepfel
Copy link
Copy Markdown
Member Author

knoepfel commented Jan 28, 2026

Does this resolve the remaining segfault issue in #213 or is that still not understood?

Yes, I believe it fixes the segfault issue in #213. (My local checkout of #213 with this PR's changes applied did not yield any stalled or segfaulting jobs when using ctest -j24, the maximum allowed parallelism on the system I was using.)

@knoepfel
Copy link
Copy Markdown
Member Author

@greenc-FNAL, I adjusted the comment and fixed some of the waiting logic...it was more complicated than it needed to be.

@knoepfel knoepfel requested a review from greenc-FNAL January 28, 2026 21:30
@knoepfel knoepfel merged commit 1455155 into Framework-R-D:main Jan 28, 2026
35 of 36 checks passed
@knoepfel knoepfel deleted the stop-driver branch January 28, 2026 21:41
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.

2 participants