-
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] Allocation route services table: show task-level services #14199
[ui] Allocation route services table: show task-level services #14199
Conversation
Ember Asset Size actionAs of f14031c Files that got Bigger 🚨:
Files that stayed the same size 🤷:
|
Ember Test Audit comparison
|
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.
I think that Service Discovery is supposed to be a real-time feature. Giving this an approval so you're not blocked on me, however.
@@ -47,10 +48,22 @@ export default class IndexController extends Controller.extend(Sortable) { | |||
} | |||
|
|||
@computed('[email protected]') | |||
get services() { | |||
get groupServices() { | |||
return (this.get('model.taskGroup.services') || []).sortBy('name'); |
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.
suggestion:
This is an asynchronous request. allocations.allocation
does not load the TaskGroup
and this should trigger a fetch request. TaskGroup
can fail as a request here and we won't notify the user.
return (this.get('model.taskGroup.services') || []).sortBy('name'); | ||
} | ||
|
||
@computed('[email protected]') |
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.
question
: Is this the correct dependent key?
Hey @ChaiWithJai — you're correct that the Service Discovery UI feature will be realtime. However, you might call this pre-work: the existing services table, currently on the allocations page in the UI, does not show any task-level services. All this PR seeks to do is to extend that already-existing functionality to include task-level services, too. |
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.
neat!
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. |
(resolves #14198)
Adds service fragments to allocations and union taskGroup and task services. Works with both Consul and Nomad service providers.
Before (Nomad)
After (Nomad)
After (Consul)