-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[ui] Pack metadata booleanified and added to the statuses endpoint #23404
Conversation
52580dc
to
d4ed5b8
Compare
nomad/job_endpoint_statuses.go
Outdated
isPack := false | ||
if job.Meta != nil { | ||
for key, value := range job.Meta { | ||
if strings.HasPrefix(key, "pack") && value != "" { | ||
isPack = true | ||
break | ||
} | ||
} | ||
} | ||
|
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.
This could very well be too expensive for this endpoint, but I'm a little blind to how much we ought to tolerate here. For example, the possibility of a job with many, many instances of metadata entries gives me pause. If there is a more clear-cut indicator of "this came from pack", I'd love to use that, but I think metadata is the best bet.
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.
since we know specific pack.
meta key(s), instead of iterating the map which could potentially be arbitrarily long, as you mentioned, and getting string processing involved, I think we can do a simple map lookup. also fun tidbit: you can attempt to access a nil map without nil-checking it, so I think all this can boil down to:
_, isPack := job.Meta["pack.name"]
but also, and this is totally personal taste, I like to do static type definition of what I'm gonna return up top, then populate it with whatever extra processing afterwards. so I'd personally do this down underneath jsj
definition somewhere instead:
_, jsj.IsPack = job.Meta["pack.name"]
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.
This is way simpler and setting it unorphaned makes a lot of sense. Updating now!
Ember Test Audit comparison
|
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.
LGTM!
4d4a1cb
to
ed028f0
Compare
Context: Way back in Two-ought-twenty-two, we added a visual indicator for whether a job was run via Nomad Pack in the UI. We could do this because the /jobs endpoint returns job meta information, and pack consistently puts
meta.pack.name
etc. such that we can detect it.With the purpose-built /statuses endpoint, we can be choosy about what we want to include and by default weren't delving into job metadata.
This PR checks to see if any of the tell-tale signs of pack are present, and if so, presents an
isPack
boolean to the endpoint that the UI turns into a little badge: