Skip to content

Commit

Permalink
Remove BeforeBackfill hook (#312)
Browse files Browse the repository at this point in the history
Remove the `BeforeBackfill` hook.

The `BeforeBackfill` hook was added along with the `BeforeStartDDL` and
`AfterStartDDL` hooks in #290, but is of limited use as a hook that runs
only after successful execution of start phase DDL. Prefer a simpler API
with fewer hooks until there is a demonstrated need for it.
  • Loading branch information
andrew-farries committed Mar 7, 2024
1 parent c88c060 commit 297dd38
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 38 deletions.
6 changes: 0 additions & 6 deletions pkg/roll/execute.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@ func (m *Roll) Start(ctx context.Context, migration *migrations.Migration, cbs .
return err
}

if m.migrationHooks.BeforeBackfill != nil {
if err := m.migrationHooks.BeforeBackfill(m); err != nil {
return fmt.Errorf("failed to execute BeforeBackfill hook: %w", err)
}
}

// perform backfills for the tables that require it
return m.performBackfills(ctx, tablesToBackfill)
}
Expand Down
32 changes: 2 additions & 30 deletions pkg/roll/execute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -478,15 +478,11 @@ func TestMigrationHooksAreInvoked(t *testing.T) {

options := []roll.Option{roll.WithMigrationHooks(roll.MigrationHooks{
BeforeStartDDL: func(m *roll.Roll) error {
_, err := m.PgConn().ExecContext(context.Background(), "CREATE TABLE IF NOT EXISTS before_start_ddl (id integer)")
_, err := m.PgConn().ExecContext(context.Background(), "CREATE TABLE before_start_ddl (id integer)")
return err
},
AfterStartDDL: func(m *roll.Roll) error {
_, err := m.PgConn().ExecContext(context.Background(), "CREATE TABLE IF NOT EXISTS after_start_ddl (id integer)")
return err
},
BeforeBackfill: func(m *roll.Roll) error {
_, err := m.PgConn().ExecContext(context.Background(), "CREATE TABLE IF NOT EXISTS before_backfill (id integer)")
_, err := m.PgConn().ExecContext(context.Background(), "CREATE TABLE after_start_ddl (id integer)")
return err
},
})}
Expand All @@ -508,30 +504,6 @@ func TestMigrationHooksAreInvoked(t *testing.T) {
// Complete the migration
err = mig.Complete(ctx)
assert.NoError(t, err)

// Insert some data into the table created by the migration
_, err = db.ExecContext(ctx, "INSERT INTO table1 (id, name) VALUES (1, 'alice')")
assert.NoError(t, err)

// Start a migration that requires a backfill
err = mig.Start(ctx, &migrations.Migration{
Name: "02_add_column",
Operations: migrations.Operations{
&migrations.OpAddColumn{
Table: "table1",
Column: migrations.Column{
Name: "description",
Type: "text",
Nullable: ptr(false),
},
Up: ptr("'this is a description'"),
},
},
})
assert.NoError(t, err)

// ensure that the before_backfill table was created
assert.True(t, tableExists(t, db, "public", "before_backfill"))
})
}

Expand Down
2 changes: 0 additions & 2 deletions pkg/roll/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ type MigrationHooks struct {
BeforeStartDDL func(*Roll) error
// AfterStartDDL is called after the DDL phase of migration start is complete
AfterStartDDL func(*Roll) error
// BeforeBackfill is called before the backfill phase of migration start
BeforeBackfill func(*Roll) error
}

type Option func(*options)
Expand Down

0 comments on commit 297dd38

Please sign in to comment.