[JIRA] (JENKINS-52189) GraphListener does not receive FlowStartNode

4 views
Skip to first unread message

jenkins@t-8ch.de (JIRA)

unread,
Jun 26, 2018, 9:47:03 AM6/26/18
to jenkinsc...@googlegroups.com
Thomas Weißschuh created an issue
 
Jenkins / Bug JENKINS-52189
GraphListener does not receive FlowStartNode
Issue Type: Bug Bug
Assignee: Unassigned
Components: workflow-cps-plugin
Created: 2018-06-26 13:46
Priority: Minor Minor
Reporter: Thomas Weißschuh

Classes implementing org.jenkinsci.plugins.workflow.flow.GraphListener are not informed about FlowStartNode s at execution start.

All other types of FlowNode are handled.

(Including the matching FlowEndNode)
Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v7.10.1#710002-sha1:6efc396)

svanoort@cloudbees.com (JIRA)

unread,
Jul 2, 2018, 9:51:02 AM7/2/18
to jenkinsc...@googlegroups.com
Sam Van Oort started work on Bug JENKINS-52189
 
Change By: Sam Van Oort
Status: Open In Progress

svanoort@cloudbees.com (JIRA)

unread,
Jul 2, 2018, 9:51:02 AM7/2/18
to jenkinsc...@googlegroups.com
Sam Van Oort assigned an issue to Sam Van Oort
Change By: Sam Van Oort
Assignee: Sam Van Oort

svanoort@cloudbees.com (JIRA)

unread,
Jul 2, 2018, 9:52:02 AM7/2/18
to jenkinsc...@googlegroups.com

dnusbaum@cloudbees.com (JIRA)

unread,
Feb 1, 2019, 4:59:02 PM2/1/19
to jenkinsc...@googlegroups.com
Devin Nusbaum resolved as Fixed
 

A fix for this issue was released in version 2.63 of Pipeline Groovy Plugin.
Change By: Devin Nusbaum
Status: In Progress Resolved
Resolution: Fixed
Released As: workflow-cps 2.63
This message was sent by Atlassian Jira (v7.11.2#711002-sha1:fdc329d)

jenkins@t-8ch.de (JIRA)

unread,
Feb 4, 2019, 9:59:02 AM2/4/19
to jenkinsc...@googlegroups.com
Thomas Weißschuh commented on Bug JENKINS-52189
 
Re: GraphListener does not receive FlowStartNode

Another point (also raised on the PR):

Should it be possible to use the instantiation based usage of GraphListener (as
explained by its javadoc?

Something like the follow test:

   @Issue("JENKINS-52189")
    @Test
    public void notifyFlowStartNodeViaFlowExecutionListener() {
        story.then(s->{
            WorkflowJob j = jenkins().createProject(WorkflowJob.class, "bob");
            j.setDefinition(new CpsFlowDefinition("echo 'I did a thing'", true));
            WorkflowRun r = story.j.buildAndAssertSuccess(j);
            FlowStartNodeFlowExectionListener listener = jenkins().getExtensionList(
                FlowStartNodeFlowExectionListener.class).get(0);
            assertThat(listener.heads, Matchers.greaterThan(1));
            assertThat(listener.execNames, Matchers.contains(r.getExecution().toString()));
        });
    }

    @TestExtension("notifyFlowStartNodeViaFlowExecutionListener")
    public static class FlowStartNodeFlowExectionListener extends FlowExecutionListener {
        int heads = 0;
        final List<String> execNames = new ArrayList<>();

        @Override
        public void onRunning(@Nonnull FlowExecution execution) {
            execution.addListener(node -> {
                heads++;
                if (node instanceof FlowStartNode) {
                    execNames.add(node.getExecution().toString());
                }
            });
        }
    }

jenkins@t-8ch.de (JIRA)

unread,
Feb 13, 2019, 11:00:02 AM2/13/19
to jenkinsc...@googlegroups.com
Thomas Weißschuh edited a comment on Bug JENKINS-52189
Another point (also raised on the PR):

Should it be possible to use the instantiation based usage of GraphListener (as
explained by its javadoc?

Something like the follow following test:

{code}

   @Issue("JENKINS-52189")
    @Test
    public void notifyFlowStartNodeViaFlowExecutionListener() {
        story.then(s->{
            WorkflowJob j = jenkins().createProject(WorkflowJob.class, "bob");
            j.setDefinition(new CpsFlowDefinition("echo 'I did a thing'", true));
            WorkflowRun r = story.j.buildAndAssertSuccess(j);
            FlowStartNodeFlowExectionListener listener = jenkins().getExtensionList(
                FlowStartNodeFlowExectionListener.class).get(0);
            assertThat(listener.heads, Matchers.greaterThan(1));
            assertThat(listener.execNames, Matchers.contains(r.getExecution().toString()));
        });
    }

    @TestExtension("notifyFlowStartNodeViaFlowExecutionListener")
    public static class FlowStartNodeFlowExectionListener extends FlowExecutionListener {
        int heads = 0;
        final List<String> execNames = new ArrayList<>();

        @Override
        public void onRunning(@Nonnull FlowExecution execution) {
            execution.addListener(node -> {
                heads++;
                if (node instanceof FlowStartNode) {
                    execNames.add(node.getExecution().toString());
                }
            });
        }
    }
{code}

jenkins@t-8ch.de (JIRA)

unread,
Feb 13, 2019, 11:00:02 AM2/13/19
to jenkinsc...@googlegroups.com
Thomas Weißschuh reopened an issue
 
Change By: Thomas Weißschuh
Resolution: Fixed
Status: Resolved Reopened
Assignee: Sam Van Oort Devin Nusbaum

dnusbaum@cloudbees.com (JIRA)

unread,
Feb 13, 2019, 11:18:02 AM2/13/19
to jenkinsc...@googlegroups.com
Devin Nusbaum commented on Bug JENKINS-52189
 
Re: GraphListener does not receive FlowStartNode

Thomas Weißschuh I don't know. If you have a use case where you need to use FlowExecutionListener.onRunning to add a listener and cannot use a global extension, then perhaps better to file a separate bug for that specific case. Otherwise, maybe we can just update the docs for FlowExecutionListener#onRunning to mention that if you add a listener in that method it will not see FlowStartNodes.

jenkins@t-8ch.de (JIRA)

unread,
Feb 13, 2019, 11:39:02 AM2/13/19
to jenkinsc...@googlegroups.com

My current code is nicer to implement with customly created listeners. I can rewrite it to use the global one but wanted to ask before, as maybe it should actually work.

My FlowExecutionListener creates a GraphListener for every new Flow (and stores it).
The following snippet works around the currently missing functionality by triggering the creation of the GraphListener "manually":

import hudson.Extension;
import org.jenkinsci.plugins.workflow.flow.GraphListener;
import org.jenkinsci.plugins.workflow.graph.FlowNode;
import org.jenkinsci.plugins.workflow.graph.FlowStartNode;

/*
 * This class works around https://issues.jenkins-ci.org/browse/JENKINS-52189
 * Especially the part where GraphListeners attached during FlowExecutionListener.onRunning() do not receive the FlowStartNode.
 * As the per instance logic is nicer to reason about and may work properly in a future version of Jenkins,
 * this class will be the bridge until then.
 */
@Extension
public class WorkaroundGraphListener implements GraphListener, GraphListener.Synchronous {

    @Override
    public void onNewHead(FlowNode node) {
        final OTFlowExecutionListener flowExecutionListener = OTFlowExecutionListener.get();
        if (node instanceof FlowStartNode) {
            flowExecutionListener.getListener(node.getExecution()).onNewHead(node);
        }
    }
}

dnusbaum@cloudbees.com (JIRA)

unread,
Feb 13, 2019, 11:46:01 AM2/13/19
to jenkinsc...@googlegroups.com

Thomas Weißschuh If you can figure out a clean way to make it work and submit a PR, then I'd be happy to review it. I don't think there is any reason why we wouldn't want it to work, just that the most obvious implementation happens to not work for that use case.

allan.lewis@youview.com (JIRA)

unread,
Feb 13, 2019, 11:58:02 AM2/13/19
to jenkinsc...@googlegroups.com
Allan Lewis assigned an issue to Allan Lewis
 
Change By: Allan Lewis
Assignee: Devin Nusbaum Allan Lewis

jenkins@t-8ch.de (JIRA)

unread,
Feb 13, 2019, 12:04:03 PM2/13/19
to jenkinsc...@googlegroups.com
Thomas Weißschuh commented on Bug JENKINS-52189
 
Re: GraphListener does not receive FlowStartNode

Allan Lewis you assigned this to yourself, are you planning on working on this?

Otherwise I may find the time to take a shot at it.

allan.lewis@youview.com (JIRA)

unread,
Feb 13, 2019, 12:09:02 PM2/13/19
to jenkinsc...@googlegroups.com
Allan Lewis assigned an issue to Sam Van Oort
 

Sorry, Thomas Weißschuh, I did that by mistake! I probably pressed 'i' while this ticket was focused.
Change By: Allan Lewis
Assignee: Allan Lewis Sam Van Oort

jenkins@t-8ch.de (JIRA)

unread,
Feb 14, 2019, 2:30:02 AM2/14/19
to jenkinsc...@googlegroups.com
 
Re: GraphListener does not receive FlowStartNode

Devin Nusbaum so the easy way would be to just fire the listeners before the execution
This however would break the behaviour of the `FlowExecution.onRunning()` method which is documented to receive an already started listener.
(The testsuite however passes)

We could also introduce a new method `FlowExecution.beforeRunning()` (or `onCreated()`) that fires before the flow is actually started.

WDYT?

jenkins@t-8ch.de (JIRA)

unread,
Apr 24, 2019, 8:13:03 AM4/24/19
to jenkinsc...@googlegroups.com
Thomas Weißschuh assigned an issue to Devin Nusbaum
 
Change By: Thomas Weißschuh
Assignee: Sam Van Oort Devin Nusbaum

dnusbaum@cloudbees.com (JIRA)

unread,
Apr 24, 2019, 9:57:02 AM4/24/19
to jenkinsc...@googlegroups.com
Devin Nusbaum assigned an issue to Unassigned
Change By: Devin Nusbaum
Assignee: Devin Nusbaum

dnusbaum@cloudbees.com (JIRA)

unread,
Apr 24, 2019, 9:57:03 AM4/24/19
to jenkinsc...@googlegroups.com
Devin Nusbaum commented on Bug JENKINS-52189
 
Re: GraphListener does not receive FlowStartNode

Thomas Weißschuh I guess introducing a new method `onCreated()` seems better in case some code in the wild relies on the documented behavior. Feel free to file a PR, ideally linked to an update to some other plugin that wants to use the new method.

jenkins@t-8ch.de (JIRA)

unread,
Apr 25, 2019, 6:30:02 AM4/25/19
to jenkinsc...@googlegroups.com

Devin Nusbaum: https://github.com/jenkinsci/workflow-job-plugin/pull/129
Unfortunately the plugin that needs it is not (yet?) public.

m.winter@sap.com (JIRA)

unread,
Aug 1, 2019, 8:54:02 AM8/1/19
to jenkinsc...@googlegroups.com

Having the same issue with the missed FlowStartNode

Any idea when the fixes will be released?

dnusbaum@cloudbees.com (JIRA)

unread,
Aug 21, 2019, 5:31:03 PM8/21/19
to jenkinsc...@googlegroups.com
Devin Nusbaum resolved as Fixed
 

The new FlowExecutionListener.onCreated method was released in Pipeline API Plugin 2.36, and is activated by Pipeline Job Plugin 2.34.
Change By: Devin Nusbaum
Status: Reopened Resolved
Assignee: Thomas Weißschuh
Resolution: Fixed
Released As: workflow-cps 2.63 , workflow-api 2.36, workflow-job 2.34
Reply all
Reply to author
Forward
0 new messages