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: Refactor all displayed units to use common utils (featuring bigger suffixes) #10257

Merged
merged 7 commits into from
Mar 31, 2021

Conversation

DingoEatingFuzz
Copy link
Contributor

Originally I just wanted to make things go THz, PHz, TiB, and PiB. But upon doing so it seemed practical to also make all formatting go through the same code paths. This was a bigger undertaking than anticipated 😄

First, the new stuff before the stuffy details:

image
A large and quite empty cluster showing aggregate capacity figures reduced to terabytes of memory and terahertz of cpu shares.

Okay, now the stuffy details.

A new utils/units.js file contains all the units logic including:

  1. Raw reducers returning the quantity and the unit separately
  2. Generic formatters that will reduce values as much as possible, optionally taking a starting unit (since memory is often already in megabytes).
  3. Special formatters for "scheduled" figures.

Then there are four helpers that use this file to expose the utils to templates.

I think the first two options listed above make immediate sense: gotta format stuff, gotta have structured output for fancy HTML.

The third option is a style guide proposal of sorts. I think sometimes it makes sense to see numbers in the human format (i.e., reduced as much as possible) and other times it makes sense to see numbers in the scheduler format (i.e., in MiB and MHz regardless of size since that's what job files and such use).

The rule of thumb is to always use the human format unless the number represents one task, allocation, or client, and the number is not realtime utilization.

@github-actions
Copy link

github-actions bot commented Mar 30, 2021

Ember Asset Size action

As of 20c3b98

Files that got Bigger 🚨:

File raw gzip
nomad-ui.js +3.41 kB +646 B

Files that stayed the same size 🤷‍:

File raw gzip
vendor.js 0 B 0 B
nomad-ui.css 0 B 0 B
vendor.css 0 B 0 B

@github-actions
Copy link

github-actions bot commented Mar 30, 2021

Ember Test Audit comparison

main 20c3b98 change
passes 1019 1072 +53
failures 0 0 0
flaky 0 0 0
duration 7m 42s 294ms 7m 41s 521ms -773ms

@github-actions
Copy link

Ember Test Audit flaky tests

Ember Test Audit detected these flaky tests on a02adc6:

  • Acceptance | topology: by default the info panel shows cluster aggregate stats

@backspace
Copy link
Contributor

Before I formally review, I was able to reproduce a failure in the aforementioned flaky test:

image

Copy link
Contributor

@backspace backspace left a comment

Choose a reason for hiding this comment

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

I love it!! I appreciate this all being centralised and rationalised. There’s the flaky test and the logging statements but apart from that this looks great. Your style guide-esque proposal makes sense to me.

ui/tests/unit/utils/units-test.js Show resolved Hide resolved
ui/app/controllers/topology.js Outdated Show resolved Hide resolved
ui/tests/acceptance/optimize-test.js Show resolved Hide resolved
ui/tests/acceptance/optimize-test.js Outdated Show resolved Hide resolved
ui/app/helpers/format-hertz.js Show resolved Hide resolved
Copy link
Contributor

@backspace backspace left a comment

Choose a reason for hiding this comment

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

total success 🥳

@DingoEatingFuzz DingoEatingFuzz merged commit cd1d533 into main Mar 31, 2021
@DingoEatingFuzz DingoEatingFuzz deleted the f-ui/bigger-units branch March 31, 2021 22:18
@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 Nov 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants