Add database schema update and database migration logic #520
Conversation
andrii-i
force-pushed
the
db_migration
branch
2 times, most recently
from
7065131
to
37e7682
Compare
andrii-i
changed the title
andrii-i
changed the title
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
| @@ -104,6 +105,7 @@ class Job(CommonColumns, Base): | |||
|
|
|||
| class JobDefinition(CommonColumns, Base): | |||
| __tablename__ = "job_definitions" | |||
| __table_args__ = {"extend_existing": True} | |||
There was a problem hiding this comment.
I had to do some testing to understand why this is necessary if we are defining update_db_schema(). Essentially, this argument is needed to extend the table metadata (e.g. column names & types) associated with the table. If this line is omitted in test_orm.py, the test fails with the error:
sqlalchemy.exc.InvalidRequestError: Table 'jobs' is already defined for this MetaData instance. Specify 'extend_existing=True' to redefine options and columns on an existing Table object.
So both the table migration (defined in update_db_schema()) and table metadata migration (defined here by setting __table_args__ = {"extend_existing": True}) must be performed when a new column is added. 😵
No action needed. I just felt it was necessary to call this out to other readers in this review.
andrii-i
dismissed
dlqqq’s stale review
@dlqqq is not available until May 30. All comments were implemented except the request to parametrize identifiers as decided during discussion on the topic with @dlqqq. Details in the comment: #520 (comment).
|
Thank you everyone for reviews. |
…er#520) * add db migration logic and a test for it * make Job and JobDefinition records extendable * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * make updated_job_model a fixture * add return types to test_orm fixtures * refactor update_db_schema logic into a separate function * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * make initial_db return a tuple * improve naming clarity * remove a level of intendation in update_db_schema * Ignore nullability and default values during the db migration, document the fact via comments * improve update_db_schema accordingly to comments --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Add database schema update and database migration logic, test cases.
With this PR, if database schema is extended between versions, schema is automatically updated and relevant tables are altered accordingly preventing errors. Note that database migration logic currently only covers extension of the tables as in addition of new columns.
Fixes #519.