Repository navigation
Add runner options, SQL preview, native locking and deployment tooling - #177
jogibear9988 merged 10 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Final review comments identify one critical logging issue and several moderate correctness and tooling issues.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (4)
What changed in this PR
Adds configurable migration execution, SQL preview, native database locks, optional DI/logging integration, and a packaged .NET CLI.
Changes:
- Adds filtering, profiles, lifecycle stages, transaction modes, and lock leases.
- Adds connected/offline SQL preview and native session locks.
- Adds DI tooling, CLI commands, packaging, and test coverage.
| File | Reviewed changes | Review notes |
|---|---|---|
src/Migrator/RunnerOptions.cs |
Runner configuration APIs | No final comment. |
src/Migrator/Migrator.cs |
Orchestration and preview integration | Moderate: preserve the initial history snapshot and initialize preview migrations consistently. |
src/Migrator/MigrationSqlPreview.cs |
SQL capture and generation | Moderate: run initialization and reverse validation during preview. |
src/Migrator/MigrationLoader.cs |
Migration discovery and activation | Moderate: do not filter discovery or duplicate checks by provider scope. |
src/Migrator/MigrationExecution.cs |
Transactions and callbacks | No final comment. |
src/Migrator/DatabaseMigrationLock.cs |
Native session locks | No final comment. |
src/Migrator.Tool/Program.cs |
CLI commands and connection setup | Moderate: narrow exception handling and register provider factories. |
src/Migrator.Tool/DotNetProjects.Migrator.Tool.csproj |
CLI packaging and providers | No final comment. |
src/Migrator.Tests/ToolingTests.cs |
Tooling coverage | No final comment. |
src/Migrator.Tests/RunnerFeatureTests.cs |
Runner and preview coverage | No final comment. |
src/Migrator.Tests/Migrator.Tests.csproj |
Test project references | No final comment. |
src/Migrator.Tests/DatabaseLockTests.cs |
Database lock coverage | No final comment. |
src/Migrator.Extensions.DependencyInjection/ServiceCollectionExtensions.cs |
DI registration | No final comment. |
src/Migrator.Extensions.DependencyInjection/MigrationLogger.cs |
Microsoft logging integration | Critical: prevent raw SQL and exception text from being logged verbatim. |
src/Migrator.Extensions.DependencyInjection/DotNetProjects.Migrator.Extensions.DependencyInjection.csproj |
Optional DI package | No final comment. |
Migrator.slnx |
Solution project registration | No final comment. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical rollback behavior and additional logging, locking, preview, and CLI correctness issues block safe approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (5)
Introduce additive runner options for tag any/all matching, ordered unversioned profiles, maintenance stages, activation and lock leases. Acquire locks before history refresh and keep version history isolated from auxiliary migrations. Preserve per-migration defaults; add no-transaction and verified-provider whole-session execution with callbacks deferred until commit. Validation: rebuilt solution; SQLite 156 tests and Unit 73 tests passed. Behavioral coverage checks rollback boundaries, post-commit ordering, profile history, tag matching and lock release. Native lock implementations and tooling follow separately.
Expose offline and connected SQL generation with opt-in legacy migration capture and explicit rejection of connection access. Add session-owned SQL Server, PostgreSQL and MySQL/MariaDB locks with timeout and release leases. Package a .NET tool for list, status, validation, migrate, rollback, plan and SQL output. Keep Microsoft dependency injection/options/logging dependencies in an optional package and omit sensitive exception/SQL details in tooling output. Validation: solution rebuilt; Unit 77 and SQLite 159 passed. Added DI activation and CLI argument/list checks, legacy preview opt-in/read-only checks, and live independent-session locking tests for the database CI matrix.
A locally installed CLI could list and generate offline SQL but failed to connect because the core factory fallback selected its historical driver assembly. Register each bundled driver's factory explicitly before creating a provider. Validation: rebuilt solution; connected SQLite CLI migrate/status/rollback smoke passed. Added a CLI regression with a disposable file database that verifies version history and final table removal; Unit 83 and SQLite 161 passed.
Omit free-form provider Log/Warn text so SQL and secrets cannot escape through the optional logging adapter. Distinguish CLI parser and known capability/lock errors from arbitrary migration-body failures. Reject InitializeOnce overrides in preview rather than executing initialization hooks. Pass independent initial-history snapshots to Started/Finished logging. Validation: rebuilt solution; Unit 86 and SQLite 167 passed. Added regressions for three migration exception types returning execution exit code 1, secret/brace logging, initialization-dependent preview rejection and stable lifecycle history arguments. Addresses all four initial PR 177 review findings.
Release the lock in a finally block and retain a secondary release exception in the original failure Data. A release failure after successful execution still propagates. Add a regression covering a failed migration and failed lease disposal together. Validation: solution build; Unit 89 passed; SQLite 172 passed.
Add an additive RollbackTo entry point that validates the refreshed execution plan after acquiring the deployment lock. Route the CLI rollback command through it so an empty database and a higher target cannot execute Up migrations. Validation: solution build, Unit 89 passed, SQLite 173 passed; the new connected CLI regression verifies no user table or version is applied. Addresses review 4072146578.
The documentation PR reproduced an Informix registry timeout followed by a successful retry, but coverage downloaded an empty startup-only artifact left by the first attempt. Enable upload-artifact overwrite for each uniquely named database suite so the gate consumes the current attempt's test results. Validation: inspected run 35735397572 attempt 2: all database jobs passed, while duplicate test-results-Informix artifacts (277 and 16046 bytes) caused the missing-suite failure. Fresh CI validates the complete artifact flow.
… locks Add a two-connection runner regression on SQL Server, PostgreSQL, MySQL and MariaDB. Seed the second runner with stale empty history, hold the first inside its migration, start the second acquisition, and verify only one Up executes after history reload. Confirm the final version and that the lock can be acquired again. Coordination uses events instead of timing sleeps. Validation: solution build and Unit 96 passed. The new concurrency cases require their four live provider CI jobs before claiming coverage.
Even when one worker faults, await the remaining worker before dropping the temporary history table or disposing its connection. This keeps the failure path of the concurrency regression deterministic.
Retain the already-validated callback CurrentMigration context, auxiliary-only history preservation and regression tests when replaying the tooling branch on the updated runner/provider bases. These fixes had equivalent patches earlier in the old stack; rebase patch deduplication otherwise omitted their final tooling integration. Preserve the original file encodings and matrix settings. The resulting source tree matches the previously tested bc35e0e exactly; only the comparison and homepage additions already merged on master are new here.
bc35e0e to
9804fef
Compare


Adds runner options for tag any/all matching, ordered unversioned profiles, maintenance stages, no-transaction/whole-session modes, activation and optional lock leases. Whole-session callbacks run only after commit; unsupported DDL providers fail explicitly. Native session locks cover SQL Server, PostgreSQL and MySQL/MariaDB.
Adds connected/offline structured SQL preview and opt-in legacy capture. Direct connection access and unsupported provider methods are blocked; arbitrary migration C# remains trusted code. Preview currently rejects unsupported operation families rather than claiming complete SQL coverage.
Includes an optional Microsoft DI/options/logging package and a packaged .NET CLI for list/status, plan validation, migrate/rollback, SQL generation and script output. CLI connections come from environment variables and diagnostic output omits raw SQL and exception messages.
Validation: rebuilt solution and ran Unit/SQLite suites. Behavioral regressions cover filtering, lifecycle ordering, transaction failure boundaries, DI activation, preview opt-in and blocked connection access. Independent-session native lock tests run in the live database matrix. This PR is stacked on #175; do not merge before its dependencies.