Merge pull request #244 from yohamta/develop

Load admin config for all commands before action

cmd/dagu.go

@@ -1,18 +1,26 @@
 package main
 
 import (
+	"fmt"
 	"io"
 	"log"
 	"os"
 	"os/signal"
+	"path"
 	"syscall"
 
 	"github.com/urfave/cli/v2"
+	"github.com/yohamta/dagu/internal/admin"
 	"github.com/yohamta/dagu/internal/constants"
+	"github.com/yohamta/dagu/internal/utils"
 )
 
-var version = "0.0.0"
-var stdin io.ReadCloser
+var (
+	version      = "0.0.0"
+	stdin        io.ReadCloser
+	sigs         chan os.Signal
+	globalConfig *admin.Config
+)
 
 func main() {
 	setVersion()
@@ -22,8 +30,6 @@ func main() {
 	}
 }
 
-var sigs chan os.Signal
-
 func listenSignals(abortFunc func(sig os.Signal)) {
 	sigs = make(chan os.Signal, 1)
 	signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)
@@ -60,5 +66,19 @@ func makeApp() *cli.App {
 			newSchedulerCommand(),
 			newVersionCommand(),
 		},
+		Before: func(c *cli.Context) error {
+			l := &admin.Loader{}
+			cfg, err := l.LoadAdminConfig(path.Join(
+				utils.MustGetUserHomeDir(), ".dagu/admin.yaml"))
+			if err == admin.ErrConfigNotFound {
+				cfg = admin.DefaultConfig()
+				err = nil
+			}
+			if err != nil {
+				return fmt.Errorf("loading admin config failed: %w", err)
+			}
+			globalConfig = cfg
+			return nil
+		},
 	}
 }

cmd/scheduler.go

@@ -2,16 +2,13 @@ package main
 
 import (
 	"os"
-	"path"
 
 	"github.com/urfave/cli/v2"
 	"github.com/yohamta/dagu/internal/admin"
 	"github.com/yohamta/dagu/internal/runner"
-	"github.com/yohamta/dagu/internal/utils"
 )
 
 func newSchedulerCommand() *cli.Command {
-	l := &admin.Loader{}
 	return &cli.Command{
 		Name:  "scheduler",
 		Usage: "dagu scheduler",
@@ -24,18 +21,11 @@ func newSchedulerCommand() *cli.Command {
 			},
 		},
 		Action: func(c *cli.Context) error {
-			cfg, err := l.LoadAdminConfig(
-				path.Join(utils.MustGetUserHomeDir(), ".dagu/admin.yaml"))
-			if err == admin.ErrConfigNotFound {
-				cfg = admin.DefaultConfig()
-			} else if err != nil {
-				return err
-			}
 			dagsDir := c.String("dags")
 			if dagsDir != "" {
-				cfg.DAGs = dagsDir
+				globalConfig.DAGs = dagsDir
 			}
-			return startScheduler(cfg)
+			return startScheduler(globalConfig)
 		},
 	}
 }

cmd/server.go

@@ -2,27 +2,18 @@ package main
 
 import (
 	"os"
-	"path"
 
 	"github.com/urfave/cli/v2"
 	"github.com/yohamta/dagu/internal/admin"
 	"github.com/yohamta/dagu/internal/utils"
 )
 
 func newServerCommand() *cli.Command {
-	l := &admin.Loader{}
 	return &cli.Command{
 		Name:  "server",
 		Usage: "dagu server",
 		Action: func(c *cli.Context) error {
-			cfg, err := l.LoadAdminConfig(
-				path.Join(utils.MustGetUserHomeDir(), ".dagu/admin.yaml"))
-			if err == admin.ErrConfigNotFound {
-				cfg = admin.DefaultConfig()
-			} else if err != nil {
-				return err
-			}
-			return startServer(cfg)
+			return startServer(globalConfig)
 		},
 	}
 }

internal/admin/config.go

@@ -3,9 +3,9 @@ package admin
 import (
 	"fmt"
 	"os"
-	"path"
 	"path/filepath"
 	"strconv"
+	"strings"
 
 	"github.com/yohamta/dagu/internal/settings"
 	"github.com/yohamta/dagu/internal/utils"
@@ -25,105 +25,119 @@ type Config struct {
 	LogDir             string
 }
 
-func (c *Config) Init() {
-	if c.Env == nil {
-		c.Env = []string{}
+func (cfg *Config) Init() {
+	if cfg.Env == nil {
+		cfg.Env = []string{}
 	}
 }
 
-func (c *Config) setup() {
-	if c.Command == "" {
-		c.Command = "dagu"
-	}
-	if c.DAGs == "" {
-		c.DAGs = path.Join(
-			settings.MustGet(settings.SETTING__ADMIN_DAGS_DIR),
-		)
-	}
-	if c.LogDir == "" {
-		c.LogDir = path.Join(
-			settings.MustGet(settings.SETTING__ADMIN_LOGS_DIR),
-		)
-	}
-	if c.Host == "" {
-		c.Host = "127.0.0.1"
-	}
-	if c.Port == "" {
-		c.Port = settings.MustGet(settings.SETTING__ADMIN_PORT)
-	}
-	if len(c.Env) == 0 {
+func (cfg *Config) setup() {
+	cfg.Command = utils.StringWithFallback(cfg.Command, "dagu")
+	cfg.DAGs = utils.StringWithFallback(cfg.DAGs,
+		settings.MustGet(settings.SETTING__ADMIN_DAGS_DIR))
+	cfg.LogDir = utils.StringWithFallback(cfg.LogDir,
+		settings.MustGet(settings.SETTING__ADMIN_LOGS_DIR))
+	cfg.Host = utils.StringWithFallback(cfg.Host, "127.0.0.1")
+	cfg.Port = utils.StringWithFallback(cfg.Port,
+		settings.MustGet(settings.SETTING__ADMIN_PORT))
+	if len(cfg.Env) == 0 {
 		env := utils.DefaultEnv()
 		env, err := loadVariables(env)
 		if err != nil {
 			panic(err)
 		}
-		c.Env = buildConfigEnv(env)
+		cfg.Env = buildConfigEnv(env)
 	}
 }
 
-func buildFromDefinition(def *configDefinition) (c *Config, err error) {
-	c = &Config{}
-	c.Init()
+func buildFromDefinition(def *configDefinition) (cfg *Config, err error) {
+	cfg = &Config{}
+	cfg.Init()
 
-	env, err := loadVariables(def.Env)
-	if err != nil {
-		return nil, err
+	for _, fn := range []func(cfg *Config, def *configDefinition) error{
+		buildEnvs,
+		buildHostPort,
+		buildDAGsDir,
+		buildCommand,
+		buildWorkDir,
+		buildBasicAuthOpts,
+		buidEncodingOpts,
+	} {
+		if err := fn(cfg, def); err != nil {
+			return nil, err
+		}
 	}
-	c.Env = buildConfigEnv(env)
 
-	c.Host, err = utils.ParseVariable(def.Host)
+	cfg.LogDir = def.LogDir
+	cfg.IsBasicAuth = def.IsBasicAuth
+
+	return cfg, nil
+}
+
+func buildEnvs(cfg *Config, def *configDefinition) error {
+	env, err := loadVariables(def.Env)
 	if err != nil {
-		return nil, err
+		return err
 	}
+	cfg.Env = buildConfigEnv(env)
+	return nil
+}
+
+func buildHostPort(cfg *Config, def *configDefinition) (err error) {
+	cfg.Host, err = utils.ParseVariable(def.Host)
 	if def.Port == 0 {
-		c.Port = settings.MustGet(settings.SETTING__ADMIN_PORT)
+		cfg.Port = settings.MustGet(settings.SETTING__ADMIN_PORT)
 	} else {
-		c.Port = strconv.Itoa(def.Port)
+		cfg.Port = strconv.Itoa(def.Port)
 	}
+	return err
+}
 
-	jd, err := utils.ParseVariable(def.Dags)
-	if err != nil {
-		return nil, err
-	}
-	if len(jd) > 0 {
-		if !filepath.IsAbs(jd) {
-			return nil, fmt.Errorf("DAGs directory should be absolute path. was %s", jd)
+func buildDAGsDir(cfg *Config, def *configDefinition) (err error) {
+	val, err := utils.ParseVariable(def.Dags)
+	if err == nil && len(val) > 0 {
+		if !filepath.IsAbs(val) {
+			return fmt.Errorf("DAGs directory should be absolute path. was %s", val)
 		}
-		c.DAGs, err = filepath.Abs(jd)
+		cfg.DAGs, err = filepath.Abs(val)
 		if err != nil {
-			return nil, err
+			return fmt.Errorf("failed to resolve DAGs directory: %w", err)
 		}
 	}
-	c.LogDir = def.LogDir
+	return err
+}
 
-	c.Command, err = utils.ParseVariable(def.Command)
-	if err != nil {
-		return nil, err
-	}
-	c.WorkDir, err = utils.ParseVariable(def.WorkDir)
-	if err != nil {
-		return nil, err
-	}
-	if c.WorkDir == "" {
-		c.WorkDir, err = os.Getwd()
+func buildCommand(cfg *Config, def *configDefinition) (err error) {
+	cfg.Command, err = utils.ParseVariable(def.Command)
+	return err
+}
+
+func buildWorkDir(cfg *Config, def *configDefinition) (err error) {
+	cfg.WorkDir, err = utils.ParseVariable(def.WorkDir)
+	if err == nil && strings.TrimSpace(cfg.WorkDir) == "" {
+		cfg.WorkDir, err = os.Getwd()
 		if err != nil {
-			return nil, err
+			return fmt.Errorf("failed to resolve working directory: %w", err)
 		}
 	}
-	c.IsBasicAuth = def.IsBasicAuth
-	c.BasicAuthUsername, err = utils.ParseVariable(def.BasicAuthUsername)
-	if err != nil {
-		return nil, err
-	}
-	c.BasicAuthPassword, err = utils.ParseVariable(def.BasicAuthPassword)
+	return err
+}
+
+func buildBasicAuthOpts(cfg *Config, def *configDefinition) (err error) {
+	cfg.BasicAuthUsername, err = utils.ParseVariable(def.BasicAuthUsername)
 	if err != nil {
-		return nil, err
+		return err
 	}
-	c.LogEncodingCharset, err = utils.ParseVariable(def.LogEncodingCharset)
+	cfg.BasicAuthPassword, err = utils.ParseVariable(def.BasicAuthPassword)
 	if err != nil {
-		return nil, err
+		return err
 	}
-	return c, nil
+	return nil
+}
+
+func buidEncodingOpts(cfg *Config, def *configDefinition) (err error) {
+	cfg.LogEncodingCharset, err = utils.ParseVariable(def.LogEncodingCharset)
+	return err
 }
 
 func buildConfigEnv(vars map[string]string) []string {

internal/admin/config_test.go

@@ -1,6 +1,7 @@
 package admin
 
 import (
+	"fmt"
 	"os"
 	"testing"
 
@@ -77,7 +78,7 @@ func TestLoadConfig(t *testing.T) {
 }
 
 func TestLoadInvalidConfigError(t *testing.T) {
-	for _, c := range []string{
+	for i, c := range []string{
 		`dags: ./relative`,
 		`dags: "` + "`ech /dags_dir`" + `"`,
 		`command: "` + "`ech cmd`" + `"`,
@@ -86,14 +87,16 @@ func TestLoadInvalidConfigError(t *testing.T) {
 		`basicAuthPassword: "` + "`ech foo`" + `"`,
 		`logEncodingCharset: "` + "`ech foo`" + `"`,
 	} {
-		l := &Loader{}
-		d, err := l.unmarshalData([]byte(c))
-		require.NoError(t, err)
+		t.Run(fmt.Sprintf("test-invalid-cfg-%d", i), func(t *testing.T) {
+			l := &Loader{}
+			d, err := l.unmarshalData([]byte(c))
+			require.NoError(t, err)
 
-		def, err := l.decode(d)
-		require.NoError(t, err)
+			def, err := l.decode(d)
+			require.NoError(t, err)
 
-		_, err = buildFromDefinition(def)
-		require.Error(t, err)
+			_, err = buildFromDefinition(def)
+			require.Error(t, err)
+		})
 	}
 }