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] Adds meta k/v tables to Task Group and Task pages #24594

Merged

Conversation

philrenaud
Copy link
Contributor

@philrenaud philrenaud commented Dec 2, 2024

Description

In addition to job.meta, job.taskGroups[].meta and job.taskGroups[].tasks[].meta blocks are valid jobspec blocks (docs)

This PR adds tables to show their key/values on relevant gruop and task pages that have them. This uses the same table that is otherwise shown on job index pages (or parameterized/periodic job dispatches).

Testing & Reproduction steps

Add a meta block with some entries to one your jobs' taskGroups and tasks, and navigate to those in the web UI.

Resolves #24345

@philrenaud philrenaud force-pushed the 24345-feature-request-ui-show-group-meta-in-task-group-view branch from 438f37d to 5902561 Compare December 3, 2024 16:27
@philrenaud philrenaud force-pushed the 24345-feature-request-ui-show-group-meta-in-task-group-view branch from 4e1bb21 to 2f5069a Compare December 3, 2024 20:53
@philrenaud philrenaud force-pushed the 24345-feature-request-ui-show-group-meta-in-task-group-view branch from 54c2ad0 to 3c49a56 Compare December 6, 2024 05:26
@philrenaud philrenaud changed the title Experimenting with a generic meta job-part component [ui] Adds meta k/v tables to Task Group and Task pages Dec 6, 2024
@philrenaud philrenaud force-pushed the 24345-feature-request-ui-show-group-meta-in-task-group-view branch from 3c49a56 to 935d000 Compare December 6, 2024 05:32
@philrenaud philrenaud marked this pull request as ready for review December 6, 2024 05:36
@philrenaud philrenaud requested review from a team as code owners December 6, 2024 05:36
@@ -60,6 +60,8 @@ export default create({
clientSource: text('[data-test-volume-client-source]'),
}),

hasMeta: isPresent('[data-test-meta]'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are "test pages": a way to have nice syntactic sugar in our actual tests (like assert.ok(task.hasMeta)) instead of making the tests themselves feel verbose.


if (task.withMeta) {
task.update({ meta: { raw: { foo: 'bar' } } });
}
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 a factory file! Used by our mocking service (Mirage), and our tests, this gives us some private convenience methods when creating mocked objects like tasks/task groups. In this case, applying withTaskMeta to a task group will make a group with as many allocations/tasks as you'd like, each of which having some basic metadata.

@@ -22,7 +22,7 @@
PlacementFailures=(component "job-page/parts/placement-failures" job=@job)
TaskGroups=(component "job-page/parts/task-groups" job=@job)
RecentAllocations=(component "job-page/parts/recent-allocations" job=@job activeTask=@activeTask setActiveTaskQueryParam=@setActiveTaskQueryParam)
Meta=(component "job-page/parts/meta" job=@job)
Meta=(component "job-page/parts/meta" meta=@job.meta)
Copy link
Contributor Author

@philrenaud philrenaud Dec 6, 2024

Choose a reason for hiding this comment

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

This generally changes the context from "This is a component for jobs, it knows how to get metadata from a job" to "This is a component for metadata, it is given metadata"

get mergedMeta() {
return {
...this.taskGroup.mergedMeta,
...this.meta,
...this.meta?.raw,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You might notice that this is using the ...this.meta.raw format where the above task-group model is using this.get('meta.raw'). The get syntax is valid JS and everything, but is more of an old-ember convention, and task group models have existed longer than task models in our codebase.

You can see these old-ember ways of doing things scattered throughout: look for a @classic decorator near the top of the class declaration.

@philrenaud philrenaud force-pushed the 24345-feature-request-ui-show-group-meta-in-task-group-view branch from 01675ae to 1fb8c7e Compare December 6, 2024 05:46
Copy link
Member

@shoenig shoenig left a comment

Choose a reason for hiding this comment

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

LGTM; comments are helpful!

@philrenaud philrenaud added the backport/1.9.x backport to 1.9.x release line label Dec 17, 2024
@philrenaud philrenaud merged commit 932c3eb into main Dec 17, 2024
25 of 27 checks passed
@philrenaud philrenaud deleted the 24345-feature-request-ui-show-group-meta-in-task-group-view branch December 17, 2024 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.9.x backport to 1.9.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature request: ui: show group meta in task group view
2 participants