Fix repo_deps_libyear column typo: current_verion -> current_version #3482
Conversation
Fixes chaoss#2662 The column 'current_verion' in repo_deps_libyear table was misspelled (missing 's'). This commit: - Adds Alembic migration (rev 38) to rename the column - Drops and recreates explorer_libyear_detail materialized view - Updates ORM model, task code, and all schema files for consistency Note: PostgreSQL supports simple column renames via ALTER TABLE, so no temp tables or data copying is needed. Signed-off-by: Shlok Gilda <gildashlok@hotmail.com>
Signed-off-by: Shlok Gilda <gildashlok@hotmail.com>
Which view? Are we still using it for anything? I know on the 8knot side we have wanted to move the 8knot-specifiv mat views over to our 8knot instance for a while. maybe theres a way we can do that more generally, not just in our instance |
Did you also run the migration? Looks like the CI is failing: Stack Trace |
MoralCode
left a comment
MoralCode
left a comment
There was a problem hiding this comment.
Looks like you may have been a little overzealous with the find and replace.
you got the model change (augur_data.py) and any dependent code changes (core.py) but you only need to add one new migration file. previous migrations are intended to help migrate users from older versions of the DB and should be using the old name since thats what exists in their DBs
| b.current_verion, | ||
| b.current_version, | ||
| b.latest_version, | ||
| b.current_release_date, | ||
| b.libyear, | ||
| max(b.data_collection_date) AS max | ||
| FROM augur_data.repo a, | ||
| augur_data.repo_deps_libyear b | ||
| GROUP BY a.repo_id, a.repo_name, b.name, b.requirement, b.current_verion, b.latest_version, b.current_release_date, b.libyear | ||
| GROUP BY a.repo_id, a.repo_name, b.name, b.requirement, b.current_version, b.latest_version, b.current_release_date, b.libyear |
There was a problem hiding this comment.
old migrations should not be touched. this is probably the source of your CI error
| conn.execute(text(""" | ||
| DROP MATERIALIZED VIEW IF EXISTS augur_data.explorer_libyear_detail; | ||
| """)) | ||
|
|
||
| # Step 2: Rename the column (simple metadata operation in PostgreSQL) | ||
| conn.execute(text(""" | ||
| ALTER TABLE augur_data.repo_deps_libyear | ||
| RENAME COLUMN current_verion TO current_version; | ||
| """)) | ||
|
|
||
| # Step 3: Recreate the materialized view with the corrected column name | ||
| conn.execute(text(""" | ||
| CREATE MATERIALIZED VIEW augur_data.explorer_libyear_detail AS |
There was a problem hiding this comment.
im not really a fan of these text-based queries in the migration.
It would be much preferable to use the alembic revision --autogenerate -m "message" command, however, until #3435 is merged, that will cause your migration to contain a lot of unrelated changes
This issue may need to wait until that goes through (unless you want to give it a try and manually remove the unrelated migrations after they are generated)
|
Hey, sorry about this. Meant to mark this as a draft since I knew there were a couple other PRs with migrations in progress (#3435). Completely lost track of this one over the Christmas break and didn't get around to testing it. Re: the feedback:
Also noticed Still planning to wait for #3435 to land so I can regenerate with |
Old migrations should be immutable - the typo fix belongs only in migration 38, not in historical migrations 1 and 4. Signed-off-by: Shlok Gilda <gildashlok@hotmail.com>
The explorer_libyear_detail view was dropped in migration 25, so no view manipulation is needed - just rename the column. Signed-off-by: Shlok Gilda <gildashlok@hotmail.com>
id say needs cleanup. up to you how to handle. |
MoralCode
added
the
waiting
3 participants
Description
current_verioncolumn tocurrent_versioninrepo_deps_libyeartableexplorer_libyear_detailmaterialized viewThis PR fixes #2662
Notes for Reviewers
@sgoggins mentioned in #2716 that renaming might require temp tables and data copying. This isn't necessary for PostgreSQL - column renames are simple metadata operations (
ALTER TABLE ... RENAME COLUMN). The only complexity is the materialized view that references the column, which we drop and recreate with the corrected name. No data copying involved.Signed commits
AI Disclosure: Used Claude Code to write this PR draft, comments in the alembic migration and verify the migration.