Migrations: fix the rename of name to label for DbComputer
#4926
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #4926 +/- ##
===========================================
- Coverage 80.06% 80.05% -0.00%
===========================================
Files 515 515
Lines 36622 36619 -3
===========================================
- Hits 29316 29313 -3
Misses 7306 7306
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| op.create_unique_constraint('db_dbcomputer_label_key', 'db_dbcomputer', ['label']) | ||
| op.drop_column('db_dbcomputer', 'name') | ||
| op.drop_constraint('db_dbcomputer_name_key', 'db_dbcomputer') |
There was a problem hiding this comment.
What happens to this constraint when you alter the column? Maybe it's better to drop it before altering (also for performance, so the index does not exist and does not need to be updated while altering)? Same for downgrade (drop before altering)
There was a problem hiding this comment.
Not sure to be honest. Since this should be a small index (shouldn't be a huge number of computers) I think it won't have a huge impact, but happy to swap the order.
The migration was broken for SqlAlchemy and wasn't detected because there were no tests. The migration of new profiles still worked because it only failed for databases that have at least one computer. The new migration properly uses `alter_column` to change the column name, instead of creating a new one and dropping the old one. Not only does this incur the exception that the new column will have null values, which is not allowed since it is not nullable, but it would also have lost all the data. Finally, the `label` column has a uniqueness constraint and the name should technically also be changed, so the constraint is dropped and added with the new name based on `label`.
sphuber
force-pushed
the
fix/4925/migration-computer-name-to-label
branch
from
966126c to
2940666
Compare
Fixes #4925
The migration was broken for SqlAlchemy and wasn't detected because
there were no tests. The migration of new profiles still worked because
it only failed for databases that have at least one computer.
The new migration properly uses
alter_columnto change the columnname, instead of creating a new one and dropping the old one. Not only
does this incur the exception that the new column will have null values,
which is not allowed since it is not nullable, but it would also have
lost all the data.
Finally, the
labelcolumn has a uniqueness constraint and the nameshould technically also be changed, so the constraint is dropped and
added with the new name based on
label.