Skip to content
This repository has been archived by the owner on Sep 4, 2024. It is now read-only.

Output the jobs logs at the API and CLI output #65

Merged
merged 1 commit into from
May 3, 2018

Conversation

dtheodor
Copy link
Contributor

@dtheodor dtheodor commented Apr 30, 2018

Added the job logs as the Log attribute to the BuildInfo, so all API responses returning the BuildInfo can provide job logs, in particular the POST /jobs/ one. The CLI build invocation uses the response to output logs if the --verbose flag is set.

  • Updated structs:
    • the Job already contained the BuildInfo (but serialized) as Job.Output. Removed that and added a proper struct Job.BuildInfo
    • move the Log to the BuildInfo, remove it from Job.Log
    • populate the BuildInfo with the log contents wherever is needed
    • update HTML template to use the updated structs
  • move common code to read a job's BuildInfo JSON to a couple of new job functions. There was relevant code duplication when listing jobs, in the ExitCode function, and in tests
  • update CLI to output the job logs

CLI output looks like this

screenshot from 2018-04-30 18-20-33

Copy link
Contributor

@agis agis left a comment

Choose a reason for hiding this comment

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

I think we should have j.ReadLog() and j.ReadBuildInfo() that just populate j.Log and j.BuildInfo accordingly.

Output string
Log template.HTML
State string
BuildInfo types.BuildInfo
Copy link
Contributor

Choose a reason for hiding this comment

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

I think semantically it's more correct to have a pointer here.


// ReadLogs returns the job's logs
func (j *Job) ReadLogs() ([]byte, error) {
return ReadJobLogs(j.ReadyBuildPath)
Copy link
Contributor

Choose a reason for hiding this comment

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

What if someone calls this when the job is pending?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if only there was an easy way to get the job's state

@@ -359,3 +353,55 @@ func (j *Job) BootstrapBuildDir(fs filesystem.FileSystem, log *log.Logger) (bool
func (j *Job) RemoveBuildDir(fs filesystem.FileSystem, log *log.Logger) error {
return fs.Remove(j.ReadyBuildPath)
}

// ReadLogs returns the job's logs
func (j *Job) ReadLogs() ([]byte, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd expect this method to just populate j.BuildInfo.Log instead of returning something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but we don't know if the job even has a BuildInfo

}

// GetBuildInfo returns the job's build info
func (j *Job) GetBuildInfo(readLogs bool) (types.BuildInfo, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's more intuitive to have do a j.ReadBuildInfo() that just populates j.BuildInfo.


// ReadyBuildLogPath returns the path of the job logs found at jobReadyPath
func ReadyBuildLogPath(jobReadyPath string) string {
return filepath.Join(jobReadyPath, BuildLogFname)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can simplify this by checking both for pending and then ready.

}

// ReadJobBuildInfo returns the BuildInfo found at jobPath
func ReadJobBuildInfo(jobPath string, readLogs bool) (*types.BuildInfo, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You could rename jobPath to the simpler path. Also readLogs to logs.

* move the Log to the BuildInfo
* remove redundant Job.Output and Job.Log, add Job.BuildInfo
* populate the BuildInfo with the Log on new job finish, and on job listing
* move common code to read a job's BuildInfo to job functions
* update HTML template to use the updated structs
* update CLI to output the job logs
@dtheodor dtheodor merged commit c56d263 into master May 3, 2018
@dtheodor dtheodor deleted the return-logs-on-build branch May 3, 2018 07:49
@dtheodor dtheodor mentioned this pull request Sep 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants