-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set type and name not null #87
base: master
Are you sure you want to change the base?
Conversation
jesusbv
commented
Sep 6, 2021
- Update Server properties type and name to not allow null values
- Update Server properties type and name to not allow null values
depends_on = None | ||
|
||
|
||
def upgrade(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could potentially clean this up a bit by defining the ENUM once instead of using duplicate definitions here in the op.alter_column() statements.
The auto-generated code is a little dumb in this regard. ;-)
I'm sure that under the hood SQLAlchemy/PostgreSQL are detecting equivalency but it would probably make the code look cleaner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I know what you mean, and I have done some refactoring in other PRs but, as a rule thumb, if the auto-generated code by alembic does not need any change, leave like that (i.e. IPv6 PR)
# ### end Alembic commands ### | ||
|
||
|
||
def downgrade(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above; we can define the ENUM once, and reference it in all the statements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general the changes are good.
However the auto-generated code has a lot of unnecessary repetition, and a little cleanup would code eliminate that and make it more readable.