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] task logs in sidebar #14612

Merged
merged 15 commits into from
Sep 22, 2022
Merged

[ui] task logs in sidebar #14612

merged 15 commits into from
Sep 22, 2022

Conversation

philrenaud
Copy link
Contributor

@philrenaud philrenaud commented Sep 16, 2022

Resolves #14579

Adds the ability to view logs for a given task from a parent context:

  • Job page
  • Client page
  • Task group page
  • Job/allocations page

In other words: from within any context that we display an allocations table.

This adds a portalTarget to the application level to reduce overhead, and hoists the sidebar from the recently-added <TaskSubRow /> component.

The logs feed is using the same component and layout that its dedicated page uses.

image

TODO

  • Add for non-service jobs (each job part has its own template file)
  • Smoke testing in periodic/parameterized (child) jobs
  • Acceptance tests
  • Visual regression tests

@github-actions
Copy link

github-actions bot commented Sep 16, 2022

Ember Asset Size action

As of 74ff4e6

Files that got Bigger 🚨:

File raw gzip
nomad-ui.js +6.34 kB +1.11 kB
nomad-ui.css +1.69 kB +319 B

Files that stayed the same size 🤷‍:

File raw gzip
vendor.js 0 B 0 B
vendor.css 0 B 0 B

@github-actions
Copy link

github-actions bot commented Sep 16, 2022

Ember Test Audit comparison

main 97fd8d3 change
passes 1421 1420 -1
failures 0 2 +2
flaky 0 0 0
duration 11m 38s 250ms 000ms -11m 38s 250ms

@@ -10,7 +10,7 @@ export default class AllocationServiceSidebarComponent extends Component {
}
keyCommands = [
{
label: 'Close Evaluations Sidebar',
label: 'Close Service Sidebar',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Side-effect; adding correct name.

Comment on lines +93 to +95
if (this.shouldFillHeight) {
this.fillAvailableHeight();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

conditionalizing fillAvailableHeight(), as it tries to fill window.height, which is greater than our sidebar wants. Handled in CSS elsewhere for our sidebar logs element.

Comment on lines +34 to +36
vertical-align: baseline;
position: relative;
top: 1px;
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 affects (positively) a few others instances where we use our element and it was slightly misaligned.

@@ -19,7 +19,7 @@
"job-page/parts/latest-deployment" job=@job handleError=this.handleError
)
TaskGroups=(component "job-page/parts/task-groups" job=@job)
RecentAllocations=(component "job-page/parts/recent-allocations" job=@job)
RecentAllocations=(component "job-page/parts/recent-allocations" job=@job activeTask=@activeTask setActiveTaskQueryParam=@setActiveTaskQueryParam)
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, and further job-page/*.hbs file changes, were tricky to figure out how to best handle (and thank you to @DingoEatingFuzz for unblocking me!)

The nature of our higher-order components is such that, to get query parameters at the /job level, we have to declare our passed variables more verbosely than desired.

Copy link
Contributor

@ChaiWithJai ChaiWithJai left a comment

Choose a reason for hiding this comment

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

There's some areas for improvement, but not worth blocking on them.

ui/app/components/task-sub-row.hbs Show resolved Hide resolved
ui/app/templates/application.hbs Show resolved Hide resolved
Copy link
Contributor

@mikenomitch mikenomitch left a comment

Choose a reason for hiding this comment

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

LGTM!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ui] Task logs in sidebar
3 participants