Skip to content

Commit 72cea4b

Browse files
sbosiosbosio
committed
Re-enabling config option for non-active migration source/destination dbs
1 parent 03e9467 commit 72cea4b

2 files changed

Lines changed: 37 additions & 27 deletions

File tree

‎src/commands/data/pg/migrate.ts‎

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
// ExtendedPostgresLevelInfo,
1616
InfoResponse,
1717
MigrationResponse,
18+
MigrationStatus,
1819
} from '../../../lib/data/types.js'
1920
// import {fetchLevelsAndPricing} from '../../../lib/data/utils.js'
2021
import {getAttachmentNamesByAddon} from '../../../lib/pg/util.js'
@@ -92,9 +93,9 @@ export default class DataPgMigrate extends BaseCommand {
9293
case '__select_migration': {
9394
const choices: Array<DistinctChoice<{ migration: string }, ListChoiceMap<{ migration: string }>>> = []
9495
for (const migration of readyMigrations) {
95-
const sourceDatabase = this.classicDatabases.find(db => db.id === migration.source_id)!
96-
const targetDatabase = this.advancedDatabases.find(db => db.id === migration.target_id)!
97-
const name = `From ${color.datastore(sourceDatabase.name)} to ${color.datastore(targetDatabase.name)}`
96+
const sourceDatabase = this.classicDatabases.find(db => db.id === migration.source_id)
97+
const targetDatabase = this.advancedDatabases.find(db => db.id === migration.target_id)
98+
const name = `From ${color.datastore(sourceDatabase?.name ?? color.dim('unknown'))} to ${color.datastore(targetDatabase?.name ?? color.dim('unknown'))}`
9899
choices.push({
99100
name,
100101
value: migration.id,
@@ -120,25 +121,24 @@ export default class DataPgMigrate extends BaseCommand {
120121

121122
case '__confirm_action': {
122123
const selectedMigration = readyMigrations.find(migration => migration.id === selectedMigrationId)!
123-
const sourceDatabase = this.classicDatabases.find(db => db.id === selectedMigration.source_id)!
124-
const targetDatabase = this.advancedDatabases.find(db => db.id === selectedMigration.target_id)!
124+
const sourceDatabase = this.classicDatabases.find(db => db.id === selectedMigration.source_id)
125+
const targetDatabase = this.advancedDatabases.find(db => db.id === selectedMigration.target_id)
125126

126127
if (migrationAction === 'start') {
127128
ux.stdout(color.info(heredoc`
128129
129-
Your database ${color.datastore(sourceDatabase.name)} will be unavailable after starting the migration until the migration is complete.
130+
Your database ${color.datastore(sourceDatabase?.name ?? color.dim('unknown'))} will be unavailable after starting the migration until the migration is complete.
130131
If there are any issues during the migration, we end the migration and make the source database available again.
131-
The database ${color.datastore(sourceDatabase.name)} can be offline for several hours during the migration.
132+
The database ${color.datastore(sourceDatabase?.name ?? color.dim('unknown'))} can be offline for several hours during the migration.
132133
You'll receive an email when the migration is complete.
133134
You can't cancel the migration after starting it.
134135
135136
`))
136137
} else {
137138
ux.stdout(color.info(heredoc`
138139
139-
After cancelling, you'll have to manually remove the destination database (${color.datastore(targetDatabase.name)}), recreate the migration configuration
140-
and wait for the migration tooling to be prepared again in order to migrate ${color.datastore(sourceDatabase.name)}.
141-
Run ${color.code(`heroku data:pg:destroy ${targetDatabase.name} -a ${targetDatabase.app.name}`)} to remove the destination database if you don't need it anymore.
140+
After cancelling, you'll have to create a new migration configuration and wait for the migration tooling to be prepared in order to
141+
migrate ${color.datastore(sourceDatabase?.name ?? color.dim('unknown'))} again.
142142
143143
`))
144144
}
@@ -156,7 +156,7 @@ export default class DataPgMigrate extends BaseCommand {
156156
currentStep = '__select_migration'
157157
} else if (action === '__confirm') {
158158
ux.stdout()
159-
ux.action.start(`${migrationAction === 'start' ? 'Starting' : 'Cancelling'} migration of ${color.datastore(sourceDatabase.name)} to ${color.datastore(targetDatabase.name)}`)
159+
ux.action.start(`${migrationAction === 'start' ? 'Starting' : 'Cancelling'} migration of ${color.datastore(sourceDatabase?.name ?? color.dim('unknown'))} to ${color.datastore(targetDatabase?.name ?? color.dim('unknown'))}`)
160160
await this.dataApi.post(`/data/postgres/v1/${selectedMigration.target_id}/migrations/${migrationAction === 'start' ? 'run' : 'cancel'}`)
161161
ux.action.stop()
162162
currentStep = '__exit'
@@ -179,9 +179,9 @@ export default class DataPgMigrate extends BaseCommand {
179179
const choices: Array<DistinctChoice<{ database: string }, ListChoiceMap<{ database: string }>>> = []
180180
for (const database of this.classicDatabases) {
181181
const name = `${color.datastore(database.name)} as ${database.attachment_names!.map(name => color.attachment(name)).join(', ')}`
182-
if (this.migrationTargets.some(migration => migration.source_id === database.id)) {
182+
if (this.migrationTargets.some(migration => migration.source_id === database.id && this.isActiveMigration(migration))) {
183183
choices.push({
184-
disabled: 'already a migration source',
184+
disabled: 'already a source database for an active migration',
185185
name: color.dim(name),
186186
value: database.id,
187187
})
@@ -214,9 +214,9 @@ export default class DataPgMigrate extends BaseCommand {
214214
const choices: Array<DistinctChoice<{ database: string }, ListChoiceMap<{ database: string }>>> = []
215215
for (const database of this.advancedDatabases) {
216216
const name = `${color.datastore(database.name)} as ${database.attachment_names!.map(name => color.attachment(name)).join(', ')}`
217-
if (this.migrationTargets.some(migration => migration.target_id === database.id)) {
217+
if (this.migrationTargets.some(migration => migration.target_id === database.id && this.isActiveMigration(migration))) {
218218
choices.push({
219-
disabled: 'already a migration destination',
219+
disabled: 'already a destination database for an active migration',
220220
name: color.dim(name),
221221
value: database.id,
222222
})
@@ -227,7 +227,7 @@ export default class DataPgMigrate extends BaseCommand {
227227
})
228228
} else {
229229
choices.push({
230-
disabled: 'database isn\'t available yet',
230+
disabled: 'database isn\'t available',
231231
name: color.dim(name),
232232
value: database.id,
233233
})
@@ -237,7 +237,7 @@ export default class DataPgMigrate extends BaseCommand {
237237
if (this.advancedDatabases.length === 0) {
238238
choices.push({
239239
disabled: true,
240-
name: color.dim(`No Heroku Postgres Advanced databases available on ${color.app(this.appName!)}`),
240+
name: color.dim(`No Heroku Postgres Advanced databases available for migration on ${color.app(this.appName!)}`),
241241
value: '__no_advanced_databases',
242242
})
243243
}
@@ -412,7 +412,10 @@ export default class DataPgMigrate extends BaseCommand {
412412

413413
for (const infoResult of infoResults) {
414414
if (infoResult.status === 'fulfilled') {
415-
this.advancedDatabases.find(db => db.id === infoResult.value.body.addon.id)!.info = infoResult.value.body
415+
const db = this.advancedDatabases.find(db => db.id === infoResult.value.body.addon.id)
416+
if (db) {
417+
db.info = infoResult.value.body
418+
}
416419
}
417420
}
418421

@@ -421,13 +424,20 @@ export default class DataPgMigrate extends BaseCommand {
421424
.map(queryResult => (queryResult as PromiseFulfilledResult<HTTP<MigrationResponse>>).value.body)
422425
}
423426

427+
private isActiveMigration(migration: MigrationResponse): boolean {
428+
return migration.status === MigrationStatus.PREPARING
429+
|| migration.status === MigrationStatus.MIGRATING
430+
|| migration.status === MigrationStatus.PROMOTING
431+
|| migration.status === MigrationStatus.READY
432+
}
433+
424434
private async loopMainMenu(app: string): Promise<string> {
425435
// Update our database lists
426436
await this.getAppDatabases(app)
427437
await this.getMigrationTargetsAndInfo()
428438

429439
const pendingMigrations = this.classicDatabases.filter(
430-
db => !this.migrationTargets.some(migration => migration.source_id === db.id),
440+
db => !this.migrationTargets.some(migration => migration.source_id === db.id && this.isActiveMigration(migration)),
431441
)
432442

433443
hux.styledHeader('Configured migrations')
@@ -436,11 +446,11 @@ export default class DataPgMigrate extends BaseCommand {
436446
/* eslint-disable perfectionist/sort-objects */
437447
hux.table(this.migrationTargets, {
438448
source: {
439-
get: (migration: MigrationResponse) => color.datastore(this.classicDatabases.find(db => db.id === migration.source_id)!.name),
449+
get: (migration: MigrationResponse) => color.datastore(this.classicDatabases.find(db => db.id === migration.source_id)?.name ?? color.dim('unknown')),
440450
header: 'Source Database',
441451
},
442452
destination: {
443-
get: (migration: MigrationResponse) => color.datastore(this.advancedDatabases.find(db => db.id === migration.target_id)!.name),
453+
get: (migration: MigrationResponse) => color.datastore(this.advancedDatabases.find(db => db.id === migration.target_id)?.name ?? color.dim('unknown')),
444454
header: 'Destination Database',
445455
},
446456
status: {

‎test/unit/commands/data/pg/migrate.unit.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -390,10 +390,10 @@ describe('data:pg:migrate', function () {
390390
const sourceDatabaseList = stdout.output.match(/(?<=Select the source database: \(Use arrow keys\)\n)(.*?)(?=Go back)/s)?.[1]
391391
expect(stderr.output).to.equal('Configuring migration... done\n')
392392
// Entry for the source database that is already a migration source should be disabled
393-
expect(sourceDatabaseList).to.contain(`⛁ ${standardDbAttachment.addon.name} as STANDARD_DB (already a migration source)`)
393+
expect(sourceDatabaseList).to.contain(`⛁ ${standardDbAttachment.addon.name} as STANDARD_DB (already a source database for an active migration)`)
394394
// Entry for the Premium database should be enabled
395395
expect(sourceDatabaseList).to.contain(`⛁ ${premiumDbAttachment.addon.name} as PREMIUM_DB`)
396-
expect(sourceDatabaseList).not.to.contain(`⛁ ${premiumDbAttachment.addon.name} as PREMIUM_DB (already a migration source)`)
396+
expect(sourceDatabaseList).not.to.contain(`⛁ ${premiumDbAttachment.addon.name} as PREMIUM_DB (already a source database for an active migration)`)
397397
// There should be no entry for the Essential database
398398
expect(sourceDatabaseList).not.to.contain(essentialDbAttachment.addon.name)
399399
// There should be no entry for the Foreign standard database
@@ -421,12 +421,12 @@ describe('data:pg:migrate', function () {
421421
const targetDatabaseList = stdout.output.match(/(?<=Select the destination database: \(Use arrow keys\)\n)(.*?)(?=Go back)/s)?.[1]
422422
expect(stderr.output).to.equal('Configuring migration... done\n')
423423
// Entry for the target database that is already a migration destination should be disabled
424-
expect(targetDatabaseList).to.contain(`⛁ ${targetAdvancedDbAttachment.addon.name} as ADVANCED_DB (already a migration destination)`)
424+
expect(targetDatabaseList).to.contain(`⛁ ${targetAdvancedDbAttachment.addon.name} as ADVANCED_DB (already a destination database for an active migration)`)
425425
// Entry for the non-target Advanced database should be enabled
426426
expect(targetDatabaseList).to.contain(`⛁ ${nonTargetAdvancedDbAttachment.addon.name} as OTHER_ADVANCED_DB`)
427-
expect(targetDatabaseList).not.to.contain(`⛁ ${nonTargetAdvancedDbAttachment.addon.name} as OTHER_ADVANCED_DB (already a migration destination)`)
427+
expect(targetDatabaseList).not.to.contain(`⛁ ${nonTargetAdvancedDbAttachment.addon.name} as OTHER_ADVANCED_DB (already a destination database for an active migration)`)
428428
// Entry for the unavailable database should be disabled
429-
expect(targetDatabaseList).to.contain(`⛁ ${unavailableAdvancedDbAttachment.addon.name} as UNAVAILABLE_DB (database isn't available yet)`)
429+
expect(targetDatabaseList).to.contain(`⛁ ${unavailableAdvancedDbAttachment.addon.name} as UNAVAILABLE_DB (database isn't available)`)
430430
// There should be no entries for non-Advanced or foreign databases
431431
expect(targetDatabaseList).not.to.contain(essentialDbAttachment.addon.name)
432432
expect(targetDatabaseList).not.to.contain(foreignAdvancedDbAttachment.addon.name)
@@ -822,7 +822,7 @@ describe('data:pg:migrate', function () {
822822
await runCommand(DataPgMigrate, ['--app=myapp'])
823823

824824
// Verify the confirmation message is displayed
825-
expect(stdout.output).to.contain('After cancelling, you\'ll have to manually remove the destination database (⛁ postgresql-lively-12345)')
825+
expect(stdout.output).to.contain('After cancelling, you\'ll have to create a new migration configuration')
826826
expect(stderr.output).to.equal('Cancelling migration of ⛁ postgresql-cubic-12345 to ⛁ postgresql-lively-12345... done\n')
827827
})
828828

0 commit comments

Comments
 (0)