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!

@dadgar dadgar merged commit cc4b0ff into hashicorp:master Aug 16, 2016
@diptanu
Copy link
Contributor

diptanu commented Aug 16, 2016

@a86c6f7964 Thanks, merging this one! Are you planning to tackle the two issues anytime soon?

@dadgar
Copy link
Contributor

dadgar commented Aug 16, 2016

We merged within 10 seconds of each other!

@camerondavison
Copy link
Contributor Author

I would like to work on the other issues next, but I have been trying to get tests to pass.

@camerondavison camerondavison deleted the add-creation-time-to-job-status branch December 28, 2016 16:58
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 12, 2023
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.

Print allocation creation time with 'nomad status'.
3 participants