Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add back confirmation handling and output formatting in verify and push #10330

Merged
merged 1 commit into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/db/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
"open": "^10.0.3",
"ora": "^7.0.1",
"prompts": "^2.4.2",
"strip-ansi": "^7.1.0",
"yargs-parser": "^21.1.1",
"zod": "^3.22.4"
},
Expand Down
12 changes: 11 additions & 1 deletion packages/db/src/core/cli/commands/push/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import { getRemoteDatabaseUrl } from '../../../utils.js';
import {
createCurrentSnapshot,
createEmptySnapshot,
formatDataLossMessage,
getMigrationQueries,
getProductionCurrentSnapshot,
} from '../../migration-queries.js';
import { red } from 'kleur/colors';

export async function cmd({
dbConfig,
Expand All @@ -24,7 +26,7 @@ export async function cmd({
const productionSnapshot = await getProductionCurrentSnapshot({ appToken: appToken.token });
const currentSnapshot = createCurrentSnapshot(dbConfig);
const isFromScratch = isForceReset || JSON.stringify(productionSnapshot) === '{}';
const { queries: migrationQueries } = await getMigrationQueries({
const { queries: migrationQueries, confirmations } = await getMigrationQueries({
oldSnapshot: isFromScratch ? createEmptySnapshot() : productionSnapshot,
newSnapshot: currentSnapshot,
});
Expand All @@ -35,6 +37,14 @@ export async function cmd({
} else {
console.log(`Database schema is out of date.`);
}

if (isForceReset) {
console.log(`Force-pushing to the database. All existing data will be erased.`);
} else if (confirmations.length > 0) {
console.log('\n' + formatDataLossMessage(confirmations) + '\n');
throw new Error('Exiting.');
}

if (isDryRun) {
console.log('Statements:', JSON.stringify(migrationQueries, undefined, 2));
} else {
Expand Down
27 changes: 23 additions & 4 deletions packages/db/src/core/cli/commands/verify/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { DBConfig } from '../../../types.js';
import {
createCurrentSnapshot,
createEmptySnapshot,
formatDataLossMessage,
getMigrationQueries,
getProductionCurrentSnapshot,
} from '../../migration-queries.js';
Expand All @@ -17,21 +18,39 @@ export async function cmd({
dbConfig: DBConfig;
flags: Arguments;
}) {
const isJson = flags.json;
const appToken = await getManagedAppTokenOrExit(flags.token);
const productionSnapshot = await getProductionCurrentSnapshot({ appToken: appToken.token });
const currentSnapshot = createCurrentSnapshot(dbConfig);
const { queries: migrationQueries } = await getMigrationQueries({
const { queries: migrationQueries, confirmations } = await getMigrationQueries({
oldSnapshot:
JSON.stringify(productionSnapshot) !== '{}' ? productionSnapshot : createEmptySnapshot(),
newSnapshot: currentSnapshot,
});

const result = { exitCode: 0, message: '', code: '', data: undefined as unknown };
if (migrationQueries.length === 0) {
console.log(`Database schema is up to date.`);
result.code = 'MATCH';
result.message = `Database schema is up to date.`;
} else {
console.log(`Database schema is out of date.`);
console.log(`Run 'astro db push' to push up your latest changes.`);
result.code = 'NO_MATCH';
result.message = `Database schema is out of date.\nRun 'astro db push' to push up your latest changes.`;
}


if (confirmations.length > 0) {
result.code = 'DATA_LOSS';
result.exitCode = 1;
result.data = confirmations;
result.message = formatDataLossMessage(confirmations, !isJson);
}

if (isJson) {
console.log(JSON.stringify(result));
} else {
console.log(result.message);
}

await appToken.destroy();
process.exit(result.exitCode);
}
38 changes: 21 additions & 17 deletions packages/db/src/core/cli/migration-queries.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import stripAnsi from 'strip-ansi';
import deepDiff from 'deep-diff';
import { SQLiteAsyncDialect } from 'drizzle-orm/sqlite-core';
import * as color from 'kleur/colors';
Expand Down Expand Up @@ -147,21 +148,12 @@ export async function getCollectionChangeQueries({
if (dataLossCheck.dataLoss) {
const { reason, columnName } = dataLossCheck;
const reasonMsgs: Record<DataLossReason, string> = {
'added-required': `New column ${color.bold(
'added-required': `You added new required column '${color.bold(
collectionName + '.' + columnName
)} is required with no default value.\nThis requires deleting existing data in the ${color.bold(
collectionName
)} collection.`,
'added-unique': `New column ${color.bold(
)}' with no default value.\n This cannot be executed on an existing table.`,
'updated-type': `Updating existing column ${color.bold(
collectionName + '.' + columnName
)} is marked as unique.\nThis requires deleting existing data in the ${color.bold(
collectionName
)} collection.`,
'updated-type': `Updated column ${color.bold(
collectionName + '.' + columnName
)} cannot convert data to new column data type.\nThis requires deleting existing data in the ${color.bold(
collectionName
)} collection.`,
)} to a new type that cannot be handled automatically.`,
};
confirmations.push(reasonMsgs[reason]);
}
Expand Down Expand Up @@ -319,7 +311,7 @@ function canAlterTableDropColumn(column: DBColumn) {
return true;
}

type DataLossReason = 'added-required' | 'added-unique' | 'updated-type';
type DataLossReason = 'added-required' | 'updated-type';
type DataLossResponse =
| { dataLoss: false }
| { dataLoss: true; columnName: string; reason: DataLossReason };
Expand All @@ -335,9 +327,6 @@ function canRecreateTableWithoutDataLoss(
if (!a.schema.optional && !hasDefault(a)) {
return { dataLoss: true, columnName, reason: 'added-required' };
}
if (!a.schema.optional && a.schema.unique) {
return { dataLoss: true, columnName, reason: 'added-unique' };
}
}
for (const [columnName, u] of Object.entries(updated)) {
if (u.old.type !== u.new.type && !canChangeTypeWithoutQuery(u.old, u.new)) {
Expand Down Expand Up @@ -454,3 +443,18 @@ export function createCurrentSnapshot({ tables = {} }: DBConfig): DBSnapshot {
export function createEmptySnapshot(): DBSnapshot {
return { experimentalVersion: 1, schema: {} };
}

export function formatDataLossMessage(confirmations: string[], isColor = true): string {
const messages = [];
messages.push(color.red('✖ We found some schema changes that cannot be handled automatically:'));
messages.push(``);
messages.push(...confirmations.map((m, i) => color.red(` (${i + 1}) `) + m));
messages.push(``);
messages.push(`To resolve, revert these changes or update your schema, and re-run the command.`);
messages.push(`You may also run 'astro db push --force-reset' to ignore all warnings and force-push your local database schema to production instead. All data will be lost and the database will be reset.`);
let finalMessage = messages.join('\n');
if (!isColor) {
finalMessage = stripAnsi(finalMessage);
}
return finalMessage;
}
2 changes: 2 additions & 0 deletions packages/db/test/fixtures/ticketing-example/db/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ const Event = defineTable({
ticketPrice: column.number(),
date: column.date(),
location: column.text(),
author3: column.text(),
author4: column.text(),
},
});

Expand Down
3 changes: 3 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading