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

Add getLog() to whitelist #27

Closed
wants to merge 2 commits into from
Closed

Conversation

ernetas
Copy link

@ernetas ernetas commented Jan 2, 2017

It would be nice to get build logs without approving method org.jenkinsci.plugins.workflow.support.steps.build.RunWrapper getRawBuild, which is fairly permissive.

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.

Neglected to document this in src/main/resources/org/jenkinsci/plugins/workflow/support/steps/build/RunWrapper/help.html. And ideally would have basic coverage in RunWrapperTest. Otherwise seems reasonable.

@@ -162,6 +162,11 @@ public String getFullProjectName() throws AbortException {
}

@Whitelisted
public List<String> getLog(int maxLines) throws IOException {
return build().getLog(maxLines);
Copy link
Member

Choose a reason for hiding this comment

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

Possibly bad for performance if the argument is large. On the other hand sometimes you would want the full log. jenkinsci/workflow-basic-steps-plugin#2 might provide a more scalable solution, at least for the common case of scanning for messages in the current build’s log after an error (this method could be used for other purposes, such as after a build step).

@@ -3,6 +3,7 @@
<dt><code>result</code></dt><dd>typically <code>SUCCESS</code>, <code>UNSTABLE</code>, or <code>FAILURE</code> (<em>may</em> be null for an ongoing build)</dd>
<dt><code>displayName</code></dt><dd>normally <code>#123</code> but sometimes set to, e.g., an SCM commit identifier</dd>
<dt><code>description</code></dt><dd>additional information about the build</dd>
<dt><code>log</code></dt><dd>console output of the build</dd>
Copy link
Member

Choose a reason for hiding this comment

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

This is a method, not a property, so must be documented differently.

@jglick
Copy link
Member

jglick commented Jan 19, 2018

cf. #51

@dwnusbaum
Copy link
Member

dwnusbaum commented Jan 15, 2020

I am going to go ahead and close this. I think we should discuss this further on JENKINS-46376 to make sure we understand the use cases, and so that we can try to find an approach that doesn't encourage users to process logs directly in their Pipelines.

@dwnusbaum dwnusbaum closed this Jan 15, 2020
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