DEVPROD-4573: Query hosts by Running status for Task Queue UI #7690
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ablack12
requested changes
Mar 29, 2024
globals.go
Outdated
| @@ -956,11 +956,10 @@ var ( | |||
| HostStarting, | |||
| } | |||
|
|
|||
| // Hosts in "initializing" status aren't actually running yet: | |||
| // Hosts in "initializing" and "building" status aren't actually running yet: | |||
There was a problem hiding this comment.
I'm not able to open the ticket because of the Jira outage, but is the context for this the idea of omitting building hosts from the task queue page? Can that query instead just use the running status? (That's the only status that's able to run tasks in this list, so its the only status we should consider). The comment as you've edited it here is false; "Building" hosts are running hosts.
There was a problem hiding this comment.
That makes sense, I will update this PR so the host count on the Task Queue page represents the Running status only.
SupaJoon
changed the title
SupaJoon
changed the title
ablack12
approved these changes
Apr 3, 2024
model/host/db.go
Outdated
| @@ -232,6 +232,16 @@ func runningHostsQuery(distroID string) bson.M { | |||
| return query | |||
| } | |||
|
|
|||
| // byRunningStatusQuery produces a query that returns all hosts | |||
| // with the running status that belong to the given. | |||
There was a problem hiding this comment.
nit: missing "distro" and the end there i think
model/host/db.go
Outdated
| @@ -263,6 +273,11 @@ func CountRunningHosts(ctx context.Context, distroID string) (int, error) { | |||
| return num, errors.Wrap(err, "counting running hosts") | |||
| } | |||
|
|
|||
| func CountRunningStatusHosts(ctx context.Context, distroID string) (int, error) { | |||
| num, err := Count(ctx, byRunningStatusQuery(distroID)) | |||
| return num, errors.Wrap(err, "counting Running status hosts") | |||
There was a problem hiding this comment.
nit: shouldn't capitalize running
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DEVPROD-4573
Description
These queries hosts by the running status for the Task Queue UI host count. Originally, the hostCount considered hosts with any of these statuses but now it only considers
running.