Implement workflow support - #10
Conversation
…ist of developers.
Fix configuration persistence
Change away from DataBoundConstructor (the empty default is still needed) to DataBoundSetters to allow both multiple pools and plain ports in a single wrap. The legacy single pool variant is still supported.
The serialize inner class cases are just suppressed for now.
|
@oldelvet @pepov Would it be possible for you to review this one for merge into the main master? I see that one of the forks appears to have an integrated version and users claim a version running on their master in https://issues.jenkins-ci.org/browse/JENKINS-31449 |
|
Sorry I'm not active anymore in this project and don't have the time right now to get up to speed again. |
| private static final long serialVersionUID = 1L; | ||
|
|
||
| @Override | ||
| public void checkRoles(RoleChecker roleChecker) throws SecurityException { |
There was a problem hiding this comment.
Do not do this! I think you meant to just extend MasterToSlaveCallable.
| } | ||
|
|
||
| @DataBoundSetter | ||
| public void setPools(String[] pools) { |
There was a problem hiding this comment.
Why two attributes? Delete setPool.
| public void setPlainports(String[] plainports) { | ||
| if (plainports != null) { | ||
| for (String port : plainports) { | ||
| this.ports.add(new DefaultPortType(port)); |
There was a problem hiding this comment.
Seems like you really wanted to have a List<PortType> attribute, with each type being a Describable with its own Descriptor, config.jelly, and @Symbol.
| public class PortAllocationWorkflowTest { | ||
|
|
||
| @Rule | ||
| public JenkinsRule j = new JenkinsRule(); |
There was a problem hiding this comment.
To meaningfully test anything relating to serialization, you will need to use RestartableJenkinsRule.
I think you should be looking at the xvnc plugin for examples of analogous work.
| WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p"); | ||
| p.setDefinition(new CpsFlowDefinition( | ||
| "node('slave') {\n" | ||
| + " wrap([$class: 'PortAllocator', pools: ['WEBLOGIC']]) {\n" |
There was a problem hiding this comment.
If you have a @Symbol you should not need to explicitly use the wrap step.
There was a problem hiding this comment.
@jglick can you give me an example plugin that does this? trying to resurrect this PR
There was a problem hiding this comment.
never mind, think I found what I need: https://jenkins.io/doc/developer/plugin-development/pipeline-integration/
There was a problem hiding this comment.
@jglick I started resurrecting this pr: https://github.com/er1c/port-allocator-plugin/tree/workflow-support but I got stuck on this @Symbol thing.
I get an error like:
java.lang.NoSuchMethodError: No such DSL method portallocator found among [archive, bat, build, catchError, checkout, dir, echo, error, fff, fileExists, injectSubtypesAsContext, input, load, mail, node, parallel, persistenceProblem, pwd, readFile, retry, sh, sleep, stage, step, timeout, tool, unarchive, waitUntil, withEnv, wrap, writeFile, ws]
at org.jenkinsci.plugins.workflow.cps.DSL.invokeMethod(DSL.java:107)
at org.jenkinsci.plugins.workflow.cps.CpsScript.invokeMethod(CpsScript.java:98)
at org.codehaus.groovy.runtime.callsite.PogoMetaClassSite.call(PogoMetaClassSite.java:48)
at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:48)
Is this something you can take a quick look at?
|
@bpedersen2 are you planning to address jglick's comments and close this anytime soon? |
|
Probably not anytime soon.... |
jglick
left a comment
jglick
left a comment
There was a problem hiding this comment.
No current plans to review again. Sounds like it needs someone to pick it up and do a bit of polish.
8 participants
based on the work by @m00re implement workflow support for with a dedicated step.