fix: correct tag-base search for dag-runs impl #1586
Conversation
yottahmd
changed the title
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR adds tag-based filtering for DAG runs throughout the system. It extends DAG run data models with an optional Tags field, implements server-side tag filtering in the store layer, propagates DAG-level tags into run statuses, and removes redundant client-side filtering logic. Additionally, command parsing is enhanced to support single-key maps formatted as "key: value" pairs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@internal/core/spec/step.go`:
- Around line 861-875: Update the error strings passed to
core.NewValidationError in the command-array parsing block so they don't end
with a period (to satisfy ST1005): change the literal in the nested case where
fmt.Errorf("command array elements must be strings. If this contains a colon,
wrap it in quotes. Got nested %T", v2) and the non-nested case
fmt.Errorf("command array elements must be strings. If this contains a colon,
wrap it in quotes.") to remove or replace the trailing period/terminal
punctuation (e.g., drop the final period or use a semicolon) while leaving the
rest of the messages and the core.NewValidationError(fmt.Sprintf("command[%d]",
i), v, ...) calls unchanged.
🧹 Nitpick comments (1)
internal/service/frontend/api/v2/transformer.go (1)
134-151: Consider omitempty behavior for Tags pointers.Using
&s.Tagsalways yields a non-nil pointer, soomitemptywon’t omit the field when tags are absent. If you want the field omitted when there are no tags, gate the pointer onlen(s.Tags) > 0.♻️ Suggested change
func toDAGRunSummary(s exec.DAGRunStatus) api.DAGRunSummary { + var tags *[]string + if len(s.Tags) > 0 { + tags = &s.Tags + } return api.DAGRunSummary{ RootDAGRunName: s.Root.Name, RootDAGRunId: s.Root.ID, @@ - Tags: &s.Tags, + Tags: tags, } } func toDAGRunDetails(s exec.DAGRunStatus) api.DAGRunDetails { + var tags *[]string + if len(s.Tags) > 0 { + tags = &s.Tags + } preconditions := make([]api.Condition, len(s.Preconditions)) @@ - Tags: &s.Tags, + Tags: tags, } }Also applies to: 154-184
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1586 +/- ##
==========================================
- Coverage 64.78% 64.78% -0.01%
==========================================
Files 259 259
Lines 28833 28871 +38
==========================================
+ Hits 18680 18703 +23
- Misses 8480 8492 +12
- Partials 1673 1676 +3
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.