-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 timestamps to evals #5881
add timestamps to evals #5881
Changes from all commits
472c7f0
0a72f11
bd0dcad
a8d464c
9a384b8
56e60d3
0ce1aca
04b943c
a405a3a
d855aaa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -235,6 +235,7 @@ func (a *Alloc) Stop(args *structs.AllocStopRequest, reply *structs.AllocStopRes | |
return fmt.Errorf(structs.ErrUnknownAllocationPrefix) | ||
} | ||
|
||
now := time.Now().UTC().UnixNano() | ||
eval := &structs.Evaluation{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Someday a helper func for making evals might be nice to ensure we always set the time properly: func NewEval(namespace, type string) *Evaluation {
// get time.now, generate uuid, and set basic fields here
} Not a blocker for this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. This was discussed but I opted not to since a big refactor on all eval creations seemed risky and outside of the scope of this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Too bad Go doesn't have constructors! 🐣 |
||
ID: uuid.Generate(), | ||
Namespace: alloc.Namespace, | ||
|
@@ -244,6 +245,8 @@ func (a *Alloc) Stop(args *structs.AllocStopRequest, reply *structs.AllocStopRes | |
JobID: alloc.Job.ID, | ||
JobModifyIndex: alloc.Job.ModifyIndex, | ||
Status: structs.EvalStatusPending, | ||
CreateTime: now, | ||
ModifyTime: now, | ||
} | ||
|
||
transitionReq := &structs.AllocUpdateDesiredTransitionRequest{ | ||
|
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.
Pointing out that for old evals this will print an empty string, which may be ok but odd UX.
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.
Agreed it's not the greatest UX. This behavior is consistent with
nomad alloc status
, although it's unlikely at this point that there are old allocs w/o a CreateTime. Going to think about it, will most likely leave as is but add a note in the docs.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.
Leaving it as is sounds good to me if there's prior art already that does this.