feat: auto-create default base.yaml with comprehensive field reference on first run#1737
feat: auto-create default base.yaml with comprehensive field reference on first run#1737
Conversation
…e on first run When Dagu starts and no base.yaml exists, a fully-commented default is created as a discoverable reference for all inheritable DAG settings. Uses marker file (.base-config-created) to avoid re-creation if the user intentionally deletes the file. Respects the skip_examples config flag.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request adds automatic initialization of a default base configuration file for DAG settings. When a context is created with a configured base config path, the system lazily creates a template YAML file with commented-out configuration options unless skipped or the file already exists. Changes
Sequence Diagram(s)sequenceDiagram
participant Context as Context<br/>(cmd)
participant Store as Store<br/>(filebaseconfig)
participant FileSystem as File System
Context->>Store: filebaseconfig.New(WithSkipDefault(...))
Store-->>Context: Store instance
Context->>Store: Initialize()
activate Store
Store->>Store: Check skipDefault flag
Store->>FileSystem: Check if base config file exists
FileSystem-->>Store: File status
Store->>FileSystem: Check if marker file exists
FileSystem-->>Store: Marker status
Store->>FileSystem: Write default config (0600)
FileSystem-->>Store: File written
Store->>FileSystem: Write marker file
FileSystem-->>Store: Marker written
deactivate Store
Store-->>Context: nil or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (1)
internal/persis/filebaseconfig/store.go (1)
66-86: Consider logging or handling the marker file write error.Line 84 ignores the error when writing the marker file. While the primary config file write is handled correctly, silently ignoring marker file errors could lead to the default config being re-created on every startup if the marker consistently fails to write (e.g., due to permissions issues after the atomic write succeeds).
This is low-risk since re-creating an identical default file is idempotent, but logging a warning would aid debugging.
♻️ Optional: Log marker file write failure
- _ = os.WriteFile(markerFile, markerContent, filePermissions) + if err := os.WriteFile(markerFile, markerContent, filePermissions); err != nil { + // Non-fatal: worst case is re-creating the same default on next startup + return nil + }Alternatively, if you have access to a logger in this package, log a warning instead of silently ignoring.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/persis/filebaseconfig/store.go` around lines 66 - 86, The Initialize method currently ignores the error returned by os.WriteFile when creating markerFile; update Store.Initialize to handle that error by checking the os.WriteFile return value and emitting a warning (via the package logger if available, otherwise fmt.Fprintf to stderr) including the markerFile path and the error; do not change the primary return behavior (still return nil on success), but ensure failures writing markerFile are logged so repeated re-creation of defaultBaseConfig can be diagnosed (refer to Store.Initialize, markerFile, markerContent, and filePermissions).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/cmd/context.go`:
- Around line 252-262: The cfg.Core.SkipExamples flag is being misused to
control base config creation; introduce a new boolean option (e.g.,
cfg.Core.SkipBaseConfig) and use it when calling
filebaseconfig.New(cfg.Paths.BaseConfig, filebaseconfig.WithSkipDefault(...))
instead of cfg.Core.SkipExamples so example DAG creation and base-config
auto-creation can be toggled independently; also ensure filebaseconfig.New
failures are not swallowed—when bcErr != nil, call logger.Warn(ctx, "Failed to
create base config store", tag.Error(bcErr)) (similar to the existing
bcStore.Initialize error handling that uses logger.Warn with
tag.Error(initErr)), and keep the existing bcStore.Initialize error logging for
initErr.
In `@internal/persis/filebaseconfig/default_base_config.go`:
- Around line 1-6: This file is missing the required GPL v3 license header; run
the project's license tool to add it: invoke the repository's make addlicense
target (e.g., `make addlicense`) to insert the standard GPLv3 header at the top
of internal/persis/filebaseconfig/default_base_config.go so the header appears
before the package declaration and constants like defaultBaseConfig; do not
manually edit the file header—use the automated tool to ensure the correct
boilerplate is applied.
---
Nitpick comments:
In `@internal/persis/filebaseconfig/store.go`:
- Around line 66-86: The Initialize method currently ignores the error returned
by os.WriteFile when creating markerFile; update Store.Initialize to handle that
error by checking the os.WriteFile return value and emitting a warning (via the
package logger if available, otherwise fmt.Fprintf to stderr) including the
markerFile path and the error; do not change the primary return behavior (still
return nil on success), but ensure failures writing markerFile are logged so
repeated re-creation of defaultBaseConfig can be diagnosed (refer to
Store.Initialize, markerFile, markerContent, and filePermissions).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f5216b28-2d7c-4a49-b377-5c98256ecd66
📒 Files selected for processing (4)
internal/cmd/context.gointernal/persis/filebaseconfig/default_base_config.gointernal/persis/filebaseconfig/store.gointernal/persis/filebaseconfig/store_test.go
- Log warning when filebaseconfig.New() fails instead of silently ignoring - Add GPL v3 license headers via make addlicense - Update SkipExamples comment to document its broader scope (examples + base config)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1737 +/- ##
==========================================
- Coverage 69.67% 69.61% -0.07%
==========================================
Files 402 402
Lines 44607 44668 +61
==========================================
+ Hits 31082 31094 +12
Misses 11019 11019
- Partials 2506 2555 +49
... and 16 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
1 participant
When Dagu starts and no base.yaml exists, a fully-commented default is created as a discoverable reference for all inheritable DAG settings. Uses marker file (.base-config-created) to avoid re-creation if the user intentionally deletes the file. Respects the skip_examples config flag.
Summary by CodeRabbit
New Features
Tests