Repository navigation
Conversation
Signed-off-by: William Tang <williamtang83@hotmail.com>
Icewind52
force-pushed
the
feat/runs-count-active-actions
branch
from
October 28, 2026 01:29
7a2717e to
9d61e90
Compare
Signed-off-by: William Tang <williamtang83@hotmail.com>
Icewind52
force-pushed
the
feat/runs-count-active-actions
branch
from
October 28, 2026 06:14
4c5b32a to
44c274a
Compare
Replace the loop that derived nonTerminalPhases with an explicit activePhases list covering every phase not in terminalPhases. Make TestCountActionsByPhase easier to read: label each expected count with its seeded action and assert the active run and action totals. Signed-off-by: William Tang <williamtang83@hotmail.com>
pingsutw
reviewed
Oct 31, 2026
Member
|
LGTM, just one nit |
Signed-off-by: William Tang <williamtang83@hotmail.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the live-action counting changes.
No open issues 🎉
1 file not reviewed
runs/repository/mocks/mocks.go: Generated file
What changed in this PR
Adds repository support for counting live actions, preparing for the flyte.active_runs and flyte.active_actions gauges tracked in #8235. The sampler remains a follow-up change.
Changes:
- Adds
CountActiveActionsByPhase, returning counts keyed by phase and root/child status. - Filters the aggregate query to non-terminal phases.
- Updates the mock and adds database tests for empty results, terminal exclusion, and root/child separation.
| File | Description |
|---|---|
runs/repository/mocks/mocks.go |
Adds mock support for the new count method. |
runs/repository/interfaces/action.go |
Defines the count method and composite PhaseKey. |
runs/repository/impl/action.go |
Defines active phases and implements the aggregate query. |
runs/repository/impl/action_test.go |
Tests filtering, grouped counts, and gauge totals. |
Review effort: Balanced
0yukali0
approved these changes
Oct 31, 2026
0yukali0
left a comment
Contributor
There was a problem hiding this comment.
+1, LGTM.
The new test is also ideal.
It uses both active and non-active actions to verify that the function returns the correct counts grouped by phase and is_root.
This branch has not been deployed
This file contains hidden or 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
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.
Tracking issue
Related to #8235
Why are the changes needed?
The
flyte.active_runsandflyte.active_actionsgauges in #8235 need a count of live actions from the database. This adds the repo method the runs sampler will call. The sampler itself is a follow-up PR.What changes were proposed in this pull request?
Adds
ActionRepo. CountActiveActionsByPhase, as suggested in the issue, with two adjustments:(phase, is_root), but amap[int32]int64keyed by phase alone would let the root and child rows for the same phase overwrite each other. The method returnsmap[PhaseKey]int64withPhaseKey{Phase, IsRoot}instead.WHERE phase = ANY(<non-terminal phases>)so it reads only live actions.Note: FAILED and TIMED_OUT count as terminal, so an action waiting between retries isn't counted until it moves back to a live phase.
How was this patch tested?
TestCountActiveActionsByPhase(embedded Postgres): an empty table, plus live and finished runs with children in every phase. It checks that only non-terminal actions are counted and that roots and children are split.Mock now regenerated. Adding a method to
ActionRepomeansruns/repository/mocks/mocks.goneeds regenerating, butmake mockscurrently fails, both locally and in CI:This started with #7949 (Go 1.26.5 → 1.27.2 in
gen.Dockerfile, mockery still v3.7.0). The first failingcheck-mocksrun is 37968610418, and it still fails today (40579043741). The job stays green because thetimedmacro in the Makefile ignores mockery's exit code, so nothing is written andgit difffinds no changes.make mocksexits 0 locally too.This is what I observed, not sure if this is a real regression.
Labels
Check all the applicable boxes