-
Notifications
You must be signed in to change notification settings - Fork 8
Output the jobs logs at the API and CLI output #65
Conversation
There was a problem hiding this 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.
cmd/mistryd/job.go
Outdated
Output string | ||
Log template.HTML | ||
State string | ||
BuildInfo types.BuildInfo |
There was a problem hiding this comment.
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.
cmd/mistryd/job.go
Outdated
|
||
// ReadLogs returns the job's logs | ||
func (j *Job) ReadLogs() ([]byte, error) { | ||
return ReadJobLogs(j.ReadyBuildPath) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
cmd/mistryd/job.go
Outdated
@@ -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) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
cmd/mistryd/job.go
Outdated
} | ||
|
||
// GetBuildInfo returns the job's build info | ||
func (j *Job) GetBuildInfo(readLogs bool) (types.BuildInfo, error) { |
There was a problem hiding this comment.
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
.
cmd/mistryd/job.go
Outdated
|
||
// ReadyBuildLogPath returns the path of the job logs found at jobReadyPath | ||
func ReadyBuildLogPath(jobReadyPath string) string { | ||
return filepath.Join(jobReadyPath, BuildLogFname) |
There was a problem hiding this comment.
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
.
4fea742
to
1333b5d
Compare
cmd/mistryd/job.go
Outdated
} | ||
|
||
// ReadJobBuildInfo returns the BuildInfo found at jobPath | ||
func ReadJobBuildInfo(jobPath string, readLogs bool) (*types.BuildInfo, error) { |
There was a problem hiding this comment.
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
.
1333b5d
to
c53c52b
Compare
* 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
c53c52b
to
7c7bc26
Compare
Added the job logs as the
Log
attribute to theBuildInfo
, so all API responses returning the BuildInfo can provide job logs, in particular thePOST /jobs/
one. The CLI build invocation uses the response to output logs if the--verbose
flag is set.CLI output looks like this