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

[ui] Pack metadata booleanified and added to the statuses endpoint #23404

Merged
merged 2 commits into from
Jul 19, 2024

Conversation

philrenaud
Copy link
Contributor

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:

image

@philrenaud philrenaud added the backport/1.8.x backport to 1.8.x release line label Jun 20, 2024
@philrenaud philrenaud requested review from jrasell and gulducat June 20, 2024 19:47
@philrenaud philrenaud self-assigned this Jun 20, 2024
@philrenaud philrenaud force-pushed the f-ui/pack-meta-on-statuses branch from 52580dc to d4ed5b8 Compare June 20, 2024 19:49
Comment on lines 204 to 213
isPack := false
if job.Meta != nil {
for key, value := range job.Meta {
if strings.HasPrefix(key, "pack") && value != "" {
isPack = true
break
}
}
}

Copy link
Contributor Author

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.

Copy link
Member

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"]

Copy link
Contributor Author

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!

Copy link

github-actions bot commented Jun 20, 2024

Ember Test Audit comparison

main ed028f0 change
passes 1578 1578 0
failures 0 0 0
flaky 0 0 0
duration 11m 38s 003ms 11m 40s 569ms +02s 566ms

Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.8.x backport to 1.8.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants