feat(BA-6553): add app_config_fragments repository layer#12307
Merged
Conversation
github-actions
Bot
added
size:XL
jopemachine
force-pushed
the
feat/BA-6552-app-config-fragments-model
branch
from
ee2522e to
e5fdd83
Compare
jopemachine
force-pushed
the
feat/BA-6553-app-config-fragments-repository
branch
from
3096cd0 to
2894d9c
Compare
jopemachine
force-pushed
the
feat/BA-6552-app-config-fragments-model
branch
from
e5fdd83 to
7fa8339
Compare
jopemachine
force-pushed
the
feat/BA-6553-app-config-fragments-repository
branch
from
2894d9c to
8675751
Compare
jopemachine
force-pushed
the
feat/BA-6553-app-config-fragments-repository
branch
from
8675751 to
792f33d
Compare
Add the AppConfigFragmentRepository (create / get / update / search / purge) over a dedicated db_source and DBOpsProvider. Fragments support update (unlike definition/allow-list) and carry a rank: create assigns the next rank within a config_name race-free via create_with_next_value (locking the parent app_config_definitions row), and update may set rank for re-ordering. Adds an applicable_fragments query (public + the user's domain + user fragments, ordered by rank) for the upcoming merge engine, plus query conditions/orders and the AppConfigFragmentNotFound error. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…mainID/UserID Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
jopemachine
force-pushed
the
feat/BA-6553-app-config-fragments-repository
branch
from
792f33d to
520d4e5
Compare
jopemachine
commented
Jun 23, 2026
jopemachine
commented
Jun 23, 2026
- Introduce typed next-value dependency (AppConfigFragmentCreateDependency / DeploymentRevisionPresetCreateDependency) instead of a bare int passed to build_row; execute_next_value_creator now takes a build_dependency factory so each domain owns an extensible dependency type. - Inject pre-created fragments via fixtures (existing_fragment / seeded_fragments) in repository tests instead of calling create inline in non-create tests. - Drop applicable_fragments from the repository/db_source per review. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This was referenced Jun 23, 2026
Match the repository convention used everywhere else (model_card, object_storage, project_resource_policy, etc.): update() receives an `Updater[Row]` (bundling the spec and pk_value), mirroring create()/purge() taking Creator/Purger — instead of separate (fragment_id, spec) params. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Mirror create()/update() and the convention used elsewhere (allow_list, etc.): purge() receives a `Purger[Row]` instead of a bare fragment_id, so the caller composes the spec/identifier object. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Follow the two-variant search convention: admin_search (superadmin/internal, batch_query_in_global, no scope) and scoped_search (batch_query_with_scopes, restricted to the given scopes). Add ConfigNameSearchScope (types.py) so a search can be scoped to one config_name; its existence check rejects an unregistered config_name up front. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
jopemachine
commented
Jun 23, 2026
Drop the per-domain dependency wrapper types and the build_dependency factory; keep the original convention where DependentCreatorSpec.build_row receives the computed next rank as a bare int (next_rank: int). execute_next_value_creator / create_with_next_value go back to DependentCreatorSpec[int, TRow]. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Drop the non-conventional _to_search_result staticmethod helper; build the AppConfigFragmentSearchResult inline in admin_search/scoped_search like every other db_source (model_card, audit_log). Removes the awkward BatchQuerierResult[sa.Row[Any]] typing and now-unused imports. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Follow the repository-test convention (model_card): each input is its own fixture that inserts the row directly via db_sess.add (no repository.create for setup, no inline spec list). Tests inject only the fixtures they need and derive expectations from them. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Replace the per-input fixtures and _seed_*/_spec helpers with a few scenario fixtures (theme_registered, existing_fragment, seeded_fragments) that inline the direct db_sess.add seeding and are injected once per test. Tests derive their expectations from the injected scenario. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Rename existing_fragment -> domain_scoped_fragment and seeded_fragments -> fragments_across_scopes so each fixture reads as the test situation it sets up. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Wrap every repository method with the repository resilience policy (metrics + fixed-backoff retry, BackendAIError treated as non-retryable), matching the convention used across the repository layer. Add the APP_CONFIG_FRAGMENT_REPOSITORY LayerType for the metric label. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces the initial repository-layer implementation for app_config_fragments (part of the BEP-1052 AppConfig stack), including CRUD-style DB operations, search/scoping primitives, and real-DB unit tests. It also aligns related AppConfig repositories with the same resilience/metrics instrumentation pattern and registers new metric layer identifiers.
Changes:
- Add
AppConfigFragmentRepositorybacked byAppConfigFragmentDBSource, plus creator/updater specs and scoped-search support. - Add query condition/order helpers for
AppConfigFragmentRowand aAppConfigFragmentSearchResultDTO. - Add real-DB
with_tablesunit tests and register new repository/db-sourceLayerTypemetric identifiers (and apply resilience decorators to app_config_definition/allow_list repos & db sources).
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/manager/repositories/app_config_fragment/test_repository.py | Real-DB repository tests for create/get/update/purge and search/scoped-search behavior |
| tests/unit/manager/repositories/app_config_fragment/BUILD | Adds Pants target for the new test module |
| tests/unit/manager/repositories/app_config_fragment/init.py | Package marker for the new test directory |
| src/ai/backend/manager/repositories/app_config_fragment/creators.py | Creator spec for rank-assigned fragment creation |
| src/ai/backend/manager/repositories/app_config_fragment/updaters.py | Updater spec for config replacement and optional rank update |
| src/ai/backend/manager/repositories/app_config_fragment/types.py | Adds a ConfigNameSearchScope with existence checks |
| src/ai/backend/manager/repositories/app_config_fragment/db_source/db_source.py | DBSource implementation for create/get/update/purge/admin_search/scoped_search + next-rank policy |
| src/ai/backend/manager/repositories/app_config_fragment/db_source/init.py | Exposes AppConfigFragmentDBSource |
| src/ai/backend/manager/repositories/app_config_fragment/repository.py | Repository facade with resilience wrapper methods |
| src/ai/backend/manager/repositories/app_config_fragment/repositories.py | Repository bundle factory (local aggregator) |
| src/ai/backend/manager/models/app_config_fragment/conditions.py | Condition helpers for filtering + cursor pagination |
| src/ai/backend/manager/models/app_config_fragment/orders.py | Order helpers for sorting fragment queries |
| src/ai/backend/manager/errors/app_config.py | Adds AppConfigFragmentNotFound error type |
| src/ai/backend/manager/data/app_config_fragment/types.py | Adds AppConfigFragmentSearchResult DTO |
| src/ai/backend/common/metrics/metric.py | Registers new LayerType entries for app_config repositories/db-sources |
| src/ai/backend/manager/repositories/app_config_definition/repository.py | Applies resilience wrapper to existing repository methods |
| src/ai/backend/manager/repositories/app_config_definition/db_source/db_source.py | Applies resilience wrapper to existing DBSource methods |
| src/ai/backend/manager/repositories/app_config_allow_list/repository.py | Applies resilience wrapper to existing repository methods |
| src/ai/backend/manager/repositories/app_config_allow_list/db_source/db_source.py | Applies resilience wrapper to existing DBSource methods |
| changes/12307.feature.md | Towncrier news fragment for this feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The applicable_fragments query was removed from this PR; the news fragment now lists only what ships (create/get/update/admin_search/scoped_search/purge + rank). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
fregataa
approved these changes
Jun 23, 2026
This was referenced Jun 23, 2026
Merged
This was referenced Jul 3, 2026
This was referenced Jul 10, 2026
Closed
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.
📚 Stacked PRs
Part of the AppConfigFragment / AppConfig stack under BEP-1052 (epic BA-5781). Merge in order:
feat(BA-6552): app_config_fragments DB model and Alembic migrationfeat(BA-6553): repository layer← you are herefeat(BA-6554): service layer (admin/self write paths, allow-list write-gate)feat(BA-6555): merge engine (deep-merge across applicable fragments)Summary
AppConfigFragmentRepository: create / get_by_id / update / purge / admin_search / scoped_search, over a dedicatedAppConfigFragmentDBSource+DBOpsProvider.createassigns the next rank within aconfig_namerace-free viacreate_with_next_value(locking the parentapp_config_definitionsrow;RANK_GAP=100leaves room to re-order).updatemay setrankexplicitly for admin re-ordering, matching BEP-1052.admin_search(superadmin/internal, unscoped) andscoped_search(restricted toConfigNameSearchScope, whose existence check rejects an unregisteredconfig_name).AppConfigFragmentNotFounderror.with_tablestests cover create/get, update (+missing), purge (+missing), search/pagination, scope_type/scope_id filters, rank ordering, the unique(config_name, scope_type, scope_id)violation, rank auto-assignment, and scoped_search.Notes
Updater/Purgerinternally), consistent with the id-based purge established in the AppConfigAllowList stack.Resolves BA-6553.