From edf55ab8853fe0435e5addfde67ae866fd18af6a Mon Sep 17 00:00:00 2001 From: brendan-coughlan Date: Mon, 13 Jan 2025 10:35:54 -0800 Subject: [PATCH] use binary/ filepath, and nicer cli errors --- cmd/createSnapshot.go | 13 +++---- cmd/restoreSnapshot.go | 13 +++---- docs/snapshots_docs.md | 4 +- pkg/snapshot/snapshot.go | 64 +++++++++++++++++++++++--------- pkg/utils/helpers.go | 14 ------- scripts/convertSnapshotSchema.sh | 4 +- 6 files changed, 61 insertions(+), 51 deletions(-) diff --git a/cmd/createSnapshot.go b/cmd/createSnapshot.go index 4372875e..50cad5eb 100644 --- a/cmd/createSnapshot.go +++ b/cmd/createSnapshot.go @@ -6,7 +6,6 @@ import ( "github.com/Layr-Labs/sidecar/internal/config" "github.com/Layr-Labs/sidecar/internal/logger" "github.com/Layr-Labs/sidecar/pkg/snapshot" - "github.com/Layr-Labs/sidecar/pkg/utils" "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/spf13/viper" @@ -25,13 +24,8 @@ var createSnapshotCmd = &cobra.Command{ return fmt.Errorf("failed to initialize logger: %w", err) } - outputFile, err := utils.ExpandHomeDir(cfg.SnapshotConfig.OutputFile) - if err != nil { - return err - } - - svc := snapshot.NewSnapshotService(&snapshot.SnapshotConfig{ - OutputFile: outputFile, + svc, err := snapshot.NewSnapshotService(&snapshot.SnapshotConfig{ + OutputFile: cfg.SnapshotConfig.OutputFile, Host: cfg.DatabaseConfig.Host, Port: cfg.DatabaseConfig.Port, User: cfg.DatabaseConfig.User, @@ -39,6 +33,9 @@ var createSnapshotCmd = &cobra.Command{ DbName: cfg.DatabaseConfig.DbName, SchemaName: cfg.DatabaseConfig.SchemaName, }, l) + if err != nil { + return err + } if err := svc.CreateSnapshot(); err != nil { return fmt.Errorf("failed to create snapshot: %w", err) diff --git a/cmd/restoreSnapshot.go b/cmd/restoreSnapshot.go index 58f5c836..4b6d8a4a 100644 --- a/cmd/restoreSnapshot.go +++ b/cmd/restoreSnapshot.go @@ -6,7 +6,6 @@ import ( "github.com/Layr-Labs/sidecar/internal/config" "github.com/Layr-Labs/sidecar/internal/logger" "github.com/Layr-Labs/sidecar/pkg/snapshot" - "github.com/Layr-Labs/sidecar/pkg/utils" "github.com/spf13/cobra" "github.com/spf13/pflag" "github.com/spf13/viper" @@ -28,13 +27,8 @@ Follow the snapshot docs if you need to convert the snapshot to a different sche return fmt.Errorf("failed to initialize logger: %w", err) } - inputFile, err := utils.ExpandHomeDir(cfg.SnapshotConfig.InputFile) - if err != nil { - return err - } - - svc := snapshot.NewSnapshotService(&snapshot.SnapshotConfig{ - InputFile: inputFile, + svc, err := snapshot.NewSnapshotService(&snapshot.SnapshotConfig{ + InputFile: cfg.SnapshotConfig.InputFile, Host: cfg.DatabaseConfig.Host, Port: cfg.DatabaseConfig.Port, User: cfg.DatabaseConfig.User, @@ -42,6 +36,9 @@ Follow the snapshot docs if you need to convert the snapshot to a different sche DbName: cfg.DatabaseConfig.DbName, SchemaName: cfg.DatabaseConfig.SchemaName, }, l) + if err != nil { + return err + } if err := svc.RestoreSnapshot(); err != nil { return fmt.Errorf("failed to restore snapshot: %w", err) diff --git a/docs/snapshots_docs.md b/docs/snapshots_docs.md index d9d546b8..f7962a98 100644 --- a/docs/snapshots_docs.md +++ b/docs/snapshots_docs.md @@ -114,7 +114,7 @@ Commonly the input schema is psql -c "CREATE DATABASE temp_sidecar_dump_schema_conversion_db;" # Use the Sidecar CLI to restore the snapshot dump into the temporary database: -go run main.go restore-snapshot \ +./bin/sidecar restore-snapshot \ --database.host=localhost \ --database.user=... \ --database.password=... \ @@ -127,7 +127,7 @@ go run main.go restore-snapshot \ psql -d temp_sidecar_dump_schema_conversion_db -c "ALTER SCHEMA RENAME TO ;" # Use the Sidecar CLI to create a new snapshot with the updated schema: -go run main.go create-snapshot \ +./bin/sidecar create-snapshot \ --database.host=localhost \ --database.user=... \ --database.password=... \ diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index 5c5581c6..d5633fe0 100644 --- a/pkg/snapshot/snapshot.go +++ b/pkg/snapshot/snapshot.go @@ -3,6 +3,8 @@ package snapshot import ( "fmt" "os" + "path/filepath" + "strings" pgcommands "github.com/habx/pg-commands" "go.uber.org/zap" @@ -27,17 +29,49 @@ type SnapshotService struct { } // NewSnapshotService initializes a new SnapshotService with the given configuration and logger. -func NewSnapshotService(cfg *SnapshotConfig, l *zap.Logger) *SnapshotService { +func NewSnapshotService(cfg *SnapshotConfig, l *zap.Logger) (*SnapshotService, error) { + var err error + + cfg.InputFile, err = resolveFilePath(cfg.InputFile) + if err != nil { + return nil, fmt.Errorf("failed to resolve input file path: %w", err) + } + cfg.OutputFile, err = resolveFilePath(cfg.OutputFile) + if err != nil { + return nil, fmt.Errorf("failed to resolve output file path: %w", err) + } + + l.Sugar().Infow("Resolved file paths", "inputFile", cfg.InputFile, "outputFile", cfg.OutputFile) + return &SnapshotService{ cfg: cfg, l: l, + }, nil +} + +// resolveFilePath expands the ~ in file paths to the user's home directory and converts relative paths to absolute paths. +func resolveFilePath(path string) (string, error) { + if path == "" { + return "", nil + } + if strings.HasPrefix(path, "~/") { + homeDir, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("failed to get user home directory: %w", err) + } + path = filepath.Join(homeDir, path[2:]) + } + absPath, err := filepath.Abs(path) + if err != nil { + return "", fmt.Errorf("failed to get absolute path: %w", err) } + return absPath, nil } // CreateSnapshot creates a snapshot of the database based on the provided configuration. func (s *SnapshotService) CreateSnapshot() error { - if !s.validateCreateSnapshotConfig() { - return fmt.Errorf("invalid snapshot configuration") + if err := s.validateCreateSnapshotConfig(); err != nil { + return err } dump, err := s.setupSnapshotDump() @@ -57,8 +91,8 @@ func (s *SnapshotService) CreateSnapshot() error { // RestoreSnapshot restores a snapshot of the database based on the provided configuration. func (s *SnapshotService) RestoreSnapshot() error { - if !s.validateRestoreConfig() { - return fmt.Errorf("invalid restore configuration") + if err := s.validateRestoreConfig(); err != nil { + return err } restore, err := s.setupRestore() @@ -79,18 +113,16 @@ func (s *SnapshotService) RestoreSnapshot() error { return nil } -func (s *SnapshotService) validateCreateSnapshotConfig() bool { +func (s *SnapshotService) validateCreateSnapshotConfig() error { if s.cfg.Host == "" { - s.l.Sugar().Error("Database host is required") - return false + return fmt.Errorf("database host is required") } if s.cfg.OutputFile == "" { - s.l.Sugar().Error("Output path i.e. `output-file` must be specified") - return false + return fmt.Errorf("output path i.e. `output-file` must be specified") } - return true + return nil } func (s *SnapshotService) setupSnapshotDump() (*pgcommands.Dump, error) { @@ -115,19 +147,17 @@ func (s *SnapshotService) setupSnapshotDump() (*pgcommands.Dump, error) { return dump, nil } -func (s *SnapshotService) validateRestoreConfig() bool { +func (s *SnapshotService) validateRestoreConfig() error { if s.cfg.InputFile == "" { - s.l.Sugar().Error("Restore snapshot file path i.e. `input-file` must be specified") - return false + return fmt.Errorf("restore snapshot file path i.e. `input-file` must be specified") } info, err := os.Stat(s.cfg.InputFile) if err != nil || info.IsDir() { - s.l.Sugar().Errorw("Snapshot file does not exist", "path", s.cfg.InputFile) - return false + return fmt.Errorf("snapshot file does not exist: %s", s.cfg.InputFile) } - return true + return nil } func (s *SnapshotService) setupRestore() (*pgcommands.Restore, error) { diff --git a/pkg/utils/helpers.go b/pkg/utils/helpers.go index 6613ef8a..c875ff3d 100644 --- a/pkg/utils/helpers.go +++ b/pkg/utils/helpers.go @@ -3,8 +3,6 @@ package utils import ( "encoding/hex" "fmt" - "os" - "path/filepath" "regexp" "strings" ) @@ -26,15 +24,3 @@ func SnakeCase(s string) string { notSnake := regexp.MustCompile(`[_-]`) return notSnake.ReplaceAllString(s, "_") } - -// ExpandHomeDir expands the ~ in file paths to the user's home directory. -func ExpandHomeDir(path string) (string, error) { - if strings.HasPrefix(path, "~/") { - homeDir, err := os.UserHomeDir() - if err != nil { - return "", fmt.Errorf("failed to get user home directory: %w", err) - } - path = filepath.Join(homeDir, path[2:]) - } - return path, nil -} diff --git a/scripts/convertSnapshotSchema.sh b/scripts/convertSnapshotSchema.sh index a5a93e7c..327ee19d 100755 --- a/scripts/convertSnapshotSchema.sh +++ b/scripts/convertSnapshotSchema.sh @@ -27,7 +27,7 @@ PGPASSWORD=$DB_PASSWORD psql -U $DB_USER -c "CREATE DATABASE $TEMP_DB_NAME;" || # Restore the snapshot dump into the temporary database echo "Restoring snapshot into temporary database" -go run main.go restore-snapshot \ +./bin/sidecar restore-snapshot \ --database.host=localhost \ --database.port=5432 \ --database.user=$DB_USER \ @@ -42,7 +42,7 @@ PGPASSWORD=$DB_PASSWORD psql -U $DB_USER -d $TEMP_DB_NAME -c "ALTER SCHEMA $INPU # Create a new snapshot with the updated schema echo "Creating new snapshot with updated schema" -go run main.go create-snapshot \ +./bin/sidecar create-snapshot \ --database.host=localhost \ --database.port=5432 \ --database.user=$DB_USER \