fix(BA-6085): seed reads_vfolder_config_files=true for custom variant#11661
Open
seedspirit wants to merge 4 commits into
Open
fix(BA-6085): seed reads_vfolder_config_files=true for custom variant#11661seedspirit wants to merge 4 commits into
seedspirit wants to merge 4 commits into
Conversation
The original runtime_variants seed migration inserted rows with only name/description, and populate_fixture's ON CONFLICT DO NOTHING never overwrites the existing rows, so the install fixture's reads_vfolder_config_files=true for custom was silently dropped. Backfill the value via a new migration so custom actually scans model-definition files from the vfolder at revision-creation time. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a one-shot Alembic migration to backfill reads_vfolder_config_files=true for the seeded custom runtime variant, so custom model-serving revisions can read model-definition.yaml from the model vfolder as intended.
Changes:
- Adds a database migration that updates the
customruntime variant flag on upgrade and resets it on downgrade. - Adds a fix news fragment describing the model-definition loading fix.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/ai/backend/manager/models/alembic/versions/0b10b2c6a972_seed_reads_vfolder_for_custom_variant.py |
Adds the backfill migration for custom.reads_vfolder_config_files. |
changes/11661.fix.md |
Adds the release-note fragment for the custom runtime variant fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+39
to
+43
| op.execute( | ||
| sa.text( | ||
| "UPDATE runtime_variants SET reads_vfolder_config_files = false WHERE name = 'custom'" | ||
| ) | ||
| ) |
The prior revision 7ea9f3c1b2d5 already sets custom.reads_vfolder_config_files to TRUE during column creation, so unconditionally writing FALSE on downgrade clobbered that valid prior state. Drop the UPDATE so downgrade leaves the row untouched. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
runtime_variantsseed migration (9229f72fa447) inserted rows with onlyname/description, soreads_vfolder_config_filestook its server default offalsefor every variant.populate_fixtureusesINSERT … ON CONFLICT DO NOTHING, so the install fixture's"reads_vfolder_config_files": trueforcustomis silently dropped on databases that already have the seed row.UPDATE runtime_variants SET reads_vfolder_config_files = true WHERE name = 'custom'so existing databases get the intended flag and the vfoldermodel-definition.yamlscan kicks in at revision-creation time.Test plan
alembic upgrade headfromba42cb865efe:custombecomestrue, other 6 variants untouchedalembic downgrade -1:customreverts tofalse, version returns toba42cb865efeupgrade headon a database wherecustomis alreadytrueis a no-op (UPDATE is idempotent)add_revisionwithcustomvariant and a model vfolder persists the resolvedmodel_definition_path(model-definition.yaml) instead of an empty stringResolves BA-6085