feat(gui): gate workflow execution on the compilation state - #7983
Open
kz930 wants to merge 1 commit into
Open
Conversation
The run button's enabled state came from ValidationWorkflowService alone, which checks an operator's properties against its JSON schema and whether every input port is connected. Neither reads the compiling service, so a workflow whose operators fail to compile was still offered for execution, and pressing run started an execution that reported the same compilation error in the console. Subscribe the menu to the compiling service's state stream and treat a failed compilation the same way as a validation error: the button reads "Invalid Workflow" and is disabled, and runWorkflow returns early. An HTTP failure of the compile endpoint yields no state change rather than a failed one, so a compiling service that is down cannot disable the button. The context menu's "execute to this operator" runs the target together with everything upstream of it, so it is disabled when any operator in the target's sub-DAG failed to compile, not only the target itself. Closes apache#7981 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7983 +/- ##
============================================
- Coverage 93.14% 93.14% -0.01%
Complexity 4654 4654
============================================
Files 1174 1174
Lines 47580 47592 +12
Branches 5316 5316
============================================
+ Hits 44318 44329 +11
Misses 1785 1785
- Partials 1477 1478 +1
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
/request-review @aglinxinyuan @carloea2 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this PR?
The run button's enabled state comes from
ValidationWorkflowServicealone. That service checks two things per operator: the operator's properties against its JSON schema with ajv, and whether every input port is connected. Neither reads the compiling service, so a workflow whose operators fail to compile is still offered for execution, and pressing run starts an execution that reports the same compilation error in the console.CompilationState.Failedreaches the frontend but its only consumer outside the compiling service decides whether the error panel is shown.This PR subscribes the menu to the compiling service's state stream and treats a failed compilation the same way as a validation error. The button reads "Invalid Workflow" and is disabled, and
runWorkflowreturns early on the same flag. Reusing the existing text and icon keeps one code path for "this workflow is not runnable", and the operator's red cross and the error panel already say which operator is at fault and why.Two notes on the shape of the guard. An HTTP failure of the compile endpoint yields no state change rather than a failed one, so a compiling service that is down cannot brick the button. And compilation is debounced, so the button flips a moment after an edit, which is already true of the red cross and the error panel.
The context menu's "execute to this operator" gets the same guard. Because that entry runs the target together with everything upstream of it, it checks the target's sub-DAG against the compilation error map rather than only the target operator. An error elsewhere in the workflow leaves the entry enabled for an operator whose own upstream compiles.
Any related issues, documentation, discussions?
Closes #7981. Also covers the context menu entry that #3068 asked about.
How was this PR tested?
Six unit tests were added. In the menu spec: the run descriptor when the workflow does not compile,
runWorkflowdeclining to submit on the same flag, and the stream wiring in both directions. In the context menu spec: the target operator failing to compile, an upstream operator failing to compile, and a compilation error outside the target's sub-DAG leaving the entry enabled. The compiling service is stubbed in the menu spec so its debounced compile request does not outlive the test injector.The two specs run 146 tests and pass.
Manually verified against the reproduction in the issue, on a CSV with a text column and a numeric label, wired to Gaussian Naive Bayes on both the training and the testing port. With Count Vectorizer on and Text Attribute empty the button reads "Invalid Workflow" from ajv, as before. Filling Text Attribute in satisfies ajv, and the button now stays disabled instead of turning into Run, with the workflow reported as valid but not compilable. The context menu entry is disabled on the Gaussian Naive Bayes operator and still enabled on the upstream CSV File Scan. Turning Count Vectorizer back off returns the button to Run.
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)