Permalink
Browse files

Major internal refactor to use subpackages

This PR moves much of Skeema's logic out of the main package and into several
new sub-packages, which can be imported by other applications if desired.

Functionality is largely unchanged, and no new features have been added. But a
few foundational benefits of this work include:

* The codebase no longer assumes a 1:1 mapping between *.sql files and tables.
  This will eventually permit non-table object types (views, procs, grants, etc)
  to be stored in the same repo as schemas, if desired. See #41 for background.

* In the upcoming Skeema 1.1.x series, it will be possible to use a local Docker
  instance for temp schema operations. This performs better in high-latency,
  high-table-count scenarios; see #25 for background.

* The limit on max *.sql file size has been removed. Closes #34.

* `skeema pull` now performs much better than before, as long as --normalize is
  enabled (which it is by default).

* The code supporting `skeema push --concurrent-instances` is now much cleaner
  and more idiomatic.

* Test coverage has been improved.
  • Loading branch information...
evanelias committed Oct 26, 2018
1 parent 51f1564 commit 42d3772bf70515135c4a2b8b6b7dfa2addae2142
Showing with 4,367 additions and 2,670 deletions.
  1. +4 −7 .travis.yml
  2. +11 −5 Gopkg.lock
  3. +4 −4 Gopkg.toml
  4. +209 −0 applier/applier.go
  5. +108 −0 applier/applier_test.go
  6. +69 −85 { → applier}/ddlstatement.go
  7. +69 −37 { → applier}/ddlstatement_test.go
  8. +59 −0 applier/printer.go
  9. +199 −0 applier/target.go
  10. +277 −0 applier/target_test.go
  11. +80 −0 applier/verifier.go
  12. +23 −22 cmd_add_env.go
  13. +3 −2 cmd_diff.go
  14. +82 −36 cmd_init.go
  15. +90 −59 cmd_lint.go
  16. +256 −224 cmd_pull.go
  17. +36 −246 cmd_push.go
  18. +0 −126 configutils_test.go
  19. +0 −709 dir.go
  20. +6 −2 doc/options.md
  21. +545 −0 fs/dir.go
  22. +96 −24 { → fs}/dir_test.go
  23. +147 −0 fs/sqlfile.go
  24. +182 −0 fs/sqlfile_test.go
  25. +331 −0 fs/statement.go
  26. +40 −0 fs/statement_test.go
  27. +61 −0 fs/testing.go
  28. +0 −11 log.go
  29. +7 −1 skeema.go
  30. +134 −152 skeema_cmd_test.go
  31. +74 −121 skeema_test.go
  32. +0 −127 sqlfile.go
  33. +0 −133 sqlfile_test.go
  34. +0 −369 target.go
  35. +3 −0 testdata/applier/multi/.skeema
  36. +1 −0 testdata/applier/multi/shards/.skeema
  37. +4 −0 testdata/applier/multi/shards/bar.sql
  38. +4 −0 testdata/applier/multi/shards/foo.sql
  39. +3 −0 testdata/applier/schemaerror/.skeema
  40. +2 −0 testdata/applier/schemaerror/four/.skeema
  41. 0 testdata/applier/schemaerror/four/four.sql
  42. +1 −0 testdata/applier/schemaerror/one/.skeema
  43. +4 −0 testdata/applier/schemaerror/one/foo.sql
  44. +1 −0 testdata/applier/schemaerror/three/.skeema
  45. 0 testdata/applier/schemaerror/three/three.sql
  46. +1 −0 testdata/applier/schemaerror/two/.skeema
  47. +4 −0 testdata/applier/schemaerror/two/bar.sql
  48. +3 −0 testdata/applier/simple/.skeema
  49. +1 −0 testdata/applier/simple/one/.skeema
  50. +4 −0 testdata/applier/simple/one/foo.sql
  51. +1 −0 testdata/applier/simple/two/.skeema
  52. +4 −0 testdata/applier/simple/two/bar.sql
  53. +3 −0 testdata/applier/sqlerror/.skeema
  54. +1 −0 testdata/applier/sqlerror/one/.skeema
  55. +3 −0 testdata/applier/sqlerror/one/bad.sql
  56. +4 −0 testdata/applier/sqlerror/one/foo.sql
  57. +1 −0 testdata/applier/sqlerror/two/.skeema
  58. +4 −0 testdata/applier/sqlerror/two/bar.sql
  59. +36 −0 testdata/statements.sql
  60. +3 −0 testdata/tempschema1.sql
  61. +1 −1 { → util}/cache.go
  62. +31 −0 util/cache_test.go
  63. +48 −41 configutils.go → util/config.go
  64. +197 −0 util/config_test.go
  65. +24 −58 { → util}/shellout.go
  66. +36 −45 { → util}/shellout_test.go
  67. +3 −3 vendor/github.com/skeema/mybase/README.md
  68. +6 −0 vendor/github.com/skeema/mybase/cli.go
  69. +11 −4 vendor/github.com/skeema/mybase/command.go
  70. +8 −12 vendor/github.com/skeema/mybase/config.go
  71. +5 −1 vendor/github.com/skeema/mybase/file.go
  72. +13 −0 vendor/github.com/skeema/mybase/testing.go
  73. +22 −3 vendor/github.com/skeema/tengo/instance.go
  74. +3 −0 vendor/golang.org/x/sync/AUTHORS
  75. +3 −0 vendor/golang.org/x/sync/CONTRIBUTORS
  76. +27 −0 vendor/golang.org/x/sync/LICENSE
  77. +22 −0 vendor/golang.org/x/sync/PATENTS
  78. +67 −0 vendor/golang.org/x/sync/errgroup/errgroup.go
  79. +92 −0 workspace/tempschema.go
  80. +92 −0 workspace/tempschema_test.go
  81. +193 −0 workspace/workspace.go
  82. +165 −0 workspace/workspace_test.go
View
@@ -24,13 +24,10 @@ before_install:
- go get github.com/mattn/goveralls
script:
- go test -v -coverprofile=coverage.out -covermode=count
- go vet
- test -z "$(gofmt -s -d *.go 2>&1)"
- golint -set_exit_status
after_script:
- goveralls -coverprofile=coverage.out -service=travis-ci
- $GOPATH/bin/goveralls -v -service=travis-ci
- go vet ./...
- test -z "$(gofmt -s -d {.,fs,workspace,util}/*.go 2>&1)"
- go list -f '{{.Dir}}' ./... | xargs golint -set_exit_status
deploy:
- provider: script
View

Some generated files are not rendered by default. Learn more.

Oops, something went wrong.
View
@@ -25,10 +25,6 @@
# unused-packages = true
[[constraint]]
name = "github.com/pmezard/go-difflib"
version = "1.0.0"
[[constraint]]
name = "github.com/sirupsen/logrus"
version = "1.0.5"
@@ -45,6 +41,10 @@
branch = "master"
name = "golang.org/x/crypto"
[[constraint]]
branch = "master"
name = "golang.org/x/sync"
[prune]
go-tests = true
unused-packages = true
View
@@ -0,0 +1,209 @@
// Package applier handles execution of generating diffs between schemas, and
// appropriate application of the generated DDL.
package applier
import (
"context"
"fmt"
"strings"
log "github.com/sirupsen/logrus"
"github.com/skeema/skeema/fs"
"github.com/skeema/tengo"
)
// Result stores the overall result of all operations the worker has completed.
type Result struct {
Differences bool
SkipCount int
UnsupportedCount int
}
// Worker reads TargetGroups from the input channel and performs the appropriate
// diff/push operation on each target per TargetGroup. When there are no more
// TargetGroups to read, it writes its aggregate Result to the output channel.
// If a fatal error occurs, it will be returned immediately; Worker is meant to
// be called via an errgroup (see golang.org/x/sync/errgroup).
func Worker(ctx context.Context, targetGroups <-chan TargetGroup, results chan<- Result, printer *Printer) error {
var result Result
for tg := range targetGroups {
TargetsInGroup:
for _, t := range tg { // iterate over each Target in the TargetGroup
// Get schema name from t.SchemaFromDir, NOT t.SchemaFromInstance, since
// t.SchemaFromInstance will be nil if the schema doesn't exist yet
schemaName := t.SchemaFromDir.Name
dryRun := t.Dir.Config.GetBool("dry-run")
brief := dryRun && t.Dir.Config.GetBool("brief")
if dryRun {
log.Infof("Generating diff of %s %s vs %s/*.sql", t.Instance, schemaName, t.Dir)
} else {
log.Infof("Pushing changes from %s/*.sql to %s %s", t.Dir, t.Instance, schemaName)
}
diff := tengo.NewSchemaDiff(t.SchemaFromInstance, t.SchemaFromDir)
var targetStmtCount int
if diff.SchemaDDL != "" {
printer.syncPrintf(t.Instance, "", "%s;\n", diff.SchemaDDL)
targetStmtCount++
if !dryRun {
var schemaDDLErr error
if strings.HasPrefix(diff.SchemaDDL, "CREATE DATABASE") && t.SchemaFromInstance == nil {
_, schemaDDLErr = t.Instance.CreateSchema(schemaName, t.SchemaFromDir.CharSet, t.SchemaFromDir.Collation)
} else if strings.HasPrefix(diff.SchemaDDL, "ALTER DATABASE") {
schemaDDLErr = t.Instance.AlterSchema(t.SchemaFromInstance.Name, t.SchemaFromDir.CharSet, t.SchemaFromDir.Collation)
} else {
return fmt.Errorf("Refusing to run unexpectedly-generated schema-level DDL: %s", diff.SchemaDDL)
}
if schemaDDLErr != nil {
log.Errorf("Error running DDL on %s %s: %s", t.Instance, schemaName, schemaDDLErr)
skipped := len(diff.TableDiffs) + 1
result.SkipCount += skipped
if skipped > 1 {
log.Warnf("Skipping %d remaining operations for %s %s due to previous error", skipped-1, t.Instance, schemaName)
}
continue
}
}
}
if t.Dir.Config.GetBool("verify") && len(diff.TableDiffs) > 0 && !brief {
if err := VerifyDiff(diff, t); err != nil {
return err
}
}
// Obtain StatementModifiers based on the dir's config
mods, err := StatementModifiersForDir(t.Dir)
if err != nil {
return ConfigError(err.Error())
}
if configFlavor := tengo.NewFlavor(t.Dir.Config.Get("flavor")); configFlavor != tengo.FlavorUnknown {
mods.Flavor = configFlavor
} else {
mods.Flavor = t.Instance.Flavor()
}
// Build DDLStatements for each TableDiff, handling pre-execution errors
// accordingly
ddls := make([]*DDLStatement, 0, len(diff.TableDiffs))
for i, tableDiff := range diff.TableDiffs {
ddl, err := NewDDLStatement(tableDiff, mods, t)
if ddl == nil && err == nil {
continue // Skip entirely if mods made the statement a noop
}
targetStmtCount++
result.Differences = true
if unsupportedErr, ok := err.(*tengo.UnsupportedDiffError); ok {
result.UnsupportedCount++
log.Warnf("Skipping table %s: unable to generate ALTER TABLE due to use of unsupported features. Use --debug for more information.", unsupportedErr.Name)
DebugLogUnsupportedDiff(unsupportedErr)
continue
} else if err != nil {
result.SkipCount++
log.Errorf(err.Error())
skipped := len(diff.TableDiffs) - i
result.SkipCount += skipped
if skipped > 1 {
log.Warnf("Skipping %d remaining operations for %s %s due to previous error", skipped-1, t.Instance, schemaName)
}
continue TargetsInGroup
}
if dryRun {
printer.syncPrintf(t.Instance, schemaName, "%s\n", ddl.String())
} else {
// IMPORTANT: only append to ddls if NOT dry-run. Do not move this out of
// this conditional!
ddls = append(ddls, ddl)
}
}
// Execute DDL. (ddls will be empty if dryRun, due to conditional above)
for i, ddl := range ddls {
printer.syncPrintf(t.Instance, schemaName, "%s\n", ddl.String())
if err := ddl.Execute(); err != nil {
log.Errorf("Error running DDL on %s %s: %s", t.Instance, schemaName, err)
skipped := len(ddls) - i
result.SkipCount += skipped
if skipped > 1 {
log.Warnf("Skipping %d remaining operations for %s %s due to previous error", skipped-1, t.Instance, schemaName)
}
break
}
}
if targetStmtCount == 0 {
log.Infof("%s %s: No differences found\n", t.Instance, schemaName)
} else {
verb := "push"
if dryRun {
verb = "diff"
}
log.Infof("%s %s: %s complete\n", t.Instance, schemaName, verb)
}
// Exit early if context cancelled
select {
case <-ctx.Done():
return nil
default:
}
}
}
results <- result
return nil
}
// SumResults adds up the supplied results to return a single combined result.
func SumResults(results []Result) Result {
var total Result
for _, r := range results {
total.Differences = total.Differences || r.Differences
total.SkipCount += r.SkipCount
total.UnsupportedCount += r.UnsupportedCount
}
return total
}
// StatementModifiersForDir returns a set of DDL modifiers, based on the
// directory's configuration.
func StatementModifiersForDir(dir *fs.Dir) (mods tengo.StatementModifiers, err error) {
mods.NextAutoInc = tengo.NextAutoIncIfIncreased
forceAllowUnsafe := dir.Config.GetBool("brief") && dir.Config.GetBool("dry-run")
mods.AllowUnsafe = forceAllowUnsafe || dir.Config.GetBool("allow-unsafe")
if dir.Config.GetBool("exact-match") {
mods.StrictIndexOrder = true
mods.StrictForeignKeyNaming = true
}
if mods.AlgorithmClause, err = dir.Config.GetEnum("alter-algorithm", "INPLACE", "COPY", "INSTANT", "DEFAULT"); err != nil {
return
}
if mods.LockClause, err = dir.Config.GetEnum("alter-lock", "NONE", "SHARED", "EXCLUSIVE", "DEFAULT"); err != nil {
return
}
if mods.IgnoreTable, err = dir.Config.GetRegexp("ignore-table"); err != nil {
return
}
return
}
// DebugLogUnsupportedDiff logs (at Debug level) the reason why a table is
// unsupported for diff/alter operations.
func DebugLogUnsupportedDiff(err *tengo.UnsupportedDiffError) {
for _, line := range strings.Split(err.ExtendedError(), "\n") {
if len(line) > 0 {
log.Debug(line)
}
}
}
// ConfigError represents a configuration problem encountered at runtime.
type ConfigError string
// Error satisfies the builtin error interface.
func (ce ConfigError) Error() string {
return string(ce)
}
Oops, something went wrong.

0 comments on commit 42d3772

Please sign in to comment.