From a6cda4cb236b8009bfb9bf2128b941436020c87e Mon Sep 17 00:00:00 2001 From: Vladimir Garvardt Date: Thu, 2 Apr 2020 16:01:17 +0200 Subject: [PATCH 1/4] Extracted spec loading to config and added some tests --- .gitignore | 4 +- cmd/init.go | 13 +++-- cmd/root.go | 53 ++++---------------- cmd/steal.go | 86 +++++++++++++++++---------------- fixtures/.klepto.toml | 34 +++++++++++++ pkg/anonymiser/anonymiser.go | 6 +-- pkg/config/config.go | 56 ++++++++++++++++++--- pkg/config/config_test.go | 32 ++++++++++++ pkg/dumper/dumper.go | 10 ++-- pkg/dumper/engine/engine.go | 6 +-- pkg/dumper/query/dumper.go | 21 ++++---- pkg/reader/postgres/postgres.go | 3 +- 12 files changed, 204 insertions(+), 120 deletions(-) create mode 100644 fixtures/.klepto.toml create mode 100644 pkg/config/config_test.go diff --git a/.gitignore b/.gitignore index b61ff7e..470392a 100644 --- a/.gitignore +++ b/.gitignore @@ -10,12 +10,10 @@ klepto # Output of the go coverage tool, specifically when used with LiteIDE *.out /dist -debug -.klepto.toml +/*.klepto.toml coverage.txt # Vendor stuff -.glide/ vendor/ *.coverprofile diff --git a/cmd/init.go b/cmd/init.go index eea773a..6a67e04 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -27,20 +27,23 @@ func NewInitCmd() *cobra.Command { // RunInit runs the init command func RunInit() error { - log.Infof("Initializing %s", configFileName) + log.Infof("Initializing %s", config.DefaultConfigFileName) - _, err := os.Stat(configFileName) + _, err := os.Stat(config.DefaultConfigFileName) if !os.IsNotExist(err) { log.Fatal("Config file already exists, refusing to overwrite") } - f, err := os.Create(configFileName) + f, err := os.Create(config.DefaultConfigFileName) if err != nil { return wErrors.Wrap(err, "could not create file") } e := toml.NewEncoder(bufio.NewWriter(f)) err = e.Encode(config.Spec{ + Matchers: map[string]string{ + "ActiveUsers": "users.active = TRUE", + }, Tables: []*config.Table{ { Name: "users", @@ -54,7 +57,7 @@ func RunInit() error { { Name: "orders", Filter: config.Filter{ - Match: "users.active = TRUE", + Match: "ActiveUsers", Limit: 10, }, Relationships: []*config.Relationship{ @@ -75,7 +78,7 @@ func RunInit() error { return wErrors.Wrap(err, "could not encode config") } - log.Infof("Created %s!", configFileName) + log.Infof("Created %s!", config.DefaultConfigFileName) return nil } diff --git a/cmd/root.go b/cmd/root.go index eb21bc5..5f0070e 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -3,19 +3,14 @@ package cmd import ( "os" - "github.com/hellofresh/klepto/pkg/config" - "github.com/hellofresh/klepto/pkg/formatter" - wErrors "github.com/pkg/errors" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" - "github.com/spf13/viper" + + "github.com/hellofresh/klepto/pkg/formatter" ) var ( - globalConfig *config.Spec - configFile string - configFileName = ".klepto.toml" - verbose bool + verbose bool // RootCmd steals and anonymises databases RootCmd = &cobra.Command{ @@ -32,48 +27,18 @@ var ( ) func init() { - RootCmd.PersistentFlags().StringVarP(&configFile, "config", "c", "", "Path to config file (default is ./.klepto)") RootCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "Make the operation more talkative") + RootCmd.PersistentPreRun = func(cmd *cobra.Command, args []string) { + if verbose { + log.SetLevel(log.DebugLevel) + } + } - RootCmd.AddCommand(NewStealCmd()) RootCmd.AddCommand(NewVersionCmd()) RootCmd.AddCommand(NewUpdateCmd()) RootCmd.AddCommand(NewInitCmd()) + RootCmd.AddCommand(NewStealCmd()) log.SetOutput(os.Stderr) log.SetFormatter(&formatter.CliFormatter{}) } - -func initConfig(c *cobra.Command, args []string) error { - if verbose { - log.SetLevel(log.DebugLevel) - } - - log.Debugf("Reading config from %s...", configFileName) - - if configFile != "" { - // Use config file from the flag. - viper.SetConfigFile(configFile) - } else { - cwd, err := os.Getwd() - if err != nil { - return wErrors.Wrap(err, "can't find current working directory") - } - - viper.SetConfigName(".klepto") - viper.AddConfigPath(cwd) - viper.AddConfigPath(".") - } - - err := viper.ReadInConfig() - if err != nil { - return wErrors.Wrap(err, "could not read configurations") - } - - err = viper.Unmarshal(&globalConfig) - if err != nil { - return wErrors.Wrap(err, "could not unmarshal config file") - } - - return nil -} diff --git a/cmd/steal.go b/cmd/steal.go index 4e8c378..eccea89 100644 --- a/cmd/steal.go +++ b/cmd/steal.go @@ -4,13 +4,15 @@ import ( "runtime" "time" - "github.com/hellofresh/klepto/pkg/anonymiser" - "github.com/hellofresh/klepto/pkg/dumper" - "github.com/hellofresh/klepto/pkg/reader" wErrors "github.com/pkg/errors" log "github.com/sirupsen/logrus" "github.com/spf13/cobra" + "github.com/hellofresh/klepto/pkg/anonymiser" + "github.com/hellofresh/klepto/pkg/config" + "github.com/hellofresh/klepto/pkg/dumper" + "github.com/hellofresh/klepto/pkg/reader" + // imports dumpers and readers _ "github.com/hellofresh/klepto/pkg/dumper/mysql" _ "github.com/hellofresh/klepto/pkg/dumper/postgres" @@ -22,6 +24,9 @@ import ( type ( // StealOptions represents the command options StealOptions struct { + configPath string + cfgSpec *config.Spec + from string to string concurrency int @@ -29,8 +34,8 @@ type ( writeOpts connOpts } connOpts struct { - timeout string - maxConnLifetime string + timeout time.Duration + maxConnLifetime time.Duration maxConns int maxIdleConns int } @@ -40,81 +45,80 @@ type ( func NewStealCmd() *cobra.Command { opts := new(StealOptions) cmd := &cobra.Command{ - Use: "steal", - Short: "Steals and anonymises databases", - PreRunE: initConfig, + Use: "steal", + Short: "Steals and anonymises databases", + PreRunE: func(cmd *cobra.Command, args []string) error { + var err error + opts.cfgSpec, err = config.LoadSpecFromFile(opts.configPath) + if err != nil { + return err + } + + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { return RunSteal(opts) }, } - cmd.PersistentFlags().StringVarP(&opts.from, "from", "f", "root:root@tcp(localhost:3306)/klepto", "Database dsn to steal from") + cmd.PersistentFlags().StringVarP(&opts.configPath, "config", "c", config.DefaultConfigFileName, "Path to config file") + cmd.PersistentFlags().StringVarP(&opts.from, "from", "f", "mysql://root:root@tcp(localhost:3306)/klepto", "Database dsn to steal from") cmd.PersistentFlags().StringVarP(&opts.to, "to", "t", "os://stdout/", "Database to output to (default writes to stdOut)") cmd.PersistentFlags().IntVar(&opts.concurrency, "concurrency", runtime.NumCPU(), "Sets the amount of dumps to be performed concurrently") - cmd.PersistentFlags().StringVar(&opts.readOpts.timeout, "read-timeout", "5m", "Sets the timeout for read operations") - cmd.PersistentFlags().StringVar(&opts.writeOpts.timeout, "write-timeout", "30s", "Sets the timeout for write operations") - cmd.PersistentFlags().StringVar(&opts.readOpts.maxConnLifetime, "read-conn-lifetime", "0", "Sets the maximum amount of time a connection may be reused on the read database") + cmd.PersistentFlags().DurationVar(&opts.readOpts.timeout, "read-timeout", 5*time.Minute, "Sets the timeout for read operations") + cmd.PersistentFlags().DurationVar(&opts.readOpts.maxConnLifetime, "read-conn-lifetime", 0, "Sets the maximum amount of time a connection may be reused on the read database") cmd.PersistentFlags().IntVar(&opts.readOpts.maxConns, "read-max-conns", 5, "Sets the maximum number of open connections to the read database") cmd.PersistentFlags().IntVar(&opts.readOpts.maxIdleConns, "read-max-idle-conns", 0, "Sets the maximum number of connections in the idle connection pool for the read database") - cmd.PersistentFlags().StringVar(&opts.writeOpts.maxConnLifetime, "write-conn-lifetime", "0", "Sets the maximum amount of time a connection may be reused on the write database") + cmd.PersistentFlags().DurationVar(&opts.writeOpts.timeout, "write-timeout", 30*time.Second, "Sets the timeout for write operations") + cmd.PersistentFlags().DurationVar(&opts.writeOpts.maxConnLifetime, "write-conn-lifetime", 0, "Sets the maximum amount of time a connection may be reused on the write database") cmd.PersistentFlags().IntVar(&opts.writeOpts.maxConns, "write-max-conns", 5, "Sets the maximum number of open connections to the write database") cmd.PersistentFlags().IntVar(&opts.writeOpts.maxIdleConns, "write-max-idle-conns", 0, "Sets the maximum number of connections in the idle connection pool for the write database") + return cmd } // RunSteal is the handler for the rootCmd. func RunSteal(opts *StealOptions) (err error) { - readTimeout, err := time.ParseDuration(opts.readOpts.timeout) - if err != nil { - return wErrors.Wrap(err, "Failed to parse read timeout duration") - } - - writeTimeout, err := time.ParseDuration(opts.readOpts.timeout) - if err != nil { - return wErrors.Wrap(err, "Failed to parse write timeout duration") - } - - readMaxConnLifetime, err := time.ParseDuration(opts.readOpts.maxConnLifetime) - if err != nil { - return wErrors.Wrap(err, "Failed to parse write timeout duration") - } - - writeMaxConnLifetime, err := time.ParseDuration(opts.writeOpts.maxConnLifetime) - if err != nil { - return wErrors.Wrap(err, "Failed to parse the timeout duration") - } - source, err := reader.Connect(reader.ConnOpts{ DSN: opts.from, - Timeout: readTimeout, - MaxConnLifetime: readMaxConnLifetime, + Timeout: opts.readOpts.timeout, + MaxConnLifetime: opts.readOpts.maxConnLifetime, MaxConns: opts.readOpts.maxConns, MaxIdleConns: opts.readOpts.maxIdleConns, }) if err != nil { return wErrors.Wrap(err, "Could not connecting to reader") } - defer source.Close() + defer func() { + if err := source.Close(); err != nil { + log.WithError(err).Error("Something is not ok with closing source connection") + } + }() - source = anonymiser.NewAnonymiser(source, globalConfig.Tables) + source = anonymiser.NewAnonymiser(source, opts.cfgSpec.Tables) target, err := dumper.NewDumper(dumper.ConnOpts{ DSN: opts.to, - Timeout: writeTimeout, - MaxConnLifetime: writeMaxConnLifetime, + Timeout: opts.writeOpts.timeout, + MaxConnLifetime: opts.writeOpts.maxConnLifetime, MaxConns: opts.writeOpts.maxConns, MaxIdleConns: opts.writeOpts.maxIdleConns, }, source) if err != nil { return wErrors.Wrap(err, "Error creating dumper") } - defer target.Close() + defer func() { + if err := target.Close(); err != nil { + log.WithError(err).Error("Something is not ok with closing target connection") + } + }() log.Info("Stealing...") done := make(chan struct{}) defer close(done) + start := time.Now() - if err := target.Dump(done, globalConfig, opts.concurrency); err != nil { + if err := target.Dump(done, opts.cfgSpec, opts.concurrency); err != nil { return wErrors.Wrap(err, "Error while dumping") } diff --git a/fixtures/.klepto.toml b/fixtures/.klepto.toml new file mode 100644 index 0000000..2f827d6 --- /dev/null +++ b/fixtures/.klepto.toml @@ -0,0 +1,34 @@ +[Matchers] + ActiveUsers = "users.active = TRUE" + +[[Tables]] + Name = "users" + IgnoreData = false + [Tables.Filter] + Match = "users.active = TRUE" + Limit = 100 + [Tables.Filter.Sorts] + "user.id" = "asc" + [Tables.Anonymise] + email = "EmailAddress" + firstName = "FirstName" + +[[Tables]] + Name = "orders" + IgnoreData = false + [Tables.Filter] + Match = "ActiveUsers" + Limit = 10 + + [[Tables.Relationships]] + Table = "" + ForeignKey = "user_id" + ReferencedTable = "users" + ReferencedKey = "id" + +[[Tables]] + Name = "logs" + IgnoreData = true + [Tables.Filter] + Match = "" + Limit = 0 diff --git a/pkg/anonymiser/anonymiser.go b/pkg/anonymiser/anonymiser.go index 52dde8a..0a90b21 100644 --- a/pkg/anonymiser/anonymiser.go +++ b/pkg/anonymiser/anonymiser.go @@ -38,9 +38,9 @@ func NewAnonymiser(source reader.Reader, tables config.Tables) reader.Reader { func (a *anonymiser) ReadTable(tableName string, rowChan chan<- database.Row, opts reader.ReadTableOpt, matchers config.Matchers) error { logger := log.WithField("table", tableName) logger.Debug("Loading anonymiser config") - table, err := a.tables.FindByName(tableName) - if err != nil { - logger.WithError(err).Debug("the table is not configured to be anonymised") + table := a.tables.FindByName(tableName) + if table == nil { + logger.Debug("the table is not configured to be anonymised") return a.Reader.ReadTable(tableName, rowChan, opts, matchers) } diff --git a/pkg/config/config.go b/pkg/config/config.go index b9c66a3..3d180f8 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1,6 +1,17 @@ package config -import "errors" +import ( + "os" + + wErrors "github.com/pkg/errors" + log "github.com/sirupsen/logrus" + "github.com/spf13/viper" +) + +// Config-related defaults +const ( + DefaultConfigFileName = ".klepto.toml" +) type ( // Spec represents the global app configuration. @@ -23,8 +34,8 @@ type ( // IgnoreData if set to true, it will dump the table structure without importing data. IgnoreData bool // Filter represents the way you want to filter the results. - Filter - // Anonymise anonymise columns. + Filter Filter + // Anonymise anonymises columns. Anonymise map[string]string // Relationship is an collection of relationship definitions. Relationships []*Relationship @@ -54,12 +65,45 @@ type ( ) // FindByName find a table by its name. -func (t Tables) FindByName(name string) (*Table, error) { +func (t Tables) FindByName(name string) *Table { for _, table := range t { if table.Name == name { - return table, nil + return table } } - return nil, errors.New("table not found") + return nil +} + +// LoadSpecFromFile loads klepto spec from file +func LoadSpecFromFile(configPath string) (*Spec, error) { + if configPath != "" { + // Use config file from the flag. + log.Debugf("Reading config from %s ...", configPath) + viper.SetConfigFile(configPath) + } else { + log.Debugf("Reading config from %s ...", DefaultConfigFileName) + + cwd, err := os.Getwd() + if err != nil { + return nil, wErrors.Wrap(err, "can't find current working directory") + } + + viper.SetConfigName(".klepto") + viper.AddConfigPath(cwd) + viper.AddConfigPath(".") + } + + err := viper.ReadInConfig() + if err != nil { + return nil, wErrors.Wrap(err, "could not read configurations") + } + + cfgSpec := new(Spec) + err = viper.Unmarshal(cfgSpec) + if err != nil { + return nil, wErrors.Wrap(err, "could not unmarshal config file") + } + + return cfgSpec, nil } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go new file mode 100644 index 0000000..282761d --- /dev/null +++ b/pkg/config/config_test.go @@ -0,0 +1,32 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLoadSpecFromFile(t *testing.T) { + cwd, err := os.Getwd() + require.NoError(t, err) + + // klepto/pkg/config/../../fixtures/.klepto.toml + configPath := filepath.Join(cwd, "..", "..", "fixtures", ".klepto.toml") + + spec, err := LoadSpecFromFile(configPath) + require.NoError(t, err) + + cfgTables := spec.Tables + require.Len(t, cfgTables, 3) + + users := cfgTables.FindByName("users") + require.NotNil(t, users) + assert.Equal(t, "users.active = TRUE", users.Filter.Match) + + orders := cfgTables.FindByName("orders") + require.NotNil(t, orders) + assert.Equal(t, "ActiveUsers", orders.Filter.Match) +} diff --git a/pkg/dumper/dumper.go b/pkg/dumper/dumper.go index c9168ae..1756d71 100644 --- a/pkg/dumper/dumper.go +++ b/pkg/dumper/dumper.go @@ -1,12 +1,14 @@ package dumper import ( + "fmt" "time" + wErrors "github.com/pkg/errors" + log "github.com/sirupsen/logrus" + "github.com/hellofresh/klepto/pkg/config" "github.com/hellofresh/klepto/pkg/reader" - "github.com/pkg/errors" - log "github.com/sirupsen/logrus" ) type ( @@ -55,10 +57,10 @@ func NewDumper(opts ConnOpts, rdr reader.Reader) (dumper Dumper, err error) { }) if dumper == nil && err == nil { - err = errors.New("no supported driver found") + err = fmt.Errorf("no supported driver found for dumper DSN %q", opts.DSN) } - err = errors.Wrapf(err, "could not create dumper for dsn: '%v'", opts.DSN) + err = wErrors.Wrapf(err, "could not create dumper for DSN: %q", opts.DSN) return } diff --git a/pkg/dumper/engine/engine.go b/pkg/dumper/engine/engine.go index c0bd27c..ae45c65 100644 --- a/pkg/dumper/engine/engine.go +++ b/pkg/dumper/engine/engine.go @@ -86,9 +86,9 @@ func (e *Engine) readAndDumpTables(done chan<- struct{}, spec *config.Spec, conc var wg sync.WaitGroup for _, tbl := range tables { logger := log.WithField("table", tbl) - tableConfig, err := spec.Tables.FindByName(tbl) - if err != nil { - logger.WithError(err).Debug("no configuration found for table") + tableConfig := spec.Tables.FindByName(tbl) + if tableConfig == nil { + logger.Debug("no configuration found for table") } var opts reader.ReadTableOpt diff --git a/pkg/dumper/query/dumper.go b/pkg/dumper/query/dumper.go index ea89780..26a2f70 100644 --- a/pkg/dumper/query/dumper.go +++ b/pkg/dumper/query/dumper.go @@ -7,12 +7,13 @@ import ( "time" sq "github.com/Masterminds/squirrel" + wErrors "github.com/pkg/errors" + log "github.com/sirupsen/logrus" + "github.com/hellofresh/klepto/pkg/config" "github.com/hellofresh/klepto/pkg/database" "github.com/hellofresh/klepto/pkg/dumper" "github.com/hellofresh/klepto/pkg/reader" - "github.com/pkg/errors" - log "github.com/sirupsen/logrus" ) type ( @@ -34,21 +35,21 @@ func NewDumper(output io.Writer, rdr reader.Reader) dumper.Dumper { func (d *textDumper) Dump(done chan<- struct{}, spec *config.Spec, concurrency int) error { tables, err := d.reader.GetTables() if err != nil { - return errors.Wrap(err, "failed to get tables") + return wErrors.Wrap(err, "failed to get tables") } structure, err := d.reader.GetStructure() if err != nil { - return errors.Wrap(err, "could not get database structure") + return wErrors.Wrap(err, "could not get database structure") } io.WriteString(d.output, structure) for _, tbl := range tables { var opts reader.ReadTableOpt - table, err := spec.Tables.FindByName(tbl) - if err != nil { - log.WithError(err).WithField("table", tbl).Debug("no configuration found for table") + table := spec.Tables.FindByName(tbl) + if table == nil { + log.WithField("table", tbl).Debug("no configuration found for table") } if table != nil { @@ -93,12 +94,12 @@ func (d *textDumper) Close() error { closer, ok := d.output.(io.WriteCloser) if ok { if err := closer.Close(); err != nil { - return errors.Wrap(err, "failed to close output stream") + return wErrors.Wrap(err, "failed to close output stream") } return nil } - return errors.New("unable to close output: wrong closer type") + return wErrors.New("unable to close output: wrong closer type") } func (d *textDumper) toSQLColumnMap(row database.Row) (map[string]interface{}, error) { @@ -152,7 +153,7 @@ func (d *textDumper) toSQLStringValue(src interface{}) (string, error) { } return d.toSQLStringValue(*(src.(*interface{}))) default: - return "", errors.New("could not parse type") + return "", wErrors.New("could not parse type") } return "", nil diff --git a/pkg/reader/postgres/postgres.go b/pkg/reader/postgres/postgres.go index a8303ef..38ade71 100644 --- a/pkg/reader/postgres/postgres.go +++ b/pkg/reader/postgres/postgres.go @@ -4,8 +4,9 @@ import ( "database/sql" "strings" - "github.com/hellofresh/klepto/pkg/reader" _ "github.com/lib/pq" // import postgres driver + + "github.com/hellofresh/klepto/pkg/reader" ) type driver struct{} From fc9a5e47e0dcf138e84f92787f202092b3fe9e0d Mon Sep 17 00:00:00 2001 From: Vladimir Garvardt Date: Thu, 2 Apr 2020 16:50:04 +0200 Subject: [PATCH 2/4] Spec config must exist --- pkg/config/config.go | 22 +++++----------------- pkg/config/config_test.go | 3 +++ 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 3d180f8..9d46170 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -1,8 +1,6 @@ package config import ( - "os" - wErrors "github.com/pkg/errors" log "github.com/sirupsen/logrus" "github.com/spf13/viper" @@ -77,23 +75,13 @@ func (t Tables) FindByName(name string) *Table { // LoadSpecFromFile loads klepto spec from file func LoadSpecFromFile(configPath string) (*Spec, error) { - if configPath != "" { - // Use config file from the flag. - log.Debugf("Reading config from %s ...", configPath) - viper.SetConfigFile(configPath) - } else { - log.Debugf("Reading config from %s ...", DefaultConfigFileName) - - cwd, err := os.Getwd() - if err != nil { - return nil, wErrors.Wrap(err, "can't find current working directory") - } - - viper.SetConfigName(".klepto") - viper.AddConfigPath(cwd) - viper.AddConfigPath(".") + if configPath == "" { + return nil, wErrors.New("config file path can not be empty") } + log.Debugf("Reading config from %s ...", configPath) + viper.SetConfigFile(configPath) + err := viper.ReadInConfig() if err != nil { return nil, wErrors.Wrap(err, "could not read configurations") diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 282761d..b39732f 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -10,6 +10,9 @@ import ( ) func TestLoadSpecFromFile(t *testing.T) { + _, err := LoadSpecFromFile("") + require.Error(t, err) + cwd, err := os.Getwd() require.NoError(t, err) From 527b44cf646d153ffc964a0a4071592d91809176 Mon Sep 17 00:00:00 2001 From: Vladimir Garvardt Date: Thu, 2 Apr 2020 16:52:16 +0200 Subject: [PATCH 3/4] Get cmd persistent flags only once --- cmd/steal.go | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/cmd/steal.go b/cmd/steal.go index eccea89..c389bc6 100644 --- a/cmd/steal.go +++ b/cmd/steal.go @@ -61,18 +61,19 @@ func NewStealCmd() *cobra.Command { }, } - cmd.PersistentFlags().StringVarP(&opts.configPath, "config", "c", config.DefaultConfigFileName, "Path to config file") - cmd.PersistentFlags().StringVarP(&opts.from, "from", "f", "mysql://root:root@tcp(localhost:3306)/klepto", "Database dsn to steal from") - cmd.PersistentFlags().StringVarP(&opts.to, "to", "t", "os://stdout/", "Database to output to (default writes to stdOut)") - cmd.PersistentFlags().IntVar(&opts.concurrency, "concurrency", runtime.NumCPU(), "Sets the amount of dumps to be performed concurrently") - cmd.PersistentFlags().DurationVar(&opts.readOpts.timeout, "read-timeout", 5*time.Minute, "Sets the timeout for read operations") - cmd.PersistentFlags().DurationVar(&opts.readOpts.maxConnLifetime, "read-conn-lifetime", 0, "Sets the maximum amount of time a connection may be reused on the read database") - cmd.PersistentFlags().IntVar(&opts.readOpts.maxConns, "read-max-conns", 5, "Sets the maximum number of open connections to the read database") - cmd.PersistentFlags().IntVar(&opts.readOpts.maxIdleConns, "read-max-idle-conns", 0, "Sets the maximum number of connections in the idle connection pool for the read database") - cmd.PersistentFlags().DurationVar(&opts.writeOpts.timeout, "write-timeout", 30*time.Second, "Sets the timeout for write operations") - cmd.PersistentFlags().DurationVar(&opts.writeOpts.maxConnLifetime, "write-conn-lifetime", 0, "Sets the maximum amount of time a connection may be reused on the write database") - cmd.PersistentFlags().IntVar(&opts.writeOpts.maxConns, "write-max-conns", 5, "Sets the maximum number of open connections to the write database") - cmd.PersistentFlags().IntVar(&opts.writeOpts.maxIdleConns, "write-max-idle-conns", 0, "Sets the maximum number of connections in the idle connection pool for the write database") + persistentFlags := cmd.PersistentFlags() + persistentFlags.StringVarP(&opts.configPath, "config", "c", config.DefaultConfigFileName, "Path to config file") + persistentFlags.StringVarP(&opts.from, "from", "f", "mysql://root:root@tcp(localhost:3306)/klepto", "Database dsn to steal from") + persistentFlags.StringVarP(&opts.to, "to", "t", "os://stdout/", "Database to output to (default writes to stdOut)") + persistentFlags.IntVar(&opts.concurrency, "concurrency", runtime.NumCPU(), "Sets the amount of dumps to be performed concurrently") + persistentFlags.DurationVar(&opts.readOpts.timeout, "read-timeout", 5*time.Minute, "Sets the timeout for read operations") + persistentFlags.DurationVar(&opts.readOpts.maxConnLifetime, "read-conn-lifetime", 0, "Sets the maximum amount of time a connection may be reused on the read database") + persistentFlags.IntVar(&opts.readOpts.maxConns, "read-max-conns", 5, "Sets the maximum number of open connections to the read database") + persistentFlags.IntVar(&opts.readOpts.maxIdleConns, "read-max-idle-conns", 0, "Sets the maximum number of connections in the idle connection pool for the read database") + persistentFlags.DurationVar(&opts.writeOpts.timeout, "write-timeout", 30*time.Second, "Sets the timeout for write operations") + persistentFlags.DurationVar(&opts.writeOpts.maxConnLifetime, "write-conn-lifetime", 0, "Sets the maximum amount of time a connection may be reused on the write database") + persistentFlags.IntVar(&opts.writeOpts.maxConns, "write-max-conns", 5, "Sets the maximum number of open connections to the write database") + persistentFlags.IntVar(&opts.writeOpts.maxIdleConns, "write-max-idle-conns", 0, "Sets the maximum number of connections in the idle connection pool for the write database") return cmd } From 0216803eeb90261062bc8ac8b0b380dc2a3481c8 Mon Sep 17 00:00:00 2001 From: Vladimir Garvardt Date: Thu, 2 Apr 2020 17:13:23 +0200 Subject: [PATCH 4/4] More obvious way of returning dumper init erorrs --- pkg/dumper/dumper.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/dumper/dumper.go b/pkg/dumper/dumper.go index 1756d71..b4ed9d6 100644 --- a/pkg/dumper/dumper.go +++ b/pkg/dumper/dumper.go @@ -56,11 +56,13 @@ func NewDumper(opts ConnOpts, rdr reader.Reader) (dumper Dumper, err error) { return false }) - if dumper == nil && err == nil { - err = fmt.Errorf("no supported driver found for dumper DSN %q", opts.DSN) + if err != nil { + return nil, wErrors.Wrapf(err, "could not create dumper for DSN: %q", opts.DSN) } - err = wErrors.Wrapf(err, "could not create dumper for DSN: %q", opts.DSN) + if dumper == nil { + return nil, fmt.Errorf("no supported driver found for dumper DSN %q", opts.DSN) + } return }