Fix : TypeDecorator rendering to preserve SchemaType name/schema #1795
Fix : TypeDecorator rendering to preserve SchemaType name/schema #1795khanjan2708 wants to merge 1 commit intosqlalchemy:mainfrom
Conversation
zzzeek
left a comment
There was a problem hiding this comment.
hey there
thanks for this. however I listed out the approaches in https://github.com/sqlalchemy/alembic/issues/1551#issuecomment-2420595115 as well as another one in https://github.com/sqlalchemy/alembic/issues/1551#issuecomment-2420603412
this approach seems to use another approach, which is to get the repr string and then tries to guess how to add "name" into it, which I'd rather not do this this way. The core issue is TypeDecorator.repr is broken and item 3 in my comment at https://github.com/sqlalchemy/alembic/issues/1551#issuecomment-2420595115 is the best way to approach this.
that is, we should be changing TypeDecorator.__repr__ first and foremost. if we want to put an interim fix on the alembic side, then it should not use the TypeDecorator.__repr__ output at all and should re-implement from scratch.
most effective would be my item 3 change, which would place this on the SQLAlchemy side. ill propose that now
|
I'm going to fix this on the SQLAlchemy side and we no longer need this fix on the alembic side. any chance you can address the test failures on your other PR at #1793 ? that PR is stalled as it does not pass tests |
Working on it too |
|
hi - the sqlalchemy fix is merged, closing this |
Fixes Issue : sqlalchemy/sqlalchemy#13140
Description
This pull request fixes Issue sqlalchemy/sqlalchemy#13140, where
sqlalchemy.Enum(and otherSchemaTypeslikeBoolean) lost theirnameand schema properties when augmented with asqlalchemy.types.TypeDecoratorduring autogenerated migrations.The Fix: I have introduced a
_type_reprhelper inalembic/autogenerate/render.pythat specifically detectsTypeDecoratorinstances. If the implementation of the decorator is aSchemaType, Alembic now manually ensures that thenameandschemaattributes are preserved in the rendered migration string.Additionally,
_user_autogenerate_prefixwas updated to use.get()instead of direct dictionary access to preventKeyErrorregressions in certain testing environments where the full autogenerate context might not be initialized.Checklist
This pull request is:
must include a complete example of the issue. one line code fixes without an
issue and demonstration will not be accepted.
Fixes: #<issue number>in the commit messageinclude a complete example of how the feature would look.
Fixes: #<issue number>in the commit messageHave a nice day!