blueocean-plugin icon indicating copy to clipboard operation
blueocean-plugin copied to clipboard

Delete `DownstreamJobListener` and instead create `NodeDownstreamBuildAction` dynamically during graph processing based on `DownstreamBuildAction`

Open dwnusbaum opened this issue 1 year ago • 6 comments

DownstreamJobListener here has the same bug as https://github.com/jenkinsci/pipeline-build-step-plugin/pull/127 for completed builds. Prior to https://github.com/jenkinsci/workflow-cps-plugin/pull/807, this only worked as long as you were using the MAX_SURVIVABILITY durability level, otherwise it did nothing when the build was already complete. https://github.com/jenkinsci/workflow-cps-plugin/pull/826 then made it so a warning was logged instead of the save being silently ignored in the problematic case, which can lead to log spam.

Untested, because right now Blue Ocean doesn't build for me (I think the main problem is Cannot download "https://github.com/sass/node-sass/releases/download/v7.0.0/darwin-x64-64_binding.node": when trying to build the frontend code, and IDK if there are any releases that support ARM-based Macs: https://github.com/sass/node-sass/issues/3033).

Submitter checklist

  • [ ] Link to JIRA ticket in description, if appropriate.
  • [ ] Change is code complete and matches issue description
  • [ ] Appropriate unit or acceptance tests or explanation to why this change has no tests
  • [ ] Reviewer's manual test instructions provided in PR description. See Reviewer's first task below.

Reviewer checklist

  • [ ] Run the changes and verified the change matches the issue description
  • [ ] Reviewed the code
  • [ ] Verified that the appropriate tests have been written or valid explanation given

dwnusbaum avatar Feb 01 '24 17:02 dwnusbaum

https://github.com/jenkinsci/blueocean-plugin/blob/a910c5348296bac732149eb7ca4e07d46eec4874/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/PipelineNodeImpl.java#L191-L192 + https://github.com/jenkinsci/blueocean-plugin/blob/a910c5348296bac732149eb7ca4e07d46eec4874/blueocean-dashboard/src/main/js/components/karaoke/components/Pipeline.jsx#L312-L314 IIUC

jglick avatar Feb 01 '24 18:02 jglick

Can we not just delete (or deprecate) NodeDownstreamBuildAction and change whatever code is calling is to look up org.jenkinsci.plugins.workflow.support.steps.build.DownstreamBuildAction on demand?

Yes I considered this kind of approach, but I think it would only move the performance issue to graph loading time (a bit better at least since it wouldn't affect non-Blue Ocean use cases). I can switch to it though.

dwnusbaum avatar Feb 01 '24 18:02 dwnusbaum

Oh, and I'm not sure if you can just change the code that looks for the action in the special Blue Ocean FlowNodeWrapper class in PipelineNodeImpl. I think you need to change the code that sets the actions when the graph is being processed so that the action propagates up to the node that is visible in Blue Ocean correctly.

dwnusbaum avatar Feb 01 '24 18:02 dwnusbaum

See https://github.com/jenkinsci/blueocean-plugin/pull/2540/commits/b7ef6079aa57050d441d2fd9097a7b5e4d0f1970 for an approach that moves the logic to graph processing time.

Delaying build loading until a user actually clicks on one of the relevant links does not seem possible without UX changes, since we expect the description of the run to be available in the UI.

dwnusbaum avatar Feb 01 '24 19:02 dwnusbaum

it wouldn't affect non-Blue Ocean use cases

Which is pretty important I think, since B.O. might be installed yet rarely used.

jglick avatar Feb 01 '24 20:02 jglick

Blue Ocean doesn't build for me

If you do not need to test the GUI interactively, you can just use https://github.com/eirslett/frontend-maven-plugin?tab=readme-ov-file#skipping-execution (which will be a lot faster too).

jglick avatar Feb 07 '24 18:02 jglick