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

Return user-readable drift summary in migrate dev #2002

Merged
merged 3 commits into from
Jun 9, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ start-mysql_8:

dev-mysql8: start-mysql_8
echo 'mysql8' > current_connector
cp $(CONFIG_PATH)/mysql58 $(CONFIG_FILE)
cp $(CONFIG_PATH)/mysql8 $(CONFIG_FILE)

start-mysql_mariadb:
docker-compose -f docker-compose.yml up -d --remove-orphans mariadb-10-0
Expand Down
2 changes: 1 addition & 1 deletion libs/datamodel/core/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub(crate) use renderer::Renderer;
#[derive(Debug, Clone, PartialEq)]
pub struct SchemaAst {
/// All models, enums, datasources, generators or type aliases
pub tops: Vec<Top>,
pub(crate) tops: Vec<Top>,
Jolg42 marked this conversation as resolved.
Show resolved Hide resolved
}

impl SchemaAst {
Expand Down
3 changes: 3 additions & 0 deletions migration-engine/connectors/migration-connector/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ pub trait DatabaseMigrationMarker: Debug + Send + Sync {

/// Is the migration empty?
fn is_empty(&self) -> bool;

/// Render a human-readable summary of the migrations content, for drift diagnostics.
fn summary(&self) -> String;
}

/// Shorthand for a [Result](https://doc.rust-lang.org/std/result/enum.Result.html) where the error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,22 @@ impl DatabaseMigrationMarker for MongoDbMigration {
fn is_empty(&self) -> bool {
self.steps.is_empty()
}

fn summary(&self) -> String {
let mut out = String::with_capacity(self.steps.len() * 10);

for step in &self.steps {
match step {
MongoDbMigrationStep::CreateCollection(collection) => {
out.push_str("- Added collection `");
out.push_str(collection);
out.push_str("`\n");
}
}
}

out
}
}

#[derive(Debug)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,11 @@ impl SqlMigration {
DriftType::RemovedTable => out.push_str("\n[-] Removed tables\n"),
DriftType::RemovedUdt => out.push_str("\n[-] Removed UDTs\n"),
DriftType::RemovedView => out.push_str("\n[-] Removed views\n"),
DriftType::RedefinedTable => out.push_str("\n[*] Redefined tables\n"),
DriftType::RedefinedTable => {
out.push_str("\n[*] Redefined table `");
out.push_str(item_name);
out.push_str("`\n")
}
DriftType::ChangedEnum => {
out.push_str("\n[*] Changed the `");
out.push_str(item_name);
Expand Down Expand Up @@ -373,6 +377,10 @@ impl DatabaseMigrationMarker for SqlMigration {
fn is_empty(&self) -> bool {
self.steps.is_empty()
}

fn summary(&self) -> String {
self.drift_summary()
}
}

// The order of the variants matters for sorting. The steps are sorted _first_
Expand Down
10 changes: 4 additions & 6 deletions migration-engine/core/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub trait GenericApi: Send + Sync + 'static {
) -> CoreResult<MarkMigrationRolledBackOutput>;

/// Prepare to create a migration.
async fn plan_migration(&self, input: &PlanMigrationInput) -> CoreResult<PlanMigrationOutput>;
async fn plan_migration(&self) -> CoreResult<()>;

/// Reset a database to an empty state (no data, no schema).
async fn reset(&self) -> CoreResult<()>;
Expand Down Expand Up @@ -99,7 +99,7 @@ impl<C: MigrationConnector> GenericApi for C {
&self,
input: &DiagnoseMigrationHistoryInput,
) -> CoreResult<DiagnoseMigrationHistoryOutput> {
DiagnoseMigrationHistoryCommand::execute(input, self)
diagnose_migration_history(input, self)
.instrument(tracing::info_span!("DiagnoseMigrationHistory"))
.await
}
Expand Down Expand Up @@ -149,10 +149,8 @@ impl<C: MigrationConnector> GenericApi for C {
.await
}

async fn plan_migration(&self, input: &PlanMigrationInput) -> CoreResult<PlanMigrationOutput> {
PlanMigrationCommand::execute(input, self)
.instrument(tracing::info_span!("PlanMigration"))
.await
async fn plan_migration(&self) -> CoreResult<()> {
unreachable!("PlanMigration command")
}

async fn reset(&self) -> CoreResult<()> {
Expand Down
2 changes: 1 addition & 1 deletion migration-engine/core/src/api/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl RpcApi {
}
RpcCommand::MarkMigrationApplied => render(executor.mark_migration_applied(&params.parse()?).await?),
RpcCommand::MarkMigrationRolledBack => render(executor.mark_migration_rolled_back(&params.parse()?).await?),
RpcCommand::PlanMigration => render(executor.plan_migration(&params.parse()?).await?),
RpcCommand::PlanMigration => render(executor.plan_migration().await?),
RpcCommand::Reset => render(executor.reset().await?),
RpcCommand::SchemaPush => render(executor.schema_push(&params.parse()?).await?),
})
Expand Down
8 changes: 3 additions & 5 deletions migration-engine/core/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,21 @@ mod evaluate_data_loss;
mod list_migration_directories;
mod mark_migration_applied;
mod mark_migration_rolled_back;
mod plan_migration;
mod schema_push;

pub use apply_migrations::{ApplyMigrationsCommand, ApplyMigrationsInput, ApplyMigrationsOutput};
pub use command::MigrationCommand;
pub use create_migration::{CreateMigrationCommand, CreateMigrationInput, CreateMigrationOutput};
pub(crate) use dev_diagnostic::dev_diagnostic;
pub use dev_diagnostic::{DevAction, DevDiagnosticInput, DevDiagnosticOutput};
pub use diagnose_migration_history::{
DiagnoseMigrationHistoryCommand, DiagnoseMigrationHistoryInput, DiagnoseMigrationHistoryOutput, DriftDiagnostic,
HistoryDiagnostic,
DiagnoseMigrationHistoryInput, DiagnoseMigrationHistoryOutput, DriftDiagnostic, HistoryDiagnostic,
};
pub use evaluate_data_loss::*;
pub use list_migration_directories::*;
pub use mark_migration_applied::{MarkMigrationAppliedCommand, MarkMigrationAppliedInput, MarkMigrationAppliedOutput};
pub use mark_migration_rolled_back::{MarkMigrationRolledBackInput, MarkMigrationRolledBackOutput};
pub use plan_migration::{PlanMigrationCommand, PlanMigrationInput, PlanMigrationOutput};
pub use schema_push::{SchemaPushCommand, SchemaPushInput, SchemaPushOutput};

pub(crate) use dev_diagnostic::dev_diagnostic;
pub(crate) use diagnose_migration_history::diagnose_migration_history;
pub(crate) use mark_migration_rolled_back::mark_migration_rolled_back;
17 changes: 8 additions & 9 deletions migration-engine/core/src/commands/dev_diagnostic.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::{
DiagnoseMigrationHistoryCommand, DiagnoseMigrationHistoryInput, DiagnoseMigrationHistoryOutput, DriftDiagnostic,
HistoryDiagnostic, MigrationCommand,
diagnose_migration_history, DiagnoseMigrationHistoryInput, DiagnoseMigrationHistoryOutput, DriftDiagnostic,
HistoryDiagnostic,
};
use migration_connector::{ConnectorResult, MigrationConnector};
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -33,8 +33,7 @@ pub(crate) async fn dev_diagnostic<C: MigrationConnector>(
opt_in_to_shadow_database: true,
};

let diagnose_migration_history_output =
DiagnoseMigrationHistoryCommand::execute(&diagnose_input, connector).await?;
let diagnose_migration_history_output = diagnose_migration_history(&diagnose_input, connector).await?;

check_for_broken_migrations(&diagnose_migration_history_output)?;

Expand Down Expand Up @@ -75,11 +74,11 @@ fn check_for_reset_conditions(output: &DiagnoseMigrationHistoryOutput) -> Option
))
}

if let Some(DriftDiagnostic::DriftDetected { rollback }) = &output.drift {
tracing::info!(rollback = rollback.as_str(), "DriftDetected diagnostic");

reset_reasons
.push("Drift detected: Your database schema is not in sync with your migration history.".to_owned())
if let Some(DriftDiagnostic::DriftDetected { summary }) = &output.drift {
let mut reason =
"Drift detected: Your database schema is not in sync with your migration history.\n".to_owned();
reason.push_str(summary);
reset_reasons.push(reason);
}

match &output.history {
Expand Down