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

Adds modify time to cli output #3449

Merged
merged 2 commits into from
Oct 28, 2017
Merged

Adds modify time to cli output #3449

merged 2 commits into from
Oct 28, 2017

Conversation

preetapan
Copy link
Member

Needs a rebase after #3446 is merged

@@ -75,6 +75,11 @@ func formatTimeDifference(first, second time.Time, d time.Duration) string {
return second.Truncate(d).Sub(first.Truncate(d)).String()
}

// formatTimePretty rounds off time difference to the nearest second for nicer display
func formatTimePretty(first, second time.Time) string {
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 use something like: https://godoc.org/github.com/dustin/go-humanize#Time

The output of this still could become huge if the allocation is many days old.

Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to specialize the 0 case since older allocations won't have the modify time and when it gets displayed it would show something like 40 years ago!

Copy link
Member Author

Choose a reason for hiding this comment

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

LOL, good way to remind people that for engineers, time doesn't exist prior to Jan 1 1970. Will fix

@@ -214,6 +214,9 @@ func (c *AllocStatusCommand) Run(args []string) int {
}

func formatAllocBasicInfo(alloc *api.Allocation, client *api.Client, uuidLength int, verbose bool) (string, error) {
formattedCreateTime := formatTimePretty(time.Unix(0, alloc.CreateTime), time.Now())
Copy link
Contributor

Choose a reason for hiding this comment

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

If verbose can we use, formatUnixNanoTime

Copy link
Contributor

Choose a reason for hiding this comment

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

Same with below

@@ -406,29 +406,33 @@ func formatAllocListStubs(stubs []*api.AllocationListStub, verbose bool, uuidLen

allocs := make([]string, len(stubs)+1)
if verbose {
allocs[0] = "ID|Eval ID|Node ID|Task Group|Version|Desired|Status|Created At"
allocs[0] = "ID|Eval ID|Node ID|Task Group|Version|Desired|Status|Created At|Modified At"
Copy link
Contributor

Choose a reason for hiding this comment

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

We should standardize on Created vs Created At and same with modify.

Copy link
Member Author

Choose a reason for hiding this comment

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

I left it this way because it is grammatically correct to say "Created at yyyy:mm:dd" and "Created 2 minutes ago". The other phrasing Created At x seconds ago sounds wrong. I'd be fine with changing both to Created

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I vote drop the At

@@ -75,6 +75,11 @@ func formatTimeDifference(first, second time.Time, d time.Duration) string {
return second.Truncate(d).Sub(first.Truncate(d)).String()
}

// formatTimePretty rounds off time difference to the nearest second for nicer display
func formatTimePretty(first, second time.Time) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to specialize the 0 case since older allocations won't have the modify time and when it gets displayed it would show something like 40 years ago!

…ring() with approximated values for days and beyond. Used in cli output for allocation create/modify times
d := second.Sub(first)
u := uint64(d.Seconds())

var buf [32]byte
Copy link
Member Author

Choose a reason for hiding this comment

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

I used a buffer for this similar to Duration.String() to prevent unnecessary allocations as we build up the output string.

@@ -422,8 +422,8 @@ func formatAllocListStubs(stubs []*api.AllocationListStub, verbose bool, uuidLen
} else {
allocs[0] = "ID|Node ID|Task Group|Version|Desired|Status|Created|Modified"
for i, alloc := range stubs {
createTimePretty := formatTimePretty(time.Unix(0, alloc.CreateTime), time.Now())
modTimePretty := formatTimePretty(time.Unix(0, alloc.ModifyTime), time.Now())
createTimePretty := prettyTimeDiff(time.Unix(0, alloc.CreateTime), time.Now())
Copy link
Contributor

Choose a reason for hiding this comment

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

Verbose switch

@preetapan preetapan merged commit 582c468 into b-nomad-0.7.1 Oct 28, 2017
@preetapan preetapan deleted the f-cli-modifytime branch November 15, 2017 14:23
@preetapan preetapan restored the f-cli-modifytime branch January 25, 2018 18:54
@preetapan preetapan deleted the f-cli-modifytime branch January 25, 2018 19:01
@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 Mar 13, 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.

None yet

2 participants