refactor(BA-4004): migrate direct access pattern of group_config to directory pattern (phase 1) #7956

kwonkwonn · 2026-01-12T08:41:17Z

resolves #8061 (BA-4004) (issue will consists of multilple PRs)

Summary

Migrate direct DB access from api/groupconfig.py to repository pattern
Create GroupConfigRepository with db_source layer separation
Add unit tests for repository layer

Changes

New files

repositories/group_config/repository.py - Repository with resilience policies
repositories/group_config/db_source/db_source.py - Pure DB query layer
tests/unit/manager/repositories/group_config/test_group_config_repository.py

Modified files

api/groupconfig.py - Use GroupConfigRepository instead of direct DB queries

Architecture

Concerns

Is it ok to migrate Model/dotfiles.py to repository dir? (no need to query in model
As group_config doesn't have service layer, we are getting Database repo = GroupConfigRepository(root_ctx.db) this way. but we only have few handlers if could over engineering.
As test contains connections between database, it is not pure unit test. but we can only validate repository pattern only with database connection.

Checklist: (if applicable)

Milestone metadata specifying the target backport version
Mention to the original issue
Installer updates including:
- Fixtures for db schema changes
- New mandatory config options
Update of end-to-end CLI integration tests in ai.backend.test
API server-client counterparts (e.g., manager API -> client SDK)
Test case(s) to:
- Demonstrate the difference of before/after
- Demonstrate the flow of abstract/conceptual models with a concrete implementation
Documentation
- Contents in the docs directory
- docstrings in public interfaces and type annotations

Copilot

Pull request overview

This PR refactors the group configuration (dotfiles) API from direct database access to a repository pattern with layered architecture. The change introduces a GroupConfigRepository with a separate GroupConfigDBSource layer and adds comprehensive unit tests for the repository layer.

Changes:

Introduced repository pattern with resilience policies (metrics and retry) for group configuration operations
Separated database access logic into a dedicated db_source layer
Added unit tests for the repository layer covering core functionality

Reviewed changes

Copilot reviewed 5 out of 9 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`src/ai/backend/manager/repositories/group_config/repository.py`	New repository layer with resilience policies for group configuration operations
`src/ai/backend/manager/repositories/group_config/db_source/db_source.py`	New database source layer handling direct SQL queries for dotfiles operations
`src/ai/backend/manager/api/groupconfig.py`	Refactored to use GroupConfigRepository instead of direct database queries
`tests/unit/manager/repositories/group_config/test_group_config_repository.py`	New unit tests for repository layer
`tests/unit/manager/repositories/group_config/BUILD`	Build configuration for tests
`tests/unit/manager/repositories/group_config/__init__.py`	Package initialization
`src/ai/backend/manager/repositories/group_config/__init__.py`	Package initialization
`src/ai/backend/manager/repositories/group_config/db_source/__init__.py`	Package initialization

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/unit/manager/repositories/group_config/test_group_config_repository.py

src/ai/backend/manager/repositories/group_config/repository.py

tests/unit/manager/repositories/group_config/test_group_config_repository.py

src/ai/backend/manager/api/groupconfig.py

fregataa

If you have a plan to add service layer, then please do not change any API handlers in this branch

src/ai/backend/manager/api/groupconfig.py

tests/unit/manager/repositories/group_config/test_group_config_repository.py

src/ai/backend/manager/api/groupconfig.py

kwonkwonn · 2026-01-16T03:41:18Z

This pr contains following functions.
migrate direct access pattern to follow db_source->repository -> service-> processor pattern.
do minimize conflicts while migration, i added tests for repository, service, processor which ensure each following layer to be called once when it is called.

fregataa

test_processors seems to be duplicate of test_service

src/ai/backend/manager/api/groupconfig.py

src/ai/backend/manager/repositories/group_config/db_source/db_source.py

kwonkwonn · 2026-01-19T06:15:43Z

new commits include:

ORM based imports
user qualification functions in datasource.py
- moving user data input inside service layer using 'current_user`

thanks for review!

HyeockJinKim · 2026-01-19T06:24:51Z

src/ai/backend/manager/repositories/group_config/db_source/db_source.py

+    async def resolve_group_id_and_domain(
+        self, group_id_or_name: uuid.UUID | str, domain_name: Optional[str]


Could you please set the Domain input first, as the concept of 'group' is nested under the domain?

src/ai/backend/manager/repositories/group_config/db_source/db_source.py

src/ai/backend/manager/services/group_config/service.py

kwonkwonn · 2026-01-19T09:52:22Z

This commits include

auth logic layer escalation
move datahandle logics down
arguments prioritization

Thanks for reveiw!

fregataa

Looks good. I left a comment to another reviewer.

fregataa · 2026-01-26T06:21:27Z

src/ai/backend/manager/repositories/project_config/repositories.py

+@dataclass
+class GroupConfigRepositories:
+    repository: GroupConfigRepository
+
+    @classmethod
+    def create(cls, args: RepositoryArgs) -> Self:
+        repository = GroupConfigRepository(args.db)
+
+        return cls(
+            repository=repository,
+        )


Do we need such repositories pattern rather than just repository? @HyeockJinKim

I'll handle this part differently once I find a suitable way to separate it.

fregataa · 2026-01-26T06:22:06Z

Please check the CI test

kwonkwonn · 2026-01-26T06:34:35Z

thanks for the review.
i'll figure out what's causing ci test errors.
And would you like me to rebase to up-to-date version to the main before re-pushing this pr?

HyeockJinKim

The code style and the fact that public methods in the repository layer are called multiple times seem to be a bit problematic. When these parts are implemented, it creates too many new database sessions, which doesn't look good.

HyeockJinKim · 2026-01-26T09:29:30Z

src/ai/backend/manager/repositories/group_config/db_source/db_source.py

+    async def resolve_group_id(
+        self, domain_name: Optional[str], group_id_or_name: uuid.UUID | str
+    ) -> uuid.UUID:
+        """


This Union doesn't look good coming in, but I'll fix this later.

HyeockJinKim · 2026-01-26T09:31:35Z

src/ai/backend/manager/repositories/project_config/repositories.py

+@dataclass
+class GroupConfigRepositories:
+    repository: GroupConfigRepository
+
+    @classmethod
+    def create(cls, args: RepositoryArgs) -> Self:
+        repository = GroupConfigRepository(args.db)
+
+        return cls(
+            repository=repository,
+        )


I'll handle this part differently once I find a suitable way to separate it.

kwonkwonn · 2026-01-26T12:44:54Z

I'll think for the point where I can make verification logic(which originally included lot of branching) close to the service,
while minimize DB_connections.
I guess there should be some jobs not just moving inner logics into splitted layer, but patternize some common instructions
and make them to share.

kwonkwonn · 2026-01-27T08:37:22Z

I've put some modifications to minimize db_connection per services,
But I couldn't find better way i can squize more connection per db_source,
while

taking away service logics such as user authorization and error handling under repositories
retaining db_source atomic
not refactoring entire logics

For now, most of the service logics have 2~3 database connections.
for group-config I think it could be tolerable as this service doesn't expect heavy traffics.

But if there's any other way to squize connections or any other ideas please let me know!
i will gladly modify on that way.

thank you.

HyeockJinKim · 2026-01-28T04:41:51Z

src/ai/backend/manager/repositories/group_config/repository.py

+        return await self._db_source.resolve_group(domain_name, group_id_or_name)
+
+    @group_config_repository_resilience.apply()
+    async def get_dotfiles(self, group_id: uuid.UUID) -> GroupDotfilesResult:


We are renaming the group to project, so please adjust to project_id.

HyeockJinKim · 2026-01-28T04:43:59Z

src/ai/backend/manager/repositories/group_config/repository.py

+        result = await self._db_source.get_dotfiles(group_id)
+
+        if result.leftover_space == 0:
+            raise DotfileCreationFailed("No leftover space for dotfile storage")
+        if len(result.dotfiles) >= 100:
+            raise DotfileCreationFailed("Dotfile creation limit reached")
+
+        duplicate = [x for x in result.dotfiles if x["path"] == dotfile.path]
+        if len(duplicate) > 0:
+            raise DotfileAlreadyExists
+
+        new_dotfiles = list(result.dotfiles)
+        new_dotfiles.append({
+            "path": dotfile.path,
+            "perm": dotfile.permission,
+            "data": dotfile.data,
+        })
+        dotfile_packed = msgpack.packb(new_dotfiles)
+        if len(dotfile_packed) > MAXIMUM_DOTFILE_SIZE:
+            raise DotfileCreationFailed("No leftover space for dotfile storage")
+
+        await self._db_source.update_dotfiles(group_id, dotfile_packed)
+


Is it really necessary to handle this at the repository layer? The issue seems to be that the database session gets separated.

HyeockJinKim · 2026-01-28T04:47:24Z

src/ai/backend/manager/services/project_config/service.py

+    async def _resolve_group_for_admin(
+        self,
+        domain_name: str | None,
+        group_id_or_name: uuid.UUID | str,
+        user_domain: str,
+        is_superadmin: bool,
+    ) -> uuid.UUID:


Reading current_user internally would make it more concise.

HyeockJinKim · 2026-01-28T04:49:24Z

src/ai/backend/manager/services/group_config/service.py

+        # Regular user: check if they are a member of the group
+        is_member = await self._group_config_repository.check_user_in_group(user_id, group.id)
+        if not is_member:
+            raise GenericForbidden("Users cannot access group dotfiles of other groups")


Please find an exception that better reflects the situation instead of GenericForbidden.

HyeockJinKim · 2026-01-28T04:50:49Z

src/ai/backend/manager/services/group_config/service.py

+        except ProjectNotFound:
+            # Original API raises DotfileNotFound when group is not found in delete
+            raise DotfileNotFound


Project Not Found doesn't seem to mean DotFileNotFound, does it?

HyeockJinKim

Please rebase the main branch and run the type checker and linter once.

…uery Introduce GroupConfigRepository with ResolvedGroup type that fetches group id and domain_name in a single query, reducing redundant DB round-trips from resolve_group_id + get_group_domain. Co-Authored-By: Claude Opus 4.5 <[email protected]>

Add service layer, action types, processors, and tests for group configuration dotfile operations. Service uses consolidated resolve_group for efficient permission validation. Co-Authored-By: Claude Opus 4.5 <[email protected]>

Copilot AI review requested due to automatic review settings January 12, 2026 08:41

github-actions bot assigned kwonkwonn Jan 12, 2026

github-actions bot added size:XL 500~ LoC comp:manager Related to Manager component labels Jan 12, 2026

Copilot started reviewing on behalf of kwonkwonn January 12, 2026 08:41 View session

Copilot AI reviewed Jan 12, 2026

View reviewed changes

fregataa requested changes Jan 12, 2026

View reviewed changes

kwonkwonn force-pushed the refactor/migrate-groupconfig-to-repository-pattern branch 2 times, most recently from 0d95297 to 1c75810 Compare January 13, 2026 05:27

kwonkwonn requested a review from fregataa January 13, 2026 05:38

kwonkwonn force-pushed the refactor/migrate-groupconfig-to-repository-pattern branch from 1c75810 to 2625aed Compare January 16, 2026 02:46

kwonkwonn commented Jan 16, 2026

View reviewed changes

src/ai/backend/manager/api/groupconfig.py Outdated Show resolved Hide resolved

kwonkwonn changed the title ~~refactor: migrate direct access pattern to directory pattern~~ refactor(BA-3833): migrate direct access pattern of group_config to directory pattern Jan 16, 2026

fregataa reviewed Jan 19, 2026

View reviewed changes

src/ai/backend/manager/api/groupconfig.py Outdated Show resolved Hide resolved

src/ai/backend/manager/repositories/group_config/db_source/db_source.py Outdated Show resolved Hide resolved

src/ai/backend/manager/repositories/group_config/db_source/db_source.py Outdated Show resolved Hide resolved

kwonkwonn force-pushed the refactor/migrate-groupconfig-to-repository-pattern branch from 2625aed to 27b3a56 Compare January 19, 2026 06:02

github-actions bot added the Intern-OKR label Jan 19, 2026

kwonkwonn force-pushed the refactor/migrate-groupconfig-to-repository-pattern branch from 27b3a56 to c007293 Compare January 19, 2026 06:13

HyeockJinKim requested changes Jan 19, 2026

View reviewed changes

kwonkwonn force-pushed the refactor/migrate-groupconfig-to-repository-pattern branch from c007293 to 429f2db Compare January 19, 2026 08:09

kwonkwonn requested review from HyeockJinKim and fregataa January 20, 2026 02:58

kwonkwonn changed the title ~~refactor(BA-3833): migrate direct access pattern of group_config to directory pattern~~ refactor(BA-3833): migrate direct access pattern of group_config to directory pattern(phase 1) Jan 21, 2026

kwonkwonn changed the title ~~refactor(BA-3833): migrate direct access pattern of group_config to directory pattern(phase 1)~~ refactor(BA-3833): migrate direct access pattern of group_config to directory pattern Jan 21, 2026

kwonkwonn force-pushed the refactor/migrate-groupconfig-to-repository-pattern branch 2 times, most recently from 26a8ea8 to b642ec9 Compare January 22, 2026 04:37

kwonkwonn changed the title ~~refactor(BA-3833): migrate direct access pattern of group_config to directory pattern~~ refactor(BA-3833): migrate direct access pattern of group_config to directory pattern (phase 1) Jan 22, 2026

kwonkwonn changed the title ~~refactor(BA-3833): migrate direct access pattern of group_config to directory pattern (phase 1)~~ refactor(BA-4004): migrate direct access pattern of group_config to directory pattern (phase 1) Jan 22, 2026

fregataa previously approved these changes Jan 26, 2026

View reviewed changes

HyeockJinKim requested changes Jan 26, 2026

View reviewed changes

kwonkwonn force-pushed the refactor/migrate-groupconfig-to-repository-pattern branch from b642ec9 to aad2a0b Compare January 27, 2026 08:19

kwonkwonn requested a review from HyeockJinKim January 27, 2026 08:37

HyeockJinKim reviewed Jan 28, 2026

View reviewed changes

kwonkwonn force-pushed the refactor/migrate-groupconfig-to-repository-pattern branch 2 times, most recently from 9c34f42 to ebde60e Compare January 29, 2026 02:53

kwonkwonn requested a review from HyeockJinKim January 29, 2026 05:09

HyeockJinKim reviewed Jan 29, 2026

View reviewed changes

kwonkwonn force-pushed the refactor/migrate-groupconfig-to-repository-pattern branch from ebde60e to 222c002 Compare January 29, 2026 09:01

kwonkwonn requested a review from HyeockJinKim January 29, 2026 09:28

kwonkwonn force-pushed the refactor/migrate-groupconfig-to-repository-pattern branch 3 times, most recently from 5672748 to f461f9c Compare February 3, 2026 01:27

kwonkwonn dismissed fregataa’s stale review via f461f9c February 3, 2026 05:49

kwonkwonn and others added 4 commits February 3, 2026 18:33

refactor: rename all group related names into projects

f9abce4

lint error fixed

0f6082f

kwonkwonn force-pushed the refactor/migrate-groupconfig-to-repository-pattern branch from f461f9c to 0f6082f Compare February 3, 2026 09:35

		async def resolve_group_id_and_domain(
		self, group_id_or_name: uuid.UUID \| str, domain_name: Optional[str]

refactor(BA-4004): migrate direct access pattern of group_config to directory pattern (phase 1) #7956

Are you sure you want to change the base?

refactor(BA-4004): migrate direct access pattern of group_config to directory pattern (phase 1) #7956

Uh oh!

Conversation

kwonkwonn commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

New files

Modified files

Architecture

Concerns

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fregataa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kwonkwonn commented Jan 16, 2026

Uh oh!

fregataa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kwonkwonn commented Jan 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kwonkwonn commented Jan 19, 2026

Uh oh!

fregataa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fregataa commented Jan 26, 2026

Uh oh!

kwonkwonn commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HyeockJinKim left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kwonkwonn commented Jan 26, 2026

Uh oh!

kwonkwonn commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

kwonkwonn commented Jan 12, 2026 •

edited

Loading

kwonkwonn commented Jan 26, 2026 •

edited

Loading

kwonkwonn commented Jan 27, 2026 •

edited

Loading