Skip to content

fix(BA-6914): give the three drifting FKs one name on both install paths#12910

Draft
jopemachine wants to merge 2 commits into
fix/BA-6913-fresh-install-missing-indexesfrom
fix/BA-6914-fk-constraint-name-drift
Draft

fix(BA-6914): give the three drifting FKs one name on both install paths#12910
jopemachine wants to merge 2 commits into
fix/BA-6913-fresh-install-missing-indexesfrom
fix/BA-6914-fk-constraint-name-drift

Conversation

@jopemachine

@jopemachine jopemachine commented Jul 16, 2026

Copy link
Copy Markdown
Member

📚 Stacked PRs

Merge in order, bottom-up. Each PR is based on the one above it, so its diff shows only its own layer.

Problem

Three migrations create a foreign key under a hardcoded name; the models left it unnamed and let the metadata naming convention generate one. Same key, same enforcement — different name depending on how the DB was built:

migration creates fresh install has
cd067180a8b1 fk_kernels_image_id fk_kernels_image_id_images
c7f2a8e31b04 fk_login_sessions_login_client_type_id fk_login_sessions_login_client_type_id_login_client_types
a3b4c5d6e7f8 fk_prometheus_query_presets_category_id fk_prometheus_query_presets_category_id_prometheus_quer_8a51 ¹

¹ the convention name exceeds PostgreSQL's 63-char identifier limit, so SQLAlchemy truncates it and appends a hash.

Not cosmetic: each downgrade drops the constraint by the hardcoded name and aborts with UndefinedObjectError on a fresh install. Whether a downgrade works depends on where the DB came from.

Which name is correct?

The convention (fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s) is the house style — 81 of 82 model FKs are unnamed. By that count the hardcoded three are the deviation, which argues for renaming migrated DBs to the convention names.

This PR goes the other way, for three reasons:

  1. It would break what works today. Those downgrades run fine on migrated (production) databases right now. Renaming to the convention would break them there too, forcing edits to three released migrations.
  2. The convention name is unusable for one of them — truncated and hash-suffixed. Converging on it would hardcode ..._8a51, tying us to SQLAlchemy's truncation algorithm.
  3. Explicit naming already exists herefk_deployment_policies_endpoint in deployment_policy/row.py.

So: name the three explicitly on the models, and rename on the databases built from the convention. Nothing outside their own migration files references these names.

This is the reviewable decision in this PR — if you'd rather converge on the convention, the cost is editing those three downgrades plus hardcoding the truncated name.

Fix

  1. name= on the three model FKs, matching what the migrations create and their downgrades drop.
  2. c8d1e5b73f92 renames the constraint where it differs. It looks the current name up via the inspector rather than hardcoding the convention name — that keeps it independent of SQLAlchemy's truncation and makes it a no-op where the name already matches. Downgrade is a no-op: undoing it would only put the drift back, and the convention name is not reconstructible.

Verification

Real databases throughout; the dev DB was only ever read.

Fresh install with the fixed models — all three come out with the short names.

Existing fresh install (schema built from main's models, so convention names), after upgrade head:

table before after
kernels fk_kernels_image_id_images fk_kernels_image_id
login_sessions fk_login_sessions_login_client_type_id_login_client_types fk_login_sessions_login_client_type_id
prometheus_query_presets fk_prometheus_query_presets_category_id_prometheus_quer_8a51 fk_prometheus_query_presets_category_id

The three walls, on a fresh install (stamp the revision, run only its downgrade) — all now pass:

migration before this PR
a3b4c5d6e7f8 FAILED — constraint "fk_prometheus_query_presets_category_id" does not exist OK
cd067180a8b1 FAILED — constraint "fk_kernels_image_id" ... does not exist OK
c7f2a8e31b04 FAILED — constraint "fk_login_sessions_login_client_type_id" ... does not exist OK

Found while surveying the downgrade chain for #12881.

🤖 Generated with Claude Code

Three migrations create a foreign key under a hardcoded name while the
models left it unnamed, so the metadata naming convention generated a
different one. The key is created and enforced either way -- only the name
differed, depending on whether the database came from `mgr schema oneshot`
or from replaying migrations. That is not cosmetic: each of those
downgrades drops the constraint by the hardcoded name and aborted on a
fresh install.

Converge on the hardcoded names rather than the convention ones. The
migrations' downgrades already expect them and work today on migrated
databases, so this direction needs no edit to released migrations, and it
sidesteps the convention name for prometheus_query_presets, which exceeds
PostgreSQL's 63-character limit and comes back truncated with a hash
suffix. Naming a constraint explicitly is not novel here -- see
fk_deployment_policies_endpoint.

The rename looks the current name up rather than hardcoding it, so it does
not depend on SQLAlchemy's truncation algorithm and is a no-op where the
name already matches.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the size:M 30~100 LoC label Jul 16, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:manager Related to Manager component require:db-migration Automatically set when alembic migrations are added or updated size:M 30~100 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant