Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-38223

FlowNode.isRunning is not very useful

XMLWordPrintable

      Give following pipeline script, b1 branch finishes immediately and b2 is not completed yet (waiting for input). I determine branch's completion from a branch's end FlowNode and calling FlowNode.isRunning() returns true instead false as its already completed. This makes BlueOcean API returns b1 as running whereas its already been completed.

      node {
    stage("hey") {
        echo "hello from hey"
    }
    
    stage("par") {
        parallel (
            "b1" : {
                echo 'hello from b1'
            },

            "b2" : {
                def branchInput = input message: 'Please input branch to test against', parameters: [[$class: 'StringParameterDefinition', defaultValue: 'master', description: '', name: 'branch']]
                echo "BRANCH NAME: ${branchInput}"
            }             
        )            
    }
       
    stage("ho") {
        echo "hello from ho"
    }
    
}

          [JENKINS-38223] FlowNode.isRunning is not very useful

          Vivek Pandey created issue -

          Jesse Glick added a comment -

          Javadoc does claim

          be false for something that has finished but is pending child node creation, such as a completed fork branch which is waiting for the join node to be created

          but I can see no way that could ever have been implemented, since the BlockEndNode for parallel is not constructed until after all branches have completed. So I guess the Javadoc needs to be edited to stop making that claim.

          The use case you cite makes no sense. If the branch has a BlockEndNode, then by definition it has completed.

          Jesse Glick added a comment - Javadoc does claim be false for something that has finished but is pending child node creation, such as a completed fork branch which is waiting for the join node to be created but I can see no way that could ever have been implemented, since the BlockEndNode for parallel is not constructed until after all branches have completed. So I guess the Javadoc needs to be edited to stop making that claim. The use case you cite makes no sense. If the branch has a BlockEndNode , then by definition it has completed.

          Vivek Pandey added a comment -

          jglick

          >but I can see no way that could ever have been implemented, since the BlockEndNode for parallel is not constructed until after all branches have completed. So I guess the Javadoc needs to be edited to stop making that claim.

          Hmm, thats the bug, I have BlockEndNode for that parallel branch, very fact that its been already constructed then it should be returning false for isRunning(), no? I am not calling isRunning() on a step inside parallel branch, its on the BlockEndNode of that branch.

          >The use case you cite makes no sense. If the branch has a BlockEndNode, then by definition it has completed.

          Rather than asking caller to infer that all has ended since you have this end block, which is true but 'isRunning() API' makes no sense to return true in this case as well.

          Vivek Pandey added a comment - jglick >but I can see no way that could ever have been implemented, since the BlockEndNode for parallel is not constructed until after all branches have completed. So I guess the Javadoc needs to be edited to stop making that claim. Hmm, thats the bug, I have BlockEndNode for that parallel branch, very fact that its been already constructed then it should be returning false for isRunning(), no? I am not calling isRunning() on a step inside parallel branch, its on the BlockEndNode of that branch. >The use case you cite makes no sense. If the branch has a BlockEndNode, then by definition it has completed. Rather than asking caller to infer that all has ended since you have this end block, which is true but 'isRunning() API' makes no sense to return true in this case as well.

          Sam Van Oort added a comment -

          > Rather than asking caller to infer that all has ended since you have this end block, which is true but 'isRunning() API' makes no sense to return true in this case as well.

          Yeah, it's a clear bug in workflow-cps here. I think fixable, by looking more closely at cases with > 1 currentHead (and then seeing if it's the end of one of the currently executing branch). May be something I can fix as a small one-off fix (probably needs a change in workflow-api and workflow-cps though, due to spec).

          In pipeline-graph-analysis I use a bit of a deeper inspection to work around this, by looking to see if the node is the BlockEndNode for a BlockStartNode which is a parallel branch start.

          Sam Van Oort added a comment - > Rather than asking caller to infer that all has ended since you have this end block, which is true but 'isRunning() API' makes no sense to return true in this case as well. Yeah, it's a clear bug in workflow-cps here. I think fixable, by looking more closely at cases with > 1 currentHead (and then seeing if it's the end of one of the currently executing branch). May be something I can fix as a small one-off fix (probably needs a change in workflow-api and workflow-cps though, due to spec). In pipeline-graph-analysis I use a bit of a deeper inspection to work around this, by looking to see if the node is the BlockEndNode for a BlockStartNode which is a parallel branch start.

          Michael Neale added a comment -

          vivek so is this fix needed or is it more a temporary thing for the current blue ocean code and should go away when bismuth is used instead?

          Michael Neale added a comment - vivek so is this fix needed or is it more a temporary thing for the current blue ocean code and should go away when bismuth is used instead?

          Sam Van Oort added a comment -

          michaelneale Bismuth doesn't suffer this issue, but it has a kind of filthy hack in place for handling parallels unfortunately (I'd like to be able to drop that).

          Sam Van Oort added a comment - michaelneale Bismuth doesn't suffer this issue, but it has a kind of filthy hack in place for handling parallels unfortunately (I'd like to be able to drop that).
          Michael Neale made changes -
          Link New: This issue blocks JENKINS-38398 [ JENKINS-38398 ]
          James Dumay made changes -
          Link Original: This issue blocks JENKINS-38398 [ JENKINS-38398 ]
          James Dumay made changes -
          Link New: This issue relates to JENKINS-38398 [ JENKINS-38398 ]

          Jesse Glick added a comment -

          I still think the method is behaving according to spec—it is just not a useful spec. When the branch has completed, a BlockEndNode is created and becomes one of the heads of the flow graph. Thus it isRunning() according to the definition.

          The spec will not be changed (that would not be compatible), so the most that would be done is to deprecate the method, and possibly introduce a new method with a different semantics: example

          Jesse Glick added a comment - I still think the method is behaving according to spec—it is just not a useful spec. When the branch has completed, a BlockEndNode is created and becomes one of the heads of the flow graph. Thus it isRunning() according to the definition. The spec will not be changed (that would not be compatible), so the most that would be done is to deprecate the method, and possibly introduce a new method with a different semantics: example

            jglick Jesse Glick
            vivek Vivek Pandey
            Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

              Created:
              Updated:
              Resolved: