-
Notifications
You must be signed in to change notification settings - Fork 71
Added support for paused status in stages that are waiting for input #1068
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
base: main
Are you sure you want to change the base?
Conversation
src/main/frontend/pipeline-graph-view/pipeline-graph/main/PipelineGraphModel.tsx
Outdated
Show resolved
Hide resolved
src/main/java/io/jenkins/plugins/pipelinegraphview/utils/PipelineStage.java
Outdated
Show resolved
Hide resolved
src/main/java/io/jenkins/plugins/pipelinegraphview/utils/PipelineStage.java
Outdated
Show resolved
Hide resolved
|
Looks fine to me on a frontend POV now - any thoughts on Java @timja @lewisbirks |
timja
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey looks good couple of minor things, but close.
| List<PipelineStageInternal> stages = getPipelineNodes(builder); | ||
|
|
||
| // Set the builder on each stage so they can check for paused steps | ||
| if (builder instanceof PipelineStepBuilderApi stepBuilder) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A separate PR could refactor the need for this instanceof check pretty easily, there's only one implementation and the indirection is no longer needed
| if (children != null && !children.isEmpty()) { | ||
| for (PipelineStage child : children) { | ||
| if (child.state == PipelineState.PAUSED) { | ||
| return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we get some test coverage for this please?
A pipeline that sets an input step on it and validates the stage is now paused.
| this.agent = aAgent; | ||
| } | ||
|
|
||
| public void setBuilder(PipelineStepBuilderApi builder) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this could be avoided by doing the refactoring mentioned in PipelineGraphApi comment and then you could just pass the builder in the constructor (or another constructor) as I'd prefer not to be mutating state
| } | ||
|
|
||
| public PipelineStage toPipelineStage(List<PipelineStage> children, String runUrl) { | ||
| boolean waitingForInput = isWaitingForInput(children); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we only do this check if there is an InputAction attached to the WorkflowRun?
I think that should be optimisable so this check is only run over stages that could be waiting for input?
|
I didn't realize this PR was open when I created the following issues. |
|
It's a bit challenging to relay how good this would be. An example: you have a pipeline with 30 stages stacked in parallel. If only one stage is paused for input, then you must either wait for most of the stages to finish before you can answer (the "last" one running is input) or you need to manually open each of the 30 stages. The last one is a bit tedious to identify in the log steps there's a pause indicating input is required. |
😄, it should be in soon, only a bit of polish needed. |
|
Note: need to confirm if this fixes #348 |
|
I was going to test this with incrementals but incrementals apparently only supports Jenkins weekly releases. I get this error on LTS Jenkins Version 2.528.3 (note: I am samrocketman; this is my proprietary GitHub profile) |
|
It’s not that incrementals supports weekly’s it’s that there’s a wider design rework going on which this plugin is integrating with so has needed to depend on a more recent release in its latest versions. |
|
Sounds good I will test in the next LTS update if this PR is not merged by that time. I have a several non-prod test environments but at the moment we generally ship LTS (I could consider making updates to support weekly at some point but for now it's all revolving around LTS). |
|
I've made the requested changes, and manually validated via a test pipeline that functionality is still the same. |
Added support for a paused stages
Fixes #967
Testing done
Before:

Stage appears as running even though its waiting for input
After:

Stage appears as paused on Pipeline Overview and Console view
Stage appears as paused on Stages view

Including parallel branches

Submitter checklist