Skip to content

Comments

feat: coordinator flag#1662

Merged
yottahmd merged 6 commits intomainfrom
feat-coordinator-flag
Feb 13, 2026
Merged

feat: coordinator flag#1662
yottahmd merged 6 commits intomainfrom
feat-coordinator-flag

Conversation

@yottahmd
Copy link
Collaborator

@yottahmd yottahmd commented Feb 13, 2026

Summary by CodeRabbit

  • New Features

    • Coordinator service is now optional and configurable. Introduced the DAGU_COORDINATOR_ENABLED environment variable (defaults to enabled) to control whether the coordinator runs, providing flexibility for distributed execution setups.
  • Documentation

    • Updated configuration schema and help text to document the new coordinator enablement option.

@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This change introduces a configuration-driven feature flag DAGU_COORDINATOR_ENABLED to conditionally enable/disable the coordinator service. Updates include adding the flag to configuration structures, loader logic, environment variable binding, and conditional client/service initialization throughout the startup sequence.

Changes

Cohort / File(s) Summary
Documentation & Schema
README.md, internal/cmn/schema/config.schema.json
Added DAGU_COORDINATOR_ENABLED environment variable documentation and schema definition (default: true) to coordinator configuration.
Configuration Definition
internal/cmn/config/config.go, internal/cmn/config/definition.go
Added Enabled bool field to Coordinator struct and Enabled *bool field to CoordinatorDef struct with mapstructure tag for configuration binding.
Configuration Loading
internal/cmn/config/loader.go, internal/cmn/config/loader_test.go
Implemented resolveCoordinatorEnabled function to resolve enablement state from viper config, definition pointer, or default (true). Added environment variable binding for COORDINATOR_ENABLED and test coverage for various enablement scenarios (default, YAML override, environment override).
Runtime & Client Initialization
internal/cmd/context.go, internal/cmd/startall.go
Added early-return guard in NewCoordinatorClient when coordinator is disabled. Replaced localhost-binding heuristic with config-driven enablement; made coordinator Start/Stop and cleanup conditional on coordinator presence (nil check). Updated help text and logging to reflect optional coordinator behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • #1577: Modifies coordinator server/client options and handler behavior that this PR now conditionally initializes and manages based on enablement flag.
🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: coordinator flag' directly aligns with the main change: introducing a new DAGU_COORDINATOR_ENABLED environment variable to toggle the coordinator service on/off.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

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

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-coordinator-flag

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@internal/cmn/config/loader.go`:
- Around line 627-635: The def.Coordinator.Enabled branch in
resolveCoordinatorEnabled is unreachable because
l.v.SetDefault("coordinator.enabled", true) ensures IsSet is always true; remove
the dead branch that checks def.Coordinator.Enabled and simplify
resolveCoordinatorEnabled to return l.v.GetBool("coordinator.enabled") (with a
fallback true only if you prefer), keeping references to
resolveCoordinatorEnabled, l.v.IsSet/GetBool, and def.Coordinator.Enabled in
your edit so reviewers can verify the change; alternatively, if you prefer the
pattern used by resolveTunnelEnabled, remove the
l.v.SetDefault("coordinator.enabled", true) call instead so the
def.Coordinator.Enabled branch becomes reachable—pick one approach and update
the code accordingly.

@yottahmd yottahmd merged commit d496dde into main Feb 13, 2026
5 checks passed
@yottahmd yottahmd deleted the feat-coordinator-flag branch February 13, 2026 13:36
@codecov
Copy link

codecov bot commented Feb 13, 2026

Codecov Report

❌ Patch coverage is 64.10256% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.16%. Comparing base (5394d09) to head (938331f).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
internal/cmd/startall.go 58.62% 9 Missing and 3 partials ⚠️
internal/cmn/config/loader.go 75.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1662      +/-   ##
==========================================
- Coverage   70.18%   70.16%   -0.02%     
==========================================
  Files         344      345       +1     
  Lines       38257    38664     +407     
==========================================
+ Hits        26851    27130     +279     
- Misses       9273     9370      +97     
- Partials     2133     2164      +31     
Files with missing lines Coverage Δ
internal/cmd/context.go 70.46% <100.00%> (-0.17%) ⬇️
internal/cmn/config/config.go 76.28% <ø> (ø)
internal/cmn/config/loader.go 82.99% <75.00%> (-0.09%) ⬇️
internal/cmd/startall.go 69.49% <58.62%> (+0.68%) ⬆️

... and 23 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5394d09...938331f. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant