-
Notifications
You must be signed in to change notification settings - Fork 132
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
feat: Alembic support #183
Conversation
The fixtures that we were autousing needed to be used explicitly, to get randomly-generated dataset names. One was only used for one test. Also, fixed copyright.
We don't require alembic and most tests should run without it, however - We run some unit tests (Python 3.8) to cover the alembic registration that happens when alembic is installed. - We have a system test that demonstrates working with alembic and proves that the things we think should work do work. :)
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.
Thanks for this. It definitely highlights we have some future opportunities to better target our bleeding and trailing edges of supported python versions with some custom testing.
""" | ||
) | ||
|
||
# The only thing we can alter about a column is we can make it |
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.
This will change, but we're not currently watching the changes for the DDL params closely. Will need to figure out how to notify on changes like this. https://cloud.google.com/bigquery/docs/release-notes is how the team signals these changes for syntax historically.
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.
Hah! They just added table rename the other day.
except ImportError: | ||
pass | ||
else: | ||
from alembic.ddl import impl |
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.
FWIW you could appease flake8 by just importing impl
above, rather than the bare import alembic
.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #65 🦕