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

Make plugin compatible to Jenkins Pipeline plugin #1

Merged
merged 1 commit into from
Aug 11, 2016

Conversation

EmteZogaf
Copy link

A simple implementation of SimpleBuildStep. Creating a dummy FreeStyleBuild and only setting properties which are needed by CommandInterpreter#perform(AbstractBuild, Launcher, TaskListener).

@EmteZogaf EmteZogaf force-pushed the make-pipeline-compatible branch 3 times, most recently from 386bdba to f27d819 Compare July 21, 2016 14:18
@EmteZogaf EmteZogaf changed the title Make plugin compatible to Jenkins Pipeline plugin [WIP] Make plugin compatible to Jenkins Pipeline plugin Jul 25, 2016
@kinow
Copy link
Member

kinow commented Jul 27, 2016

Hi @EmteZogaf ,

Thank you for the pull request.

I have another plugin that received a similar pull request, so I already have in my TODO list to sit down, and spend some hours learning the internals of the Jenkins Pipeline plug-in, and how to add support for it in other plug-ins.

I thought given the simplicity of the R plug-in your PR could be a good primer for me, so that I could learn a little bit more about the Jenkins Pipeline plug-in, but looks like in order to include a CommandInterpreter you have to use a BuildStep? Interesting. I'll need a bit more of time to understand why, then will merge and cut a new release ;)

If you have some pointers to educate me on the pipeline plug-in, feel free to paste them here.

Thanks again
Bruno

@EmteZogaf
Copy link
Author

I just followed https://github.com/jenkinsci/pipeline-plugin/blob/master/DEVGUIDE.md#build-steps but I still need time to figure where to configure the PATH as the RScript executable is currently not found when called from pipeline script.

@EmteZogaf
Copy link
Author

It's still not working in a workflow/pipeline project because of JENKINS-29144. The build step is not getting the PATH environment variable and hence cannot find the RScript executable. Hope they will fixed it soon...

@kinow
Copy link
Member

kinow commented Aug 1, 2016

Thanks @EmteZogaf

Feel free to keep the PR up to date as necessary, and ping me in case you need anything and I take too long to reply here.

@EmteZogaf EmteZogaf force-pushed the make-pipeline-compatible branch 2 times, most recently from 2d57b41 to fbbaba3 Compare August 2, 2016 13:01
@EmteZogaf
Copy link
Author

It's working for me now. I tried it with local (master) executor and a ssh slave. I tried it with both Jenkins versions 1.580.1 and 2.7.1.

@EmteZogaf EmteZogaf changed the title [WIP] Make plugin compatible to Jenkins Pipeline plugin Make plugin compatible to Jenkins Pipeline plugin Aug 2, 2016
@EmteZogaf
Copy link
Author

The current implementation is just a quick hack to be able to use R scripts inside pipeline scripts. But it blocks the groovy process executing the pipeline script till the R process returns.

We should use the step api like the DurableTaskStep does. I'll give it a try in the upcoming weeks.

Add compatibility with workflow/pipeline plugin. Copy behavior of
ShellScript provided by workflow-durable-task-step-plugin for reusing
the durability feature. The call to Rscript is wrapped in a shell
script.
@EmteZogaf EmteZogaf force-pushed the make-pipeline-compatible branch from fbbaba3 to fef63a6 Compare August 4, 2016 14:59
@EmteZogaf
Copy link
Author

As lazy as a sloth I copied the implementation of ShellStep and call the R script from inside the shell script.

@kinow
Copy link
Member

kinow commented Aug 4, 2016

Laziness is a bless @EmteZogaf :)

Is this PR ready to be reviewed? Or is there anything else missing?

@EmteZogaf
Copy link
Author

I'll test it today in production server and will give you report.

@Le0Michine Le0Michine merged commit 1d4fb79 into jenkinsci:master Aug 11, 2016
@Le0Michine
Copy link
Member

I will check once again and create a new release this weekend

@kinow
Copy link
Member

kinow commented Aug 12, 2016

Hi @Le0Michine I'm the maintainer of the r-plugin. Are you talking about releasing the R plug-in this weekend?

@kinow
Copy link
Member

kinow commented Aug 12, 2016

Hi @EmteZogaf , reverted the merge. Can you submit again your pull request once it's done, and let me know how your test in the production server went. Still rolling a development cycle for BioUno plug-ins, so will cut a new release of image-gallery, active-choices, r-plugin, etc, in the next days. So we can include your change too.

@Le0Michine
Copy link
Member

@kinow sorry man, I just though it is my plugin because I received a notification, I won't do anything at the evening after hard work :)

@kinow
Copy link
Member

kinow commented Aug 12, 2016

Not a problem @Le0Michine

I think by default GitHub subscribes you to all repositories in your organisations (or it used to do that). I disabled that option in my profile settings, to reduce the amount of notifications (-:

In case you ever feel like contributing and even co-maintaining r-plugin or any other plug-in, just drop a note or send some pull requests :+1

@EmteZogaf
Copy link
Author

Ok, thx. It's currently working so far as expected. I'll try some more advanced and long-lasting scripts today and see, if the durability feature is given (restart jenkins during build).

@EmteZogaf
Copy link
Author

Restarting jenkins during pipeline build, re-establishing connection to running R process and completing build works fine. I'll re-submit pr.

@EmteZogaf
Copy link
Author

see #4

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

Successfully merging this pull request may close these issues.

3 participants