Skip to content

[JENKINS-54532] Document how to get parameters from a Run #3724

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.

Sign up for GitHub

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

Closed
ringerc wants to merge 5 commits into from
Closed

[JENKINS-54532] Document how to get parameters from a Run #3724

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c6a4b46
[JENKINS-54532] Document how to get parameters from a Run
ringerc Nov 8, 2018
075f462
[JENKINS-54532] - Fix formatting of Javadoc
oleg-nenashev Nov 16, 2018
4c6fdd6
[JENKINS-54532] - More fixes
oleg-nenashev Nov 16, 2018
ae778a1
Update Run.java
oleg-nenashev Jan 29, 2019
cdcb613
Fix Javadoc
daniel-beck Apr 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion core/src/main/java/hudson/model/Run.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,16 @@
* a custom {@link Job} type, so there's no separate registration
* mechanism for custom {@link Run} types.
*
* Many details of a run are contained in {@link Action} instances
* accessible via {@link Actionable#getAllActions} and {@link #getAction(Class)}.
* For example, build access to build parameters is obtained with
* <code>run.getAction(hudson.model.ParametersAction.class)</code> to
Copy link
Member

@jglick jglick Apr 11, 2019

Choose a reason for hiding this comment

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

Suggested change
* <code>run.getAction(hudson.model.ParametersAction.class)</code> to
* {@code run.getAction(ParametersAction.class)} to

* get the build's {@link ParametersAction} object.
*
* @author Kohsuke Kawaguchi
* @see RunListener
* @see Job
* @see Action
*/
@ExportedBean
public abstract class Run <JobT extends Job<JobT,RunT>,RunT extends Run<JobT,RunT>>
Expand Down Expand Up @@ -1441,7 +1449,9 @@ public Collection<Fingerprint> getBuildFingerprints() {

/**
* Returns an input stream that reads from the log file.
* It will use a gzip-compressed log file (log.gz) if that exists.
*
* It will use a gzip-compressed log file (log.gz) if that exists, decompressing
Copy link
Member

@oleg-nenashev oleg-nenashev May 22, 2019

Choose a reason for hiding this comment

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

Suggested change
* It will use a gzip-compressed log file (log.gz) if that exists, decompressing

* it on the fly to return the uncompressed data.
Copy link
Member

@daniel-beck daniel-beck Apr 10, 2019

Choose a reason for hiding this comment

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

Not a fan of this being documented in the API (strictly speaking, this means JEP-210 is invalid), but at least this PR doesn't add it.

Copy link
Member

@jglick jglick Apr 11, 2019

Choose a reason for hiding this comment

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

Yeah to complement #3963 I would suggest simply deleting this sentence. How or where the log text is stored is a matter of no concern to the caller of this method, and for Pipeline there is no built-in gzip support (this implementation is not used).

Copy link
Member

@batmat batmat Apr 11, 2019

Choose a reason for hiding this comment

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

+1, also this would become wrong indeed if we ended up merging #3966

Copy link
Member

@oleg-nenashev oleg-nenashev May 22, 2019

Choose a reason for hiding this comment

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

Removing it at all since there is a consensus. Not much related to the PR anyway

Copy link
Member

@oleg-nenashev oleg-nenashev May 22, 2019

Choose a reason for hiding this comment

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

Suggested change
* it on the fly to return the uncompressed data.

*
* @throws IOException
* @return An input stream from the log file.
Expand Down Expand Up @@ -1469,6 +1479,14 @@ public Collection<Fingerprint> getBuildFingerprints() {
return new ByteArrayInputStream(charset != null ? message.getBytes(charset) : message.getBytes());
}

/**
* Returns a Reader that reads from the log file.
Copy link
Member

@jglick jglick Apr 11, 2019

Choose a reason for hiding this comment

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

Suggested change
* Returns a Reader that reads from the log file.
* Reads from the log file.

Stating the return type of a method (and without @code or @link, at that) is not helpful.

*
* This is the simplest way to access the job's console log.
Copy link
Member

@jglick jglick Apr 11, 2019

Choose a reason for hiding this comment

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

Well…not necessarily. Depends on what you wanted to accomplish. For example, this will contain raw console notes, which might be unwanted in some contexts. getLogText is the most general way to access the log, as you can use that to work with the log in a variety of ways.

Copy link
Member

@oleg-nenashev oleg-nenashev May 22, 2019

Choose a reason for hiding this comment

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

Agreed, we should not be doing arguable suggestions, especially if we return to JEP-207

Suggested change
* This is the simplest way to access the job's console log.

*
* @throws IOException Log access error
* @return Character-oriented log file stream as a Reader
*/
public @Nonnull Reader getLogReader() throws IOException {
if (charset==null) return new InputStreamReader(getLogInputStream());
else return new InputStreamReader(getLogInputStream(),charset);
Expand Down