cli: improve cli package error propogation in migrate operations

https://github.com/hasura/graphql-engine-mono/pull/1887

GitOrigin-RevId: 40c7e1a16e8dd25a46e0c58afc7cbc9169af8f0b
This commit is contained in:
Aravind K P 2021-08-16 12:13:11 +05:30 committed by hasura-bot
parent e66bf923cf
commit f2e74da5aa
7 changed files with 224 additions and 93 deletions

View File

@ -109,6 +109,13 @@ func (o *MigrateApplyOptions) Validate() error {
o.Source.Name = ""
}
if o.DryRun && o.SkipExecution {
return errors.New("both --skip-execution and --dry-run flags cannot be used together")
}
if o.DryRun && o.AllDatabases {
return errors.New("both --all-databases and --dry-run flags cannot be used together")
}
if o.EC.Config.Version >= cli.V3 {
if !o.AllDatabases && len(o.Source.Name) == 0 {
return fmt.Errorf("unable to determine database on which migration should be applied")
@ -141,87 +148,111 @@ type errDatabaseMigrationDirectoryNotFound struct {
func (e *errDatabaseMigrationDirectoryNotFound) Error() string {
return e.message
}
func (o *MigrateApplyOptions) Run() error {
results, err := o.Apply()
if err != nil {
return err
}
for result := range results {
if result.Error != nil {
o.EC.Logger.Errorf("%v", result.Error)
} else if len(result.Message) > 0 {
o.EC.Logger.Infof(result.Message)
}
}
return nil
}
type MigrateApplyResult struct {
DatabaseName string
Message string
Error error
}
func (o *MigrateApplyOptions) Apply() (chan MigrateApplyResult, error) {
resultChan := make(chan MigrateApplyResult)
handleError := func(err error) (string, error) {
if err == migrate.ErrNoChange {
return fmt.Sprintf("nothing to apply on database %s", o.Source.Name), nil
} else if e, ok := err.(*os.PathError); ok {
// If Op is first, then log No migrations to apply
if e.Op == "first" {
return fmt.Sprintf("nothing to apply on database %s", o.Source.Name), nil
}
} else if e, ok := err.(*errDatabaseMigrationDirectoryNotFound); ok {
// check if the returned error is a directory not found error
// ie might be because a migrations/<source_name> directory is not found
// if so skip this
return "", fmt.Errorf("skipping applying migrations on database %s, encountered: \n%s", o.Source.Name, e.Error())
} else if err != nil {
return "", fmt.Errorf("skipping applying migrations on database %s, encountered: \n%v", o.Source.Name, err)
}
return "", nil
}
if len(o.Source.Name) == 0 {
o.Source = o.EC.Source
}
if err := o.Validate(); err != nil {
return err
}
if o.DryRun && o.SkipExecution {
return errors.New("both --skip-execution and --dry-run flags cannot be used together")
return nil, err
}
if o.AllDatabases && o.EC.Config.Version >= cli.V3 {
o.EC.Spin("getting lists of databases from server ")
sourcesAndKind, err := metadatautil.GetSourcesAndKind(o.EC.APIClient.V1Metadata.ExportMetadata)
o.EC.Spinner.Stop()
if err != nil {
return fmt.Errorf("determing list of connected sources and kind: %w", err)
return nil, err
}
for _, source := range sourcesAndKind {
o.Source.Kind = source.Kind
o.Source.Name = source.Name
go func() {
defer close(resultChan)
for _, source := range sourcesAndKind {
result := MigrateApplyResult{
DatabaseName: source.Name,
Message: "",
Error: nil,
}
o.Source.Kind = source.Kind
o.Source.Name = source.Name
if !o.DryRun {
o.EC.Spin(fmt.Sprintf("Applying migrations on database: %s ", o.Source.Name))
}
err := o.Exec()
o.EC.Spinner.Stop()
if err != nil {
result.Message, result.Error = handleError(err)
} else {
result.Message = fmt.Sprintf("migrations applied on database: %s", o.Source.Name)
}
resultChan <- result
}
}()
} else {
go func() {
defer close(resultChan)
if !o.DryRun {
o.EC.Spin(fmt.Sprintf("Applying migrations on database: %s ", o.Source.Name))
o.EC.Spin("Applying migrations...")
}
result := MigrateApplyResult{
DatabaseName: o.Source.Name,
Message: "",
Error: nil,
}
err := o.Exec()
o.EC.Spinner.Stop()
if err != nil {
if err == migrate.ErrNoChange {
o.EC.Logger.Infof("nothing to apply on database: %s", o.Source.Name)
continue
}
if e, ok := err.(*os.PathError); ok {
// If Op is first, then log No migrations to apply
if e.Op == "first" {
o.EC.Logger.Infof("nothing to apply on database: %s", o.Source.Name)
continue
}
}
// check if the returned error is a directory not found error
// ie might be because a migrations/<source_name> directory is not found
// if so skip this
if e, ok := err.(*errDatabaseMigrationDirectoryNotFound); ok {
o.EC.Logger.Errorf("skipping applying migrations for database %s, encountered: \n%s", o.Source.Name, e.Error())
continue
}
o.EC.Logger.Errorf("skipping applying migrations for database %s, encountered: \n%v", o.Source.Name, err)
continue
result.Message, result.Error = handleError(err)
} else {
result.Message = "migrations applied"
}
o.EC.Logger.Infof("applied migrations on database: %s", o.Source.Name)
}
} else {
if !o.DryRun {
o.EC.Spin("Applying migrations...")
}
err := o.Exec()
o.EC.Spinner.Stop()
if err != nil {
if err == migrate.ErrNoChange {
o.EC.Logger.Info("nothing to apply")
return nil
}
// check if the returned error is a directory not found error
// ie might be because a migrations/<source_name> directory is not found
// if so skip this
if e, ok := err.(*errDatabaseMigrationDirectoryNotFound); ok {
return fmt.Errorf("applying migrations on database %s: %w", o.Source.Name, e)
}
if e, ok := err.(*os.PathError); ok {
// If Op is first, then log No migrations to apply
if e.Op == "first" {
o.EC.Logger.Info("nothing to apply")
return nil
}
}
return fmt.Errorf("apply failed\n%w", err)
}
if !o.DryRun {
o.EC.Logger.Info("migrations applied")
}
resultChan <- result
}()
}
return nil
return resultChan, nil
}
func (o *MigrateApplyOptions) Exec() error {
if o.EC.Config.Version >= cli.V3 {
// check if a migrations directory exists for source in project

View File

@ -110,6 +110,20 @@ func GetSourcesAndKind(exportMetadata func() (io.Reader, error)) ([]Source, erro
return sources, nil
}
var ErrNoConnectedSources = errors.New("0 connected sources found on hasura")
// GetSourcesAndKindStrict is like GetSourcesAndKind but will return an error when no sources are found
func GetSourcesAndKindStrict(exportMetadata func() (io.Reader, error)) ([]Source, error) {
sources, err := GetSourcesAndKind(exportMetadata)
if err != nil {
return nil, err
}
if len(sources) == 0 {
return nil, ErrNoConnectedSources
}
return sources, nil
}
func DatabaseChooserUI(exportMetadata func() (io.Reader, error)) (string, error) {
sources, err := GetSources(exportMetadata)
if err != nil {

View File

@ -220,7 +220,7 @@ func TestProjectMetadata_GetInconsistentMetadata(t *testing.T) {
// - apply metadata
// - drop a table via run_sql API
// - reload metadata
err := migrations.Apply(migrate.ApplyOnAllDatabases())
_, err := migrations.Apply(migrate.ApplyOnAllDatabases())
require.NoError(t, err)
_, err = metadata.Apply()
require.NoError(t, err)

View File

@ -41,9 +41,17 @@ func ApplyVersion(version string, direction MigrationDirection) ProjectMigration
}
}
func (p *projectMigrationsApplier) Apply(opts ...ProjectMigrationApplierOption) error {
func (p *projectMigrationsApplier) apply(opts ...ProjectMigrationApplierOption) ([]ApplyResult, error) {
for _, opt := range opts {
opt(p)
}
return p.opts.Run()
var results []ApplyResult
resultChan, err := p.opts.Apply()
if err != nil {
return nil, err
}
for v := range resultChan {
results = append(results, ApplyResult(v))
}
return results, nil
}

View File

@ -4,6 +4,8 @@ import (
"fmt"
"io"
"github.com/hasura/graphql-engine/cli/v2/commands"
"github.com/hasura/graphql-engine/cli/v2"
"github.com/spf13/viper"
)
@ -28,9 +30,11 @@ func (p *ProjectMigrate) StatusJSON(opts ...ProjectMigrationStatusOption) (io.Re
return lister.StatusJSON(opts...)
}
func (p *ProjectMigrate) Apply(opts ...ProjectMigrationApplierOption) error {
type ApplyResult commands.MigrateApplyResult
func (p *ProjectMigrate) Apply(opts ...ProjectMigrationApplierOption) ([]ApplyResult, error) {
applier := newProjectMigrationsApplier(p.ec)
return applier.Apply(opts...)
return applier.apply(opts...)
}
func NewProjectMigrate(projectDirectory string, opts ...ProjectMigrateOption) (*ProjectMigrate, error) {

View File

@ -37,6 +37,7 @@ func TestProjectMigrate_ApplyConfig_v3(t *testing.T) {
name string
fields fields
args args
want []ApplyResult
wantErr bool
}{
{
@ -48,6 +49,18 @@ func TestProjectMigrate_ApplyConfig_v3(t *testing.T) {
args{
[]ProjectMigrationApplierOption{ApplyOnAllDatabases()},
},
[]ApplyResult{
{
"s1",
"migrations applied on database: s1",
nil,
},
{
"s2",
"migrations applied on database: s2",
nil,
},
},
false,
},
{
@ -59,6 +72,13 @@ func TestProjectMigrate_ApplyConfig_v3(t *testing.T) {
args{
[]ProjectMigrationApplierOption{ApplyOnDatabaseName("s1"), ApplyVersion("1623841477474", MigrationDirectionDown)},
},
[]ApplyResult{
{
"s1",
"migrations applied",
nil,
},
},
false,
},
{
@ -70,6 +90,13 @@ func TestProjectMigrate_ApplyConfig_v3(t *testing.T) {
args{
[]ProjectMigrationApplierOption{ApplyOnDatabaseName("s1"), ApplyVersion("1623841477474", MigrationDirectionUp)},
},
[]ApplyResult{
{
"s1",
"migrations applied",
nil,
},
},
false,
},
}
@ -77,11 +104,12 @@ func TestProjectMigrate_ApplyConfig_v3(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
p, err := NewProjectMigrate(tt.fields.projectDirectory, WithAdminSecret(testutil.TestAdminSecret), WithEndpoint(tt.fields.endpointString))
require.NoError(t, err)
err = p.Apply(tt.args.opts...)
got, err := p.Apply(tt.args.opts...)
if tt.wantErr {
require.Error(t, err)
} else {
require.NoError(t, err)
require.Equal(t, tt.want, got)
}
})
}
@ -103,6 +131,7 @@ func TestProjectMigrate_Apply_Configv2(t *testing.T) {
name string
fields fields
args args
want []ApplyResult
wantErr bool
}{
{
@ -115,6 +144,11 @@ func TestProjectMigrate_Apply_Configv2(t *testing.T) {
args{
[]ProjectMigrationApplierOption{ApplyOnAllDatabases()},
},
[]ApplyResult{
{
Message: "migrations applied",
},
},
false,
},
{
@ -127,6 +161,11 @@ func TestProjectMigrate_Apply_Configv2(t *testing.T) {
args{
[]ProjectMigrationApplierOption{ApplyVersion("1623842054907", MigrationDirectionDown)},
},
[]ApplyResult{
{
Message: "migrations applied",
},
},
false,
},
{
@ -139,7 +178,12 @@ func TestProjectMigrate_Apply_Configv2(t *testing.T) {
args{
[]ProjectMigrationApplierOption{ApplyVersion("1623842054907", MigrationDirectionDown)},
},
true,
[]ApplyResult{
{
Error: fmt.Errorf("skipping applying migrations on database , encountered: \nMigration not applied in database"),
},
},
false,
},
{
"can apply up migrations of a version on a config v2 project",
@ -151,6 +195,11 @@ func TestProjectMigrate_Apply_Configv2(t *testing.T) {
args{
[]ProjectMigrationApplierOption{ApplyVersion("1623842054907", MigrationDirectionUp)},
},
[]ApplyResult{
{
Message: "migrations applied",
},
},
false,
},
}
@ -158,11 +207,12 @@ func TestProjectMigrate_Apply_Configv2(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
p, err := NewProjectMigrate(tt.fields.projectDirectory, WithAdminSecret(testutil.TestAdminSecret), WithEndpoint(tt.fields.endpointString))
require.NoError(t, err)
err = p.Apply(tt.args.opts...)
got, err := p.Apply(tt.args.opts...)
if tt.wantErr {
require.Error(t, err)
} else {
require.NoError(t, err)
require.Equal(t, tt.want, got)
}
})
}
@ -287,7 +337,8 @@ func TestProjectMigrate_Status_ConfigV2(t *testing.T) {
]`,
false,
func(t *testing.T, p *ProjectMigrate) {
assert.NoError(t, p.Apply(ApplyOnAllDatabases()))
_, err := p.Apply(ApplyOnAllDatabases())
assert.NoError(t, err)
},
},
}
@ -332,25 +383,24 @@ func TestProjectMigrate_Status_ConfigV3(t *testing.T) {
type fields struct {
projectDirectory string
adminSecret string
endpointString string
}
type args struct {
opts []ProjectMigrationStatusOption
}
tests := []struct {
name string
fields fields
args args
want string
wantErr bool
before func(t *testing.T, p *ProjectMigrate)
name string
fields fields
args args
want string
wantErr bool
testSetup func() (hgeEndpoint string, teardown func())
before func(t *testing.T, p *ProjectMigrate)
}{
{
"can get status of migrations",
fields{
projectDirectory: "testdata/projectv3",
adminSecret: "",
endpointString: hgeEndpoint,
},
args{
opts: []ProjectMigrationStatusOption{},
@ -426,6 +476,7 @@ func TestProjectMigrate_Status_ConfigV3(t *testing.T) {
}
]`,
false,
func() (string, func()) { return hgeEndpoint, func() {} },
func(t *testing.T, p *ProjectMigrate) {},
},
{
@ -433,7 +484,6 @@ func TestProjectMigrate_Status_ConfigV3(t *testing.T) {
fields{
projectDirectory: "testdata/projectv3",
adminSecret: "",
endpointString: hgeEndpoint,
},
args{
opts: []ProjectMigrationStatusOption{},
@ -510,32 +560,56 @@ func TestProjectMigrate_Status_ConfigV3(t *testing.T) {
}
]`,
false,
func() (string, func()) { return hgeEndpoint, func() {} },
func(t *testing.T, p *ProjectMigrate) {
assert.NoError(t, p.Apply(ApplyOnAllDatabases()))
_, err := p.Apply(ApplyOnAllDatabases())
assert.NoError(t, err)
},
},
{
"can throw an error when no databases are connected to hge",
fields{
projectDirectory: "testdata/projectv3",
adminSecret: "",
},
args{
opts: []ProjectMigrationStatusOption{},
},
``,
true,
func() (string, func()) {
port, teardown := testutil.StartHasuraWithMetadataDatabase(t, testutil.HasuraDockerImage)
return fmt.Sprintf("http://%s:%s", testutil.Hostname, port), teardown
},
func(t *testing.T, p *ProjectMigrate) {
_, err := p.Apply(ApplyOnAllDatabases())
assert.NoError(t, err)
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
p, err := NewProjectMigrate(tt.fields.projectDirectory, WithAdminSecret(testutil.TestAdminSecret), WithEndpoint(tt.fields.endpointString))
hgeEndpoint, setupTeardown := tt.testSetup()
defer setupTeardown()
p, err := NewProjectMigrate(tt.fields.projectDirectory, WithAdminSecret(testutil.TestAdminSecret), WithEndpoint(hgeEndpoint))
require.NoError(t, err)
applier, err := NewProjectMigrate(tt.fields.projectDirectory, WithAdminSecret(testutil.TestAdminSecret), WithEndpoint(tt.fields.endpointString))
applier, err := NewProjectMigrate(tt.fields.projectDirectory, WithAdminSecret(testutil.TestAdminSecret), WithEndpoint(hgeEndpoint))
require.NoError(t, err)
tt.before(t, applier)
got, err := p.status(tt.args.opts...)
if tt.wantErr {
require.Error(t, err)
}
require.NoError(t, err)
gotJSON, err := json.Marshal(got)
require.NoError(t, err)
require.JSONEq(t, tt.want, string(gotJSON))
} else {
gotJSON, err := json.Marshal(got)
require.NoError(t, err)
require.JSONEq(t, tt.want, string(gotJSON))
statusJson, err := p.StatusJSON(tt.args.opts...)
require.NoError(t, err)
statusJsonb, err := ioutil.ReadAll(statusJson)
require.NoError(t, err)
require.JSONEq(t, tt.want, string(statusJsonb))
statusJson, err := p.StatusJSON(tt.args.opts...)
require.NoError(t, err)
statusJsonb, err := ioutil.ReadAll(statusJson)
require.NoError(t, err)
require.JSONEq(t, tt.want, string(statusJsonb))
}
})
}
}

View File

@ -29,7 +29,7 @@ func (p *projectMigrationsStatus) Status(opts ...ProjectMigrationStatusOption) (
}
if p.allDatabases {
metadataOps := cli.GetCommonMetadataOps(p.ec)
sources, err := metadatautil.GetSourcesAndKind(metadataOps.ExportMetadata)
sources, err := metadatautil.GetSourcesAndKindStrict(metadataOps.ExportMetadata)
if err != nil {
return nil, err
}