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] Jobspec UI block: Descriptions and Links #18292
[ui] Jobspec UI block: Descriptions and Links #18292
Conversation
7f7a8ca
to
524fe53
Compare
Ember Test Audit comparison
|
f86dd0b
to
bafe2ed
Compare
This is a great proposal, +1 |
7beb105
to
57b935d
Compare
}, | ||
}, | ||
Expected: &JobDiff{ | ||
Type: DiffTypeEdited, |
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.
Weirdly no, because there's an empty JobUIConfig
block here, which the new job populates.
nomad/structs/structs.go
Outdated
@@ -4554,6 +4594,7 @@ func (j *Job) Copy() *Job { | |||
nj.Constraints = CopySliceConstraints(nj.Constraints) | |||
nj.Affinities = CopySliceAffinities(nj.Affinities) | |||
nj.Multiregion = nj.Multiregion.Copy() |
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 reads strangely but still works the same either way. After we do *nj = *j
, then nj.UI
and j.UI
are pointing to the same location in memory. By doing nj.UI = nj.UI.Copy()
we're copying that object to a new memory location and assigning that pointer to nj.UI
but not j.UI
.
For readability, it does feel like it'd make more sense to use j
as the receiver for all 5 of these deep copy lines, but it's harmless.
Wondering what it would take to perhaps have a config flag that gates this functionality for the UI? |
@david-yu I believe a sentinel policy could target this block. |
That could possibly work. May be a good suggestion for a template to add? |
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.
Last item on the pointer on JobUIConfig.Links
and this should be good-to-go
61948b0
to
da068a8
Compare
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!
Juanita is out and I've verified the PR addresses here concerns
Adds a
ui
block to Nomad job specs. Users can define any number oflink
s and a singledescription
that will show up on the Job index page in the web UI:from spec
Notably, this supports markdown, so you can, for example, add a table with a jobspec like this:
Resolves #18127
Resolves #15706