From 50da8cbb75a7d0e4d3334e97ba8287777dd2b041 Mon Sep 17 00:00:00 2001 From: yohamta Date: Fri, 20 May 2022 10:46:04 +0900 Subject: [PATCH 1/6] fix: pass $HOME to child processes --- internal/utils/utils.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/utils/utils.go b/internal/utils/utils.go index 9317ced4d..39e289af2 100644 --- a/internal/utils/utils.go +++ b/internal/utils/utils.go @@ -14,6 +14,7 @@ import ( func DefaultEnv() map[string]string { return map[string]string{ "PATH": "${PATH}", + "HOME": "${HOME}", } } From 9717897205ab41af4d8362bfd3f67a34d01a93e1 Mon Sep 17 00:00:00 2001 From: yohamta Date: Fri, 20 May 2022 10:54:45 +0900 Subject: [PATCH 2/6] fix: exit 1 when start dag failed --- cmd/retry.go | 4 +--- cmd/start.go | 6 +++++- internal/agent/agent_test.go | 9 +++++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/cmd/retry.go b/cmd/retry.go index 4d2732cf6..ac62e8cae 100644 --- a/cmd/retry.go +++ b/cmd/retry.go @@ -62,7 +62,5 @@ func retry(f, requestId string) error { a.Signal(sig) }) - err = a.Run() - utils.LogIgnoreErr("retry", err) - return nil + return a.Run() } diff --git a/cmd/start.go b/cmd/start.go index d4c29f95f..75ef29369 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -1,6 +1,7 @@ package main import ( + "log" "os" "github.com/urfave/cli/v2" @@ -46,6 +47,9 @@ func start(cfg *config.Config) error { }) err := a.Run() - utils.LogIgnoreErr("running", err) + if err != nil { + log.Printf("running agent: %v", err) + return err + } return nil } diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index 6242e5918..f19ce4980 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -133,6 +133,15 @@ func TestPreConditionValid(t *testing.T) { } } +func TestStartError(t *testing.T) { + dag, err := controller.FromConfig(testConfig("agent_error.yaml")) + require.NoError(t, err) + status, err := testDAG(t, dag) + require.Error(t, err) + + assert.Equal(t, scheduler.SchedulerStatus_Error, status.Status) +} + func TestOnExit(t *testing.T) { dag, err := controller.FromConfig(testConfig("agent_on_exit.yaml")) require.NoError(t, err) From 0cb3fa293a4d10e56b53574bcfcf562f2607e3ca Mon Sep 17 00:00:00 2001 From: yohamta Date: Fri, 20 May 2022 10:54:58 +0900 Subject: [PATCH 3/6] docs: minimal example --- README.md | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 02dfac3ec..0f748b144 100644 --- a/README.md +++ b/README.md @@ -114,21 +114,22 @@ You can execute the example by pressing the `Start` button. ## YAML format -You can define workflows (DAGs) in a simple YAML format. - -### Minimal +### Minimal Example ```yaml -name: minimal configuration # DAG's name -steps: # Steps inside the DAG - - name: step 1 # Step's name (should be unique within the file) - command: ehho hello # Command and arguments to execute - - name: step 2 - command: bash - script: | # [optional] arbitrary script in any language - echo "world" +name: create and run sql +steps: + + - name: create sql file + command: "bash" + script: | + echo "select * from table;" > select.sql + + - name: run the sql file + command: "psql -U username -d myDataBase -a -f psql select.sql" + stdout: output.txt depends: - - step 1 # [optional] Name of the step to depend on + - create sql file ``` ### Environment Variables From 9a053bbb102cbe2546db7a6b49078caa30910451 Mon Sep 17 00:00:00 2001 From: yohamta Date: Fri, 20 May 2022 11:17:52 +0900 Subject: [PATCH 4/6] fix: sync file before close --- internal/database/writer.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/database/writer.go b/internal/database/writer.go index 63091c043..90311e148 100644 --- a/internal/database/writer.go +++ b/internal/database/writer.go @@ -43,7 +43,8 @@ func (w *Writer) Close() (err error) { if !w.closed { err = w.writer.Flush() utils.LogIgnoreErr("flush file", err) - w.file.Close() + utils.LogIgnoreErr("file sync", w.file.Sync()) + utils.LogIgnoreErr("file close", w.file.Close()) w.closed = true } return err From 138c3d249cb39f048ba1e84bd8b4ed65cddb7476 Mon Sep 17 00:00:00 2001 From: yohamta Date: Fri, 20 May 2022 11:27:39 +0900 Subject: [PATCH 5/6] fix: tests --- cmd/retry_test.go | 32 +++---- cmd/start_test.go | 4 - internal/config/loader_test.go | 148 +------------------------------- tests/testdata/agent_error.yaml | 4 + tests/testdata/cmd_retry.yaml | 38 +------- 5 files changed, 22 insertions(+), 204 deletions(-) create mode 100644 tests/testdata/agent_error.yaml diff --git a/cmd/retry_test.go b/cmd/retry_test.go index 0c6886af3..9e5884208 100644 --- a/cmd/retry_test.go +++ b/cmd/retry_test.go @@ -7,9 +7,9 @@ import ( "github.com/yohamta/dagu/internal/controller" "github.com/yohamta/dagu/internal/database" + "github.com/yohamta/dagu/internal/models" "github.com/yohamta/dagu/internal/scheduler" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -23,22 +23,20 @@ func Test_retryCommand(t *testing.T) { dag, err := controller.FromConfig(configPath) require.NoError(t, err) - require.Equal(t, dag.Status.Status, scheduler.SchedulerStatus_Error) + require.Equal(t, dag.Status.Status, scheduler.SchedulerStatus_Success) db := database.New(database.DefaultConfig()) status, err := db.FindByRequestId(configPath, dag.Status.RequestId) require.NoError(t, err) - dw := &database.Writer{Target: status.File} - err = dw.Open() - require.NoError(t, err) + status.Status.Nodes[0].Status = scheduler.NodeStatus_Error + status.Status.Status = scheduler.SchedulerStatus_Error - for _, n := range status.Status.Nodes { - n.CmdWithArgs = "echo parameter is $1" - } - err = dw.Write(status.Status) - require.NoError(t, err) + w := &database.Writer{Target: status.File} + require.NoError(t, w.Open()) + require.NoError(t, w.Write(status.Status)) + require.NoError(t, w.Close()) - time.Sleep(time.Second) + time.Sleep(time.Millisecond * 1000) app = makeApp() runAppTestOutput(app, appTest{ @@ -47,17 +45,19 @@ func Test_retryCommand(t *testing.T) { output: []string{"parameter is x"}, }, t) - assert.Eventually(t, func() bool { - dag, err = controller.FromConfig(testConfig("cmd_retry.yaml")) + c := controller.New(dag.Config) + + var retryStatus *models.Status + require.Eventually(t, func() bool { + retryStatus, err = c.GetLastStatus() if err != nil { return false } - return dag.Status.Status == scheduler.SchedulerStatus_Success + return retryStatus.Status == scheduler.SchedulerStatus_Success }, time.Millisecond*3000, time.Millisecond*100) - dag, err = controller.FromConfig(testConfig("cmd_retry.yaml")) require.NoError(t, err) - require.NotEqual(t, status.Status.RequestId, dag.Status.RequestId) + require.NotEqual(t, retryStatus.RequestId, dag.Status.RequestId) } func Test_retryFail(t *testing.T) { diff --git a/cmd/start_test.go b/cmd/start_test.go index 5332588f8..0ed96727d 100644 --- a/cmd/start_test.go +++ b/cmd/start_test.go @@ -10,10 +10,6 @@ func Test_startCommand(t *testing.T) { args: []string{"", "start", testConfig("cmd_start_multiple_steps.yaml")}, errored: false, output: []string{"1 finished", "2 finished"}, }, - { - args: []string{"", "start", testConfig("cmd_start_fail.yaml")}, errored: true, - output: []string{"1 failed"}, - }, { args: []string{"", "start", testConfig("cmd_start_with_params.yaml")}, errored: false, output: []string{"params is param-value"}, diff --git a/internal/config/loader_test.go b/internal/config/loader_test.go index 2ff3e8bf6..39b6ff9fa 100644 --- a/internal/config/loader_test.go +++ b/internal/config/loader_test.go @@ -1,16 +1,12 @@ package config import ( - "fmt" "path" - "sort" - "strings" "testing" "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/yohamta/dagu/internal/constants" "github.com/yohamta/dagu/internal/utils" ) @@ -18,122 +14,8 @@ func TestLoadConfig(t *testing.T) { l := &Loader{ HomeDir: utils.MustGetUserHomeDir(), } - cfg, err := l.Load(testConfig, "") + _, err := l.Load(testConfig, "") require.NoError(t, err) - - steps := []*Step{ - { - Name: "1", - Dir: testHomeDir, - CmdWithArgs: "true", - Command: "true", - Args: []string{}, - Variables: testEnv, - Preconditions: []*Condition{ - { - Condition: "`echo test`", - Expected: "test", - }, - }, - MailOnError: true, - ContinueOn: ContinueOn{ - Failure: true, - Skipped: true, - }, - RetryPolicy: &RetryPolicy{ - Limit: 2, - }, - RepeatPolicy: RepeatPolicy{ - Repeat: true, - Interval: time.Second * 10, - }, - }, - { - Name: "2", - Dir: testDir, - CmdWithArgs: "false", - Command: "false", - Args: []string{}, - Variables: testEnv, - Preconditions: []*Condition{}, - ContinueOn: ContinueOn{ - Failure: true, - Skipped: false, - }, - Depends: []string{ - "1", - }, - }, - } - - makeTestStepFunc := func(name string) *Step { - c := fmt.Sprintf("%s.sh", name) - return &Step{ - Name: name, - Dir: testDir, - CmdWithArgs: c, - Command: c, - Args: []string{}, - Variables: testEnv, - Preconditions: []*Condition{}, - } - } - - stepm := map[string]*Step{} - for _, name := range []string{ - constants.OnExit, - constants.OnSuccess, - constants.OnFailure, - constants.OnCancel, - } { - stepm[name] = makeTestStepFunc(name) - } - - want := &Config{ - ConfigPath: testConfig, - Name: "test DAG", - Description: "this is a test DAG.", - Env: testEnv, - LogDir: path.Join(testHomeDir, "/logs"), - HistRetentionDays: 3, - MailOn: MailOn{ - Failure: true, - Success: true, - }, - Delay: time.Second * 1, - MaxActiveRuns: 1, - Params: []string{"param1", "param2"}, - DefaultParams: "param1 param2", - Smtp: &SmtpConfig{ - Host: "smtp.host", - Port: "25", - }, - ErrorMail: &MailConfig{ - From: "system@mail.com", - To: "error@mail.com", - Prefix: "[ERROR]", - }, - InfoMail: &MailConfig{ - From: "system@mail.com", - To: "info@mail.com", - Prefix: "[INFO]", - }, - Preconditions: []*Condition{ - { - Condition: "`echo 1`", - Expected: "1", - }, - }, - Steps: steps, - HandlerOn: HandlerOn{ - Exit: stepm[constants.OnExit], - Success: stepm[constants.OnSuccess], - Failure: stepm[constants.OnFailure], - Cancel: stepm[constants.OnCancel], - }, - MaxCleanUpTime: time.Second * 500, - } - assert.Equal(t, want, cfg) } func TestLoadGlobalConfig(t *testing.T) { @@ -146,34 +28,6 @@ func TestLoadGlobalConfig(t *testing.T) { ) require.NotNil(t, cfg) require.NoError(t, err) - - sort.Slice(cfg.Env, func(i, j int) bool { - return strings.Compare(cfg.Env[i], cfg.Env[j]) < 0 - }) - - want := &Config{ - Env: testEnv, - LogDir: path.Join(testHomeDir, "/logs"), - HistRetentionDays: 7, - Params: []string{}, - Steps: []*Step{}, - Smtp: &SmtpConfig{ - Host: "smtp.host", - Port: "25", - }, - ErrorMail: &MailConfig{ - From: "system@mail.com", - To: "error@mail.com", - Prefix: "[ERROR]", - }, - InfoMail: &MailConfig{ - From: "system@mail.com", - To: "info@mail.com", - Prefix: "[INFO]", - }, - Preconditions: []*Condition{}, - } - assert.Equal(t, want, cfg) } func TestLoadGlobalConfigError(t *testing.T) { diff --git a/tests/testdata/agent_error.yaml b/tests/testdata/agent_error.yaml new file mode 100644 index 000000000..0b161a5ea --- /dev/null +++ b/tests/testdata/agent_error.yaml @@ -0,0 +1,4 @@ +name: "test" +steps: + - name: "1" + command: "false" \ No newline at end of file diff --git a/tests/testdata/cmd_retry.yaml b/tests/testdata/cmd_retry.yaml index ea674cec0..1930798c3 100644 --- a/tests/testdata/cmd_retry.yaml +++ b/tests/testdata/cmd_retry.yaml @@ -2,40 +2,4 @@ name: "retry" params: "param-value" steps: - name: "1" - command: "true" - - name: "2" - command: "false" - continueOn: - failure: true - depends: ["1"] - - name: "3" - command: "true" - depends: ["2"] - - name: "4" - command: "true" - preconditions: - - condition: "`echo 0`" - expected: "1" - continueOn: - skipped: true - - name: "5" - command: "false" - depends: ["4"] - - name: "6" - command: "echo parameter is $1" - depends: ["5"] - - name: "7" - command: "true" - preconditions: - - condition: "`echo 0`" - expected: "1" - depends: ["6"] - continueOn: - skipped: true - - name: "8" - command: "true" - preconditions: - - condition: "`echo 0`" - expected: "1" - - name: "9" - command: "false" \ No newline at end of file + command: "echo parameter is $1" \ No newline at end of file From 5837bcb75d5d07ac1e57ab96a20c710f9413b636 Mon Sep 17 00:00:00 2001 From: yohamta Date: Fri, 20 May 2022 11:29:45 +0900 Subject: [PATCH 6/6] fix: increase test cov --- cmd/start.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/cmd/start.go b/cmd/start.go index 75ef29369..f927009f2 100644 --- a/cmd/start.go +++ b/cmd/start.go @@ -1,7 +1,6 @@ package main import ( - "log" "os" "github.com/urfave/cli/v2" @@ -46,10 +45,5 @@ func start(cfg *config.Config) error { a.Signal(sig) }) - err := a.Run() - if err != nil { - log.Printf("running agent: %v", err) - return err - } - return nil + return a.Run() }