fix: honor --dry-run flag in migration commands (fixes #133) #135
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The --dry-run flag was being accepted but ignored, causing migrations to actually modify the database when users expected no changes. Changes: - Add dry_run parameter to upgrade() and downgrade() methods in both SyncMigrationCommands and AsyncMigrationCommands - Skip database operations (execute_upgrade/downgrade and version table updates) when dry_run=True - Add prominent "DRY RUN MODE" banner to console output - Show "Would apply/revert" instead of "Applying/Reverting" in dry-run - Display migration file paths during dry-run - Add final confirmation message when dry-run completes - Update CLI to pass dry_run flag to migration commands Testing: - Linting: ruff check passed - Type checking: pyright 0 errors Fixes #133
cofin
force-pushed
the
fix/issue-133-dry-run-bypass
branch
from
38b3679 to
90706c4
Compare
cofin
added a commit
that referenced
this pull request
Oct 20, 2025
Adds automatic CLOB-to-string conversion in Oracle adapter to enable seamless msgspec/Pydantic integration. CLOB handles are now read automatically before schema conversion, eliminating the need for DBMS_LOB.SUBSTR workarounds. ## Summary Oracle's oracledb driver returns CLOB columns as LOB/AsyncLOB objects with .read() methods. This causes msgspec validation to fail with "Expected 'str', got 'AsyncLOB'" errors. Users previously worked around this by wrapping CLOB columns in DBMS_LOB.SUBSTR, truncating to 4000 chars and duplicating boilerplate. ## The Solution Automatically coerce LOB handles to concrete values during result processing: - Added _coerce_sync_row_values() and _coerce_async_row_values() helpers - Integrated into both sync and async _execute_statement() methods - Reads CLOB handles before dict construction - Applies JSON detection for JSON-in-CLOB scenarios - BLOB handling unchanged (remains bytes) ## Key Features - Automatic CLOB hydration in sync driver (line 547) - Automatic CLOB hydration in async driver (lines 740-743) - JSON detection via OracleTypeConverter.convert_if_detected() - Error handling for LOB read failures - Zero configuration required ## Testing Created comprehensive msgspec integration test suite: - tests/integration/test_adapters/test_oracledb/test_msgspec_clob.py (528 lines) - 12 tests covering: basic hydration, mixed types, JSON detection, BLOB, multi-CLOB, NULL - All 158 Oracle tests passing (146 existing + 12 new) ## Documentation - Updated docs/guides/adapters/oracle.md with CLOB/BLOB handling section - Added LOB Hydration Pattern to AGENTS.md for future reference - Includes before/after examples showing eliminated workarounds Closes #135
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes the critical data safety bug where the
--dry-runflag was accepted but completely ignored, causing migrations to actually modify the database when users expected no changes to occur.Closes #133
Problem
The
--dry-runflag was defined in the CLI commands but never passed through to the migration execution functions. This meant that:sqlspec upgrade --dry-runwould have their database actually modifiedsqlspec downgrade --dry-runwould have migrations actually revertedThis is a critical data safety issue.
Solution
Changes Made
Migration Commands (sqlspec/migrations/commands.py)
dry_run: bool = Falseparameter toupgrade()anddowngrade()methodsdry_run=True:runner.execute_upgrade()/runner.execute_downgrade()tracker.record_migration()/tracker.remove_migration()SyncMigrationCommandsandAsyncMigrationCommandsCLI Commands (sqlspec/cli.py)
dry_run=dry_runparameter:Example Output
Before (BUG):
```
$ sqlspec downgrade --dry-run
Are you sure you want to downgrade the database to the
-1revision? [y/n]: yReverting 1 migrations
Reverting 0002: oauth
✓ Reverted in 12ms
$ sqlspec show-current-revision
Current version: 0001 # DATABASE WAS MODIFIED!
```
After (FIXED):
```
$ sqlspec downgrade --dry-run
DRY RUN MODE: No database changes will be applied
Are you sure you want to downgrade the database to the
-1revision? [y/n]: yReverting 1 migrations
Would revert 0002: oauth
Migration file: /path/to/migrations/0002_oauth.sql
Dry run complete. No changes were made to the database.
$ sqlspec show-current-revision
Current version: 0002 # DATABASE UNCHANGED
```
Checklist