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

[JENKINS-37324] Retain script argument to the batch/shell steps so we can retrieve it upon demand #22

Closed
wants to merge 3 commits into from

Conversation

@svanoort
Copy link
Member

commented Nov 23, 2016

Implements JENKINS-37324 for shell & batch step, more or less.

This gives us the script argument that we really need to implement smarter display of batch steps. Note that I'm not modifying the displayName or functionName for the step, because IIRC there is existing logic dependent on that -- but consumers of the flow graph can do the following to get what is needed:

ScriptArgumentAction action = flowNode.getAction(ScriptArgumentAction.class);
String batchOrShellScript = action.getScript();

Ta-da! CC @vivek and @michaelneale

I'm not marking it ready to review yet because need to look at it with fresh eyes tomorrow and see if there's a smarter way to implement this.

TODO:

  • Rejigger this as described by Jesse using a direct serialization of the Step in the Actions for the FlowNode (initial version, avoids some compatibility issues there). This way we can get at far more info from the execution.

Catch: this serializes far more data. It might reduce performance if done without a rewrite of flow node storage.

@svanoort svanoort changed the title Retain script argument to the batch/shell steps so we can retrieve it upon demand [JENKINS-37324] Retain script argument to the batch/shell steps so we can retrieve it upon demand Nov 23, 2016

@i386

This comment has been minimized.

Copy link

commented Nov 23, 2016

Would be nice if steps could expose this easily through their implementations. One request was for shell, batch and scm steps but undoubtably there will be more steps.

Love that we are tackling this one.

@michaelneale

This comment has been minimized.

Copy link
Member

commented Nov 23, 2016

I like it

@vivek

This comment has been minimized.

Copy link
Contributor

commented Nov 23, 2016

@svanoort This is great but I agree with @i386, in blueocean we will always have to check for such actions vs this is exposed from underlying API.

@i386

This comment has been minimized.

Copy link

commented Nov 23, 2016

Here's a good example of where SCM step descriptions would help the user

changes 1

@jglick
Copy link
Member

left a comment

As previously mentioned, this had better be done at a more generic level.

@svanoort

This comment has been minimized.

Copy link
Member Author

commented Dec 6, 2016

Closing this because we're going to do this in a better way that is portable across steps, by serializing the step instance itself :)

@svanoort svanoort closed this Dec 6, 2016

@svanoort svanoort deleted the retain-step-script-JENKINS-37324 branch Aug 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.