Can a nested Jelly config figure out whether it's in a Build or Post Build?

[Alexandra Goultiaeva]

I'm trying to write a wrapper to allow the Google Storage Plugin Post-Build step to also act as a Build step.

Here is my first attempt. There is an inner class (e.g., ClassicUpload) that might be instantiated in a Build step or in a Post Build step.

It has an option ("on failed build") which makes sense for Post-Build, but not for Build Step. I wanted to make that option hidden for the Build Step version but not for the Post Build.

I have tried the following approaches, but was not able to get either of them to work:

- Use a jelly built in variable. The problem is, the class is nested. So, in the corresponding jelly config, the "it" variable points to FreeStyleProject, and the "instance" points to the inner class (ClassicUpload), which doesn't know whether it's in a Build Step or Post-Build Step.

- Set a jelly variable "isBuild" in the parent class jelly (BuildStep or Post-Build Step), and read it in the child class. It works correctly when the page is originally rendered, but if I'm adding a nested class through the UI the jelly variable, then "isBuild" is undefined

- Get the nested class to remember if it's BuildStep or Post-Build: create a bool isBuild, set it through the constructor, read it in the jelly. The problem is that the magic happens through "hetero-list". It would be easy to set isBuild in the original default list, but is there a way to pass something as a parameter to the constructor?

Does anyone have any suggestions on how to approach this?

Thanks!

[Jesse Glick]

On Thu, Sep 14, 2017 at 11:51 AM, 'Alexandra Goultiaeva' via Jenkins
Developers <jenkin...@googlegroups.com> wrote:
> Does anyone have any suggestions on how to approach this?

Make it a `Publisher & SimpleBuildStep`. For freestyle projects, it
will appear as a post-build action, which is generally where it should
be (guessing based purely on the name of the plugin). Users with more
complex requirements will be able to run the step from a Pipeline job
whenever and however they like.

[Alexandra Goultiaeva]

Thanks for the suggestion!

Do I understand it correctly, that you are suggesting that we make the base class implement SimpleBuildStep, which will allow it to be used in a pipeline - but will not expose it as a Build step for Freestyle project?

We specifically wanted to allow this to be a Build step in Freestyle projects - do you feel this is ill-advised?

There's some teams unable to switch to pipeline for other compatibility reasons, we wanted to allow them to use the upload step as a build step as well.

As a next step, we planned to add Download functionality, making this plugin no longer only relevant to post-build - or do you feel that functionality would be better as a separate plugin?

[Jesse Glick]

On Fri, Sep 15, 2017 at 10:43 AM, 'Alexandra Goultiaeva' via Jenkins
Developers <jenkin...@googlegroups.com> wrote:
> Do I understand it correctly, that you are suggesting that we make the base
> class implement SimpleBuildStep, which will allow it to be used in a
> pipeline

Correct.

> but will not expose it as a Build step for Freestyle project?

Ignored by freestyle projects. If it is a `Publisher`, it will be
offered as a post-build action; if a `Builder`, as a build step.

> There's some teams unable to switch to pipeline for other compatibility
> reasons

Please make sure those are filed in JIRA.

> we wanted to allow them to use the upload step as a build step as
> well.

Then make a separate `Builder` (which would be the more natural one to
mark as a `SimpleBuildStep` as well, if you have to choose). Obviously
most implementation can be shared via a utility class.

> As a next step, we planned to add Download functionality, making this plugin
> no longer only relevant to post-build - or do you feel that functionality
> would be better as a separate plugin?

Sounds appropriate as a build step in the same plugin. Again it could
be useful from Pipeline.

As a general matter I recommend developers stay away from overly
specific plugin features like “upload this to Google Storage if the
build is stable” or “download this from Google Storage and unzip to
the workspace”. Again I know nothing about the specifics of your
plugin, but you can consider a different style: have the project’s
`Makefile` / `pom.xml` / `build.gradle` / etc. actually do those
things if, when, and how it chooses. Presumably using some kind of
convenient CLI tool specific to the service—you do have a nice,
tested, well-documented CLI somewhere, right?—and limit the Jenkins
plugin to the minimum necessary to set up an environment conducive to
running that tool. Typically this would be a `SimpleBuildWrapper`
(usable equally from freestyle or Pipeline), for example integrating
with the `credentials` plugin to let a project configure an access
token or whatever to connect to the service account; perhaps using
`ToolInstallation` to ensure that your CLI is installed and ready in
`$PATH` (for those benighted users who are not yet running all builds
inside a reproducible Docker image).

[Alexandra Goultiaeva]

Then make a separate `Builder` (which would be the more natural one to
mark as a `SimpleBuildStep` as well, if you have to choose). Obviously
most implementation can be shared via a utility class.

Thanks! This is exactly what I've tried to do in the code I've linked to at the beginning.

I've created a separate Builder that utilized existing implementation.

My original question was about whether I could reuse the (nested) jelly files as well, and if the framework was flexible enough to allow me to make the config.jelly display slightly differently in the Post-Build Step and Build Step.

 

As a general matter I recommend developers stay away from overly
specific plugin features like “upload this to Google Storage if the
build is stable” or “download this from Google Storage and unzip to
the workspace”.

Agreed, the download capability we're planning is pretty generic. Any post-processing can be done by e.g., a bash step.

[Jesse Glick]

On Thu, Sep 14, 2017 at 11:51 AM, 'Alexandra Goultiaeva' via Jenkins
Developers <jenkin...@googlegroups.com> wrote:
> It has an option ("on failed build") which makes sense for Post-Build, but
> not for Build Step. I wanted to make that option hidden for the Build Step
> version but not for the Post Build.

Make the common parts into a `Describable` with a `config.jelly`.
Include those into the `Builder` and the `Publisher` as a field bound
to `f:property`. Then the `Publisher`’s `config.jelly` would simply
have another entry with an `f:checkbox`.

(Or, again IMO, do not clutter the plugin with options like this. A
`Publisher` designed for freestyle projects should just not run when a
build is unstable—good enough. Declarative Pipeline users, when
consuming a `Builder & SimpleBuildStep`, have straightforward detailed
control over such behaviors using `when` blocks.)

[Alexandra Goultiaeva]

Make the common parts into a `Describable` with a `config.jelly`.
Include those into the `Builder` and the `Publisher` as a field bound
to `f:property`. Then the `Publisher`’s `config.jelly` would simply
have another entry with an `f:checkbox`.

That makes sense! That would mean changing the interface, but it seems warranted in this case.

I guess to make it backward compatible I could still keep the old field, forward the setting from the Publisher and pass in a hardcoded value from the Builder.

(Or, again IMO, do not clutter the plugin with options like this. A
`Publisher` designed for freestyle projects should just not run when a
build is unstable—good enough. Declarative Pipeline users, when
consuming a `Builder & SimpleBuildStep`, have straightforward detailed
control over such behaviors using `when` blocks.)

The plugin does seem a bit over-engineered, but that's the way I inherited it.

If I want my changes to be backward compatible, I assumed I have to keep them.

Do you know, apart from the overall download count, if there is any way to check if features are being used?

[Jesse Glick]

On Fri, Sep 15, 2017 at 5:55 PM, 'Alexandra Goultiaeva' via Jenkins
Developers <jenkin...@googlegroups.com> wrote:
> I guess to make it backward compatible I could still keep the old field

The developer docs includes a page on using `readResolve` and the
like, incl. testing with `@LocalData`.

> Do you know, apart from the overall download count, if there is any way to
> check if features are being used?

I am afraid not.