Skip to content
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

Implement workflow support #10

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

bpedersen2
Copy link

based on the work by @m00re implement workflow support for with a dedicated step.

m00re and others added 9 commits July 22, 2016 08:56
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.
@svanoort
Copy link
Member

@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

@pepov
Copy link
Contributor

pepov commented May 31, 2017

Sorry I'm not active anymore in this project and don't have the time right now to get up to speed again.

@@ -254,5 +256,11 @@ public Integer call() throws IOException {
}

private static final long serialVersionUID = 1L;

@Override
public void checkRoles(RoleChecker roleChecker) throws SecurityException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not do this! I think you meant to just extend MasterToSlaveCallable.

}

@DataBoundSetter
public void setPools(String[] pools) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why two attributes? Delete setPool.

public void setPlainports(String[] plainports) {
if (plainports != null) {
for (String port : plainports) {
this.ports.add(new DefaultPortType(port));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have a @Symbol you should not need to explicitly use the wrap step.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jglick can you give me an example plugin that does this? trying to resurrect this PR

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

@jglick jglick self-requested a review May 31, 2017 15:36
@abishek3876
Copy link

@bpedersen2 are you planning to address jglick's comments and close this anytime soon?

@bpedersen2
Copy link
Author

Probably not anytime soon....

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No current plans to review again. Sounds like it needs someone to pick it up and do a bit of polish.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants