Decouple BuildSelector from Copy Artifact plugin into a separate repo

[Alexandru Somai]

Hello,

Currently I'm working to add more features to the External Workspace Manager plugin. One of the features that I need is to allow the user to choose a specific build from the upstream job. For this, I was thinking to integrate the BuildSelector from the Copy Artifact plugin.

I've seen that there is some work going on for the Copy Artifact 2.0: https://github.com/jenkinsci/copyartifact-plugin/pull/71. I think that this may be a good opportunity to move the BuildSelector extension point (and all of its implementations) to a separate repository, e.g. build-selector-plugin. In this case, the Copy Artifact plugin and the External Workspace Manager plugin can both have as dependency this new plugin. Also, in the future, if other plugins may need to integrate a build selection feature, they can easily add as dependency this new plugin.

Ikedam, I've seen that you are the maintainer of the Copy Artifact plugin, and you are working on version 2.0. What do you think about the idea of moving the BuildSelector extension point to a separate plugin repository?

Regards,

Alex

[Ikedam]

Hello, Alex.

> What do you think about the idea of moving the BuildSelector extension point to a separate plugin repository?

Sounds good to me.
As you pointed, the build selection feature of CopyArtifact is also useful for other plugins.

Though CopyArtifact 2.0 is still in progress (for long time...), I belive we should base on CopyArtifact 2.0:

* It outputs diagnostic messages.
* It works with `BuildFilter`, which enables us to specify more complicated conditions to pick builds.
* Copying features are completely isolated from BuildSelectors.

We can start in a following way:

* Clone jenkinsci/copyartifact-plugin into ikedam/build-selector-plugin (or alexsomain/build-selector-plugin)
* Merge copyartifact-2.0. (#71)
* Rename packages and classes, and remove deprecated features.
* Ask to fork it to jenkinsci/build-selector-plugin
* Release build-selector-plugin 0.1.0.

Some issues:

* CopyArtifactPermissionProperty. I think it shouldn't moved to build-selector-plugin and build-selector-plugin should provide an alternate mechanism.

Regards,
ikedam

[Oleg Nenashev]

Hi,

The approach looks good to me. We really have much plugins, which may benefit from the Build Selector functionality. If required, I could create a clone of the plugin directly in the jenkinsci organization and make you both plugin developers.

From the terminology perspective, maybe makes sense to use "RunSelector" instead of the "BuildSelector". It's not so important, but maybe it's a good opportunity to align it.

Best regards,

Oleg

[Alexandru Somai]

Hello,

If required, I could create a clone of the plugin directly in the jenkinsci organization and make you both plugin developers.

This would be useful.

I think that the PR #71 from Copy Artifact plugin should be also merged as it is into the new repository.

Afterwards, we can start renaming packages, classes and remove deprecated features on top of that.

Regarding:

maybe makes sense to use "RunSelector" instead of the "BuildSelector"

 

I don't have any strong opinion for this one. So in this case, should we call it run-selector-plugin?

CopyArtifactPermissionProperty. I think it shouldn't moved to build-selector-plugin and build-selector-plugin should provide an alternate mechanism.

I do agree with this one. But in the first phase I'd suggest on focusing to have the selectors into a separate plugin. And afterwards, as a second phase, we can think of a mechanism for permissions.

Regards,

Alex

[Oleg Nenashev]

Hello,

The code has been copied to https://github.com/jenkinsci/run-selector-plugin . I decided not to do perform a GitHub fork in order to avoid future links between two plugins in history search and Github's diff interfaces. Can convert it to fork if there are strong opinions.

alexsomai and ikedam have developer permissions.

Have a good hacking!

BR, Oleg

[Alexandru Somai]

Hello,

Thank you Oleg for setting-up the repo.

The first step that I'll do is to remove the deprecated features and code that was inherited from the Copy Artifact plugin.

I'll create a PR as soon as I'm done with this.

Afterwards, I think that we should rename the packages and classes.

Regards,

Alex

[Ikedam]

Hello.

Thanks for the repository.
I don't have so much time to participate in the development, and alexsomai is the maintainer of this plugin.
I'll participate in the development only by sending a pull request.

>> CopyArtifactPermissionProperty. I think it shouldn't moved to build-selector-plugin and build-selector-plugin should provide an alternate mechanism.

> I do agree with this one. But in the first phase I'd suggest on focusing to have the selectors into a separate plugin. And afterwards, as a second phase, we can think of a mechanism for permissions.

I agree.
We can start without CopyArtifactPermissionProperty.

Regards,
ikedam

[Ikedam]

> We can start without CopyArtifactPermissionProperty.

I found that `CopyArtifactPermissionProperty` works only for job selection,
and does nothing for run selection.
(Copyartifact2.0 doesn't yet support permissions of runs.
Related: https://issues.jenkins-ci.org/browse/JENKINS-22460 )

No alternate mechanism is needed in run-selector for migration
if run-selector handles processes only after job selection

Regards,
ikedam

[Jesse Glick]

On Tue, Jul 5, 2016 at 8:08 AM, Alexandru Somai
<somai.a...@gmail.com> wrote:
> One of the features that I need is to allow the user to choose a
> specific build from the upstream job. For this, I was thinking to integrate
> the BuildSelector from the Copy Artifact plugin.

As discussed in the Gitter chat, I think this is a bad idea, at least
for usage for Pipeline.

[Alexandru Somai]

Hello everyone,

I have some concerns about how a specific build from the upstream job should be selected in the case of the plugin that I am currently developing: https://github.com/jenkinsci/external-workspace-manager-plugin.

Please note that the plugin focus is mainly on Pipeline jobs.

Based on other discussions and suggestions, I have reached to the following possible solutions.

I have the following use cases:

• Select the last stable (or any other status) build from the upstream job

1. The first example uses the RunSelector plugin, that is currently refactored, being decoupled from the Copy Artifact plugin.

In this case, I would also need to add a RunSelectorWrapper that can be applied as a Pipeline step, and return a RunWrapper object.

def run = step([$class: 'RunSelectorWrapper', upstream: 'upstream-name', selector: [$class: ['PermalinkRunSelector'], id: 'lastStableBuild']])

// this is a step specific to the External Workspace Manager Plugin, that accepts as input a RunWrapper object.

def extWorksace = exwsAllocate selectedRun: run 

2. Second example is based on creating a complete new step, that can be used in the Pipeline code.

This step would return a RunWrapper object, that is, in this case, the last stable build from the upstream job.

def run = runSelector upstream: 'upstream-name', status: 'lastStableBuild'

// this is a step specific to the External Workspace Manager Plugin, that accepts as input a RunWrapper object.

def extWorksace = exwsAllocate selectedRun: run

• Select a specific build number from the upstream job

1. First example, making use of RunSelector plugin

def run = step([$class: 'RunSelectorWrapper', upstream: 'upstream-name', selector: [$class: ['SpecificRunSelector'], buildNumber: '20']])

2.  Second example, based on the new created step:

def run = runSelector upstream: 'upstream-name', buildNumber: 20

I would really appreciate some feedback.

Thank you,

Alex

[Jesse Glick]

On Mon, Jul 11, 2016 at 11:27 AM, Alexandru Somai
<somai.a...@gmail.com> wrote:
> def run = step([$class: 'RunSelectorWrapper', upstream: 'upstream-name',
> selector: [$class: ['PermalinkRunSelector'], id: 'lastStableBuild']])

`RunSelectorWrapper` is a poor name as it suggests
`[Simple]BuildWrapper`, used with the `wrap` metastep, whereas you are
here creating a `SimpleBuildStep`.

With JENKINS-29922 and some `@Symbol`s this would wind up looking something like

def run = selectRun upstream: 'upstream-name', selector:
permalink('lastStableBuild')

> // this is a step specific to the External Workspace Manager Plugin, that
> accepts as input a RunWrapper object.
> def extWorksace = exwsAllocate selectedRun: run

Possible, though choosing that pattern makes it impossible to use
_Snippet Generator_ to get syntax examples. Generally recommend that
step parameters be simple types like `String`, `int`, or collections
of some `Describable` subtype, all of which are amenable to Jenkins
databinding.

> def run = runSelector upstream: 'upstream-name', buildNumber: 20

This is a misleading example. Obviously your script would not
hard-code an upstream build number. It would pick it up from a build
parameter; for example:

def run = runSelector upstream: 'upstream-name', buildNumber:

UPSTREAM_BUILD_NUMBER

[martinda]

See my question in-line below.

On Tuesday, July 12, 2016 at 11:36:15 PM UTC-4, Jesse Glick wrote:

> // this is a step specific to the External Workspace Manager Plugin, that
> accepts as input a RunWrapper object.
> def extWorksace = exwsAllocate selectedRun: run

Possible, though choosing that pattern makes it impossible to use
_Snippet Generator_ to get syntax examples. Generally recommend that
step parameters be simple types like `String`, `int`, or collections
of some `Describable` subtype, all of which are amenable to Jenkins
databinding.

To refer to another build in a single object, we need a composite object containing the job name (string) and the build number (int). Would using a "collection of some `Describable` subtype" provide such composite object?

Martin

[Jesse Glick]

On Wed, Jul 13, 2016 at 8:20 AM, martinda <martin....@gmail.com> wrote:
>> choosing that pattern makes it impossible to use
>> _Snippet Generator_ to get syntax examples. Generally recommend that
>> step parameters be simple types like `String`, `int`, or collections
>> of some `Describable` subtype, all of which are amenable to Jenkins
>> databinding.
>
> To refer to another build in a single object, we need a composite object
> containing the job name (string) and the build number (int).

Not really. You can just accept a `String` parameter and an `int` parameter.