-
Notifications
You must be signed in to change notification settings - Fork 2k
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] Show ALL regions' leaders when viewing servers route #24723
base: main
Are you sure you want to change the base?
[ui] Show ALL regions' leaders when viewing servers route #24723
Conversation
16ad689
to
98d0de0
Compare
@tracked isLeader = false; | ||
|
||
@action async checkForLeadership() { | ||
const leaders = await this.system.leaders; | ||
this.isLeader = leaders.includes(this.rpcAddr); | ||
return this.isLeader; |
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.
using tracked
is more idiomatic than the isLeader getter in Ember Octane. And since service getters (like this.get('system.leader....)
was the thing holding it back as a classic model, removed the @classic
tag as well.
.authorizedRequest(`/${namespace}/status/leader?region=${region}`) | ||
.then((res) => res.json()); | ||
}) | ||
); |
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.
Best as I can tell, the only thing that uses system.leader was the servers page, which is now using .leaders instead. Removed the old one.
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.
What's the ${namespace}
referring to here? v1
?
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.
Yeah, v1
! It's my third favourite among the three ways we use the term "namespace" in the UI.
It imports from
nomad/ui/app/adapters/application.js
Line 15 in 0324e78
export const namespace = 'v1'; |
|
||
beforeModel() { | ||
return this.get('system.leader'); | ||
} |
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.
As best as I can tell, this was accidentally left in here by copying the servers route page ~9 years ago.
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.
@text={{if | ||
this.agent.isLeader | ||
(if | ||
this.agent.system.shouldShowRegions | ||
(concat "True" " (" this.agent.region ")") | ||
"True" | ||
) | ||
"False" | ||
}} |
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.
Funky formatting, but:
- If not a leader, just show "False"
- If a leader, show "True"
- If there are multiple regions at play, show region name in parentheses.
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!
Resolves #24309
In a multi-region setup:
In a single-region setup: