Switch to column keyword arguments#161
Conversation
jklukas
left a comment
There was a problem hiding this comment.
I haven't deeply absorbed the 1.3.0 changes yet, so may have more comments, but this looks reasonable at first read-through.
| pass | ||
| else: | ||
| from alembic.ddl.base import RenameTable | ||
|
|
There was a problem hiding this comment.
Can you revert whitespace changes or refactor them to a separate PR?
There was a problem hiding this comment.
I can, but for some reason this particular whitespace was causing travis build to fail
| ... sa.Column('id', sa.Integer, primary_key=True), | ||
| ... sa.Column( | ||
| ... 'name', sa.String, info={'distkey': True, 'sortkey': True} | ||
| ... 'name', sa.String, |
There was a problem hiding this comment.
Can we either split these to separate lines, or roll the last two options onto this same line for better consistency?
|
@ms32035 - If you make the whitespace and docs changes here as commented, I'll kick off CI that hits Redshift and we should be good to merge if it's all passing. |
Co-Authored-By: ms32035 <ms32035@gmail.com>
Co-Authored-By: ms32035 <ms32035@gmail.com>
|
looks great, feel free to add me as reviewers if you have another one of these in the future |
infoto dialect specific kwargs, which are now supported in SQLAlchemy>=1.3.0, as per recommendation from the author: https://bitbucket.org/zzzeek/alembic/pull-requests/86/add-support-for-column-info/diffinfosupport for SQLAlchemy < 1.3.0 and minimal SQLAlchemy version retainedTodos