fix: align embedded engine run labels by yohamta0 · Pull Request #2014 · dagucloud/dagu

yohamta0 · 2026-04-19T14:55:28Z

Summary

align embedded run label overrides with core.DAG.Labels
add WithLabels for embedded runs while keeping WithTags as a deprecated alias
pass distributed embedded run labels through executor.WithLabels

Testing

make fmt
go test . ./internal/engine
make check (local go fix/go fmt passed; local golangci-lint exited before analysis with no go files to analyze, so GitHub CI should validate lint)

Summary by CodeRabbit

New Features
- Configuration for run metadata now uses labels instead of tags, providing updated support for specifying metadata
- Deprecated tags option remains fully available for backward compatibility with existing implementations
- Labels are now properly integrated across the entire execution pipeline including distributed task handling

coderabbitai · 2026-04-19T14:55:47Z

📝 Walkthrough

Walkthrough

This pull request renames the run-option mechanism from tags to labels across the engine package. It updates function names, struct fields, and internal logic while providing a deprecated WithTags wrapper for backwards compatibility.

Changes

Cohort / File(s)	Summary
API & Type Definitions `engine.go`, `types.go`	Renames `WithTags(...)` to `WithLabels(...)`, adds deprecated `WithTags` wrapper, updates `RunOptions` struct field from `Tags` to `Labels`, and changes field mapping from `iengine.RunOptions.Tags` to `iengine.RunOptions.Labels`.
Implementation `internal/engine/run.go`	Updates `applyRunOverrides` to merge override labels into DAG labels instead of tags; updates `runDistributed` to propagate labels via `runtimeexec.WithLabels(...)` and check `len(dag.Labels) > 0` instead of tags equivalents.
Tests `internal/engine/run_test.go`	Adds test `TestApplyRunOverridesMergesLabels` that verifies label merging behavior in `applyRunOverrides` with initial and override labels.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

feat: make DAG labels canonical #2013: Broad refactor that makes "labels" canonical across the codebase, replacing "tags" usages to align with this pull request's renaming.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: align embedded engine run labels' accurately reflects the main change: renaming tags to labels throughout the embedded engine run functionality.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix-embedded-engine-labels

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

🧹 Nitpick comments (2)

engine.go (1)
359-371: Public API change + deprecation alias LGTM.

WithLabels + deprecated WithTags forwarder preserves backwards compatibility. Two small nits:

Line 366 comment "adds labels to one run" for WithTags is fine but the Deprecated marker is the important part — the current wording is correct and go doc-friendly (blank line before Deprecated:).

Consider calling out the target version for removal in the deprecation notice (e.g., "Deprecated: use WithLabels; WithTags will be removed in vX.Y.") so downstream users can plan.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@engine.go` around lines 359 - 371, Update the Deprecated doc for the WithTags
forwarder to include a target removal version while preserving the blank line
before the "Deprecated:" marker; specifically edit the comment above the
WithTags function (which currently calls WithLabels) to read something like
"Deprecated: use WithLabels; WithTags will be removed in vX.Y." so downstream
users can plan, keeping the rest of the comment and the WithTags -> WithLabels
implementation unchanged.
internal/engine/run_test.go (1)
13-23: LGTM.

Test exercises both the dedup (env=prod kept once) and append (team=platform added) paths. Uses stretchr/testify/require per coding guidelines.

Optional: a second case asserting ordering when overrides come before existing labels, or that core.NewLabels normalizes equivalent forms (e.g., env = prod vs env=prod) would harden the contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/engine/run_test.go` around lines 13 - 23, Add an additional unit
test variant to harden TestApplyRunOverridesMergesLabels by asserting
deterministic ordering and normalization: add a test of applyRunOverrides using
RunOptions where overrides come before existing labels (e.g., Labels:
[]string{"team=platform", "env=prod"}) and another case where input uses spaced
forms (e.g., "env = prod") to verify core.NewLabels normalization; reference the
existing test function TestApplyRunOverridesMergesLabels and the
applyRunOverrides and core.NewLabels symbols when adding these assertions.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@engine.go`:
- Around line 359-371: Update the Deprecated doc for the WithTags forwarder to
include a target removal version while preserving the blank line before the
"Deprecated:" marker; specifically edit the comment above the WithTags function
(which currently calls WithLabels) to read something like "Deprecated: use
WithLabels; WithTags will be removed in vX.Y." so downstream users can plan,
keeping the rest of the comment and the WithTags -> WithLabels implementation
unchanged.

In `@internal/engine/run_test.go`:
- Around line 13-23: Add an additional unit test variant to harden
TestApplyRunOverridesMergesLabels by asserting deterministic ordering and
normalization: add a test of applyRunOverrides using RunOptions where overrides
come before existing labels (e.g., Labels: []string{"team=platform",
"env=prod"}) and another case where input uses spaced forms (e.g., "env = prod")
to verify core.NewLabels normalization; reference the existing test function
TestApplyRunOverridesMergesLabels and the applyRunOverrides and core.NewLabels
symbols when adding these assertions.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ca8beb58-32a5-4a11-848a-5e6cb2d22eed

📥 Commits

Reviewing files that changed from the base of the PR and between d502813 and 608e6ef.

📒 Files selected for processing (4)

engine.go
internal/engine/run.go
internal/engine/run_test.go
internal/engine/types.go

Merged features from upstream: - feat: add embedded Dagu API (dagucloud#2011) - feat: add edit-and-retry DAG runs (dagucloud#2010) - feat: add bulk DAG-run deletion in web UI (dagucloud#2009) - feat: add kubernetes secret provider (dagucloud#2006) - feat: make workspace selection global (dagucloud#2015) - feat: show cockpit run artifacts (dagucloud#2017) - feat: allow disabling DAG retry policy (dagucloud#2018) - feat: make DAG labels canonical, deprecate tags (dagucloud#2013) - feat: add workflow design workspace (dagucloud#2012) - feat: add step with/config alias (dagucloud#2021) - fix: allow runtime custom step inputs (dagucloud#2005) - fix: align embedded engine run labels (dagucloud#2014) - Various CI/test/docs improvements Conflict resolution: - Kept fork i18n files (ui/src/i18n/, LanguageSwitcher) - Merged i18n hooks with upstream structural changes in App.tsx, Layout.tsx, menu.tsx, DAGStatus.tsx, etc. - Adopted upstream labels-over-tags rename (TagCombobox -> LabelCombobox)

fix: align embedded engine run labels

608e6ef

coderabbitai Bot reviewed Apr 19, 2026

View reviewed changes

yohamta0 merged commit f8d2e69 into main Apr 19, 2026
10 checks passed

yohamta0 deleted the fix-embedded-engine-labels branch April 19, 2026 15:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: align embedded engine run labels#2014

fix: align embedded engine run labels#2014
yohamta0 merged 1 commit into
mainfrom
fix-embedded-engine-labels

yohamta0 commented Apr 19, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 19, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

yohamta0 commented Apr 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

yohamta0 commented Apr 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 19, 2026 •

edited

Loading