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 creation time to job status #1540

Merged
merged 5 commits into from
Aug 16, 2016
Merged

Add creation time to job status #1540

merged 5 commits into from
Aug 16, 2016

Conversation

camerondavison
Copy link
Contributor

@camerondavison camerondavison commented Aug 8, 2016

closes #1217

I decided to take the approach of adding a command line flag to add the extra metadata, the other idea would be to make -verbose add the extra metadata.

@diptanu
Copy link
Contributor

diptanu commented Aug 8, 2016

@a86c6f7964 Let's show created time by default since this essentially shows how long the allocation has been running for.

@camerondavison
Copy link
Contributor Author

@diptanu if an allocation is retried the creation time does not change. That table is already pretty wide if you turn on -verbose are we really sure that we want to add created to it for everything even though you can see much better information about start/stop/restart time in the nomad alloc-status command?

@diptanu
Copy link
Contributor

diptanu commented Aug 8, 2016

@a86c6f7964 Yeah it is ok to add the created time in the allocation table, as it provides a sense of the age of the allocation which is operationally relevant most of the time.

@@ -364,6 +358,12 @@ func (c *StatusCommand) outputFailedPlacements(failedEval *api.Evaluation) {
}
}

// formatUnixNanoTime is a helper for formatting time for output.
func (c *StatusCommand) formatUnixNanoTime(nano int64) string {
Copy link
Contributor

@dadgar dadgar Aug 8, 2016

Choose a reason for hiding this comment

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

Move this to same file as formatTime and it doesn't have to be an instance method

@dadgar
Copy link
Contributor

dadgar commented Aug 8, 2016

@camerondavison
Copy link
Contributor Author

okay I made changes from the feedback. thanks for the help. do you think that we should add creation time to the allocations that nomad node-status returns as well, since it now always showing up for allocations?

@diptanu
Copy link
Contributor

diptanu commented Aug 9, 2016

@a86c6f7964 Yes that would be nice, and in addition to the node-status we should have this in alloc-status too.

@camerondavison
Copy link
Contributor Author

are you waiting on something from me? Do you only want to merge this once the other issues are complete?

@@ -319,15 +301,16 @@ func (c *StatusCommand) outputJobInfo(client *api.Client, job *api.Job) error {
c.Ui.Output(c.Colorize().Color("\n[bold]Allocations[reset]"))
if len(jobAllocs) > 0 {
allocs = make([]string, len(jobAllocs)+1)
allocs[0] = "ID|Eval ID|Node ID|Task Group|Desired|Status"
allocs[0] = "ID|Eval ID|Node ID|Task Group|Desired|Status|Created"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change Created to Created At

@dadgar
Copy link
Contributor

dadgar commented Aug 15, 2016

LGTM. One small cosmetic comment

@camerondavison
Copy link
Contributor Author

changed to "Created At"

@dadgar
Copy link
Contributor

dadgar commented Aug 16, 2016

Nice thank you!