Bugfix: Fix migration crash when using MSSQL as mage database #4695
Conversation
…nstraint drop correctly with mssql by using alter command.
dy46
added
enhancement
| unique_constraints = insp.get_unique_constraints('secret') | ||
| for constraint in unique_constraints: | ||
| batch_op.drop_constraint(constraint['name'], type_='unique') | ||
| insp = sa.inspect(bind) |
There was a problem hiding this comment.
Changing this line affects the insp in the else block as well.
There was a problem hiding this comment.
Yes, but it did not affect the results when I tested with different sql servers.
MSSQL server inspection did not work if I had the original line sa.inspect(bind.engine). All inspections calls freeze the migration if bind.engine is used with MSSQL server. I read the documentation the inspect method does accept either an engine or a connection which the bind variable is https://docs.sqlalchemy.org/en/20/core/inspection.html#available-inspection-targets .
There was a problem hiding this comment.
Here is also documentation about the get_bind() function that states that it returns a Connection object when "online" mode https://alembic.sqlalchemy.org/en/latest/api/runtime.html#alembic.runtime.environment.EnvironmentContext.get_bind
…nstraint drop correctly with mssql by using alter command. (mage-ai#4695)
Description
This is a fix for issue #4681 where migration file 90d978a8aef8_update_unique_constraint_for_secret.py crashed due to alembic not supporting constraint retrieval for MSSQL as constraint are implemented as indexes that need to fetched using get_indexes() and dropped via alter table sql command.
How Has This Been Tested?
I verified the changes by setting mage.ai to use sql server as the database and also tested with postgres to make sure changes didn't cause side effects to existing migration code.
Checklist
docs/mint.jsoncc:
@dy46