Skip to content

Allow db_url parameter to create_database_migration_numbered_style#32

Merged
RudolfCardinal merged 2 commits intomasterfrom
improve_alembic_automigration
Mar 12, 2025
Merged

Allow db_url parameter to create_database_migration_numbered_style#32
RudolfCardinal merged 2 commits intomasterfrom
improve_alembic_automigration

Conversation

@RudolfCardinal
Copy link
Owner

Part of fixing the missing-migration-check.yml problem in ucam-department-of-psychiatry/camcops#388

@RudolfCardinal
Copy link
Owner Author

If you're happy, I'll (a) merge this and push to PyPI, (b) complete the changes to ucam-department-of-psychiatry/camcops#388 with a proper cardinal_pythonlib version number.

@martinburchell
Copy link
Collaborator

Unfortunately we've duplicated some effort here but it might that we can combine our efforts.

master...throw-exception-if-migration-fails is perhaps a little less ugly. I infer from sqlalchemy/alembic#1089 that it is OK to call the command API directly, though specifying the autogenerate flag again in the additional cmd_opts does seem a bit suboptimal.

How about we adopt my solution but keep your other cosmetic changes (though the release date in the changelog appears to have gone back in time). If we wanted to stick to calling alembic through subprocess.call() we'd need to change that to subprocess.run() with check=True otherwise any error from alembic gets hidden.

@RudolfCardinal
Copy link
Owner Author

Yours is much nicer. Yes, if we can use the API directly that is far preferable, and allows a much better db_url tweak.

@RudolfCardinal RudolfCardinal merged commit 18c6fdc into master Mar 12, 2025
5 checks passed
@RudolfCardinal RudolfCardinal deleted the improve_alembic_automigration branch March 12, 2025 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants