refactor(BA-4082): add DB Source Pattern in Group repository #8502

MinJuTur · 2026-02-02T02:09:08Z

Summary

Extract all DB session-opening logic from GroupRepository into GroupDBSource, making GroupRepository a thin delegation layer with resilience decorators only
External dependencies (valkey_stat_client, config_provider, storage_manager) are passed as method parameters to GroupDBSource instead of being stored as fields
Remove dead code (_get_group_by_id) and unnecessary except VFolderOperationFailed: raise pattern
Refactor tests to avoid private field chaining by testing GroupDBSource directly where appropriate

Related Issue

Jira: BA-4082

Test plan

All existing group repository tests pass
Lint (ruff check) passes
Type check (mypy) passes on changed files

🤖 Generated with Claude Code

Copilot

Pull request overview

This PR refactors the GroupRepository by extracting all database session-opening logic into a new GroupDBSource class, making GroupRepository a thin delegation layer that only applies resilience decorators. External dependencies are now passed as method parameters to GroupDBSource instead of being stored as fields, improving the separation of concerns. Dead code was removed and tests were updated to directly test GroupDBSource where appropriate.

Changes:

Created new GroupDBSource class to handle all database operations
Refactored GroupRepository to delegate to GroupDBSource with resilience decorators
Updated tests to test GroupDBSource directly and avoid private field chaining

Reviewed changes

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

File	Description
`src/ai/backend/manager/repositories/group/repository.py`	Simplified to a thin delegation layer; removed all DB logic and delegated to `GroupDBSource`
`src/ai/backend/manager/repositories/group/db_source/db_source.py`	New file containing all DB operations previously in `GroupRepository`
`src/ai/backend/manager/repositories/group/db_source/__init__.py`	New init file exposing `GroupDBSource`
`tests/unit/manager/repositories/group/test_group_repository.py`	Updated test fixtures to test `GroupDBSource` directly; improved fixture composition to avoid private field access

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

Copilot · 2026-02-02T02:16:22Z

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

+
+import copy
+import logging
+import uuid


Module 'uuid' is imported with both 'import' and 'import from'.

This is the same pattern used in the original repository.py.
—import uuid is used for uuid.uuid4() calls, while from uuid import UUID is used for type annotations like Sequence[UUID].

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

MinJuTur · 2026-02-02T02:24:00Z

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

+    _db: ExtendedAsyncSAEngine
+    _role_manager: RoleManager
+
+    def __init__(self, db: ExtendedAsyncSAEngine) -> None:
+        self._db = db
+        self._role_manager = RoleManager()


GroupDBSource stores only _db and _role_manager as fields. External dependencies like storage_manager, valkey_stat_client, and config_provider are passed as method parameters instead.

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

MinJuTur · 2026-02-02T02:30:42Z

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

        if user_update_mode not in (None, "add", "remove"):
            raise InvalidUserUpdateMode("invalid user_update_mode")


This input validation intentionally stays in GroupRepository rather than moving to GroupDBSource, as it only checks the parameter value without any DB access.

MinJuTur · 2026-02-02T02:31:53Z

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

-    async def _get_group_by_id(self, session: SASession, group_id: uuid.UUID) -> GroupRow | None:
-        """Private method to get a group by ID using an existing session."""
-        result = await session.execute(sa.select(GroupRow).where(groups.c.id == group_id))
-        return result.scalar_one_or_none()


Removed _get_group_by_id — confirmed zero references across the entire codebase.

MinJuTur · 2026-02-02T02:37:25Z

tests/unit/manager/repositories/group/test_group_repository.py

+    async def group_db_source(
        self,
        db_with_cleanup: ExtendedAsyncSAEngine,
-        storage_manager_mock: StorageSessionManager,
-    ) -> GroupRepository:
-        """Create GroupRepository instance"""
-        return GroupRepository(
-            db=db_with_cleanup,
-            config_provider=MagicMock(),
-            valkey_stat_client=MagicMock(),
-            storage_manager=storage_manager_mock,
-        )
+    ) -> GroupDBSource:
+        """Create GroupDBSource instance"""
+        return GroupDBSource(db=db_with_cleanup)


Refactored tests to avoid private field chaining (repo._db_source._method). TestGroupRepositoryDeleteEndpoints now tests GroupDBSource directly.

MinJuTur · 2026-02-02T02:38:20Z

tests/unit/manager/repositories/group/test_group_repository.py

+    @pytest.fixture
+    async def group_db_source_with_mock_role_manager(
+        self,
+        db_with_cleanup: ExtendedAsyncSAEngine,
+    ) -> GroupDBSource:
+        """GroupDBSource with mocked RoleManager for create tests."""
+        db_source = GroupDBSource(db=db_with_cleanup)
+        mock_role_manager = MagicMock()
+        mock_role_manager.create_system_role = AsyncMock(return_value=None)
+        db_source._role_manager = mock_role_manager
+        return db_source
+


For create tests that need the full repository flow (with resilience), a group_db_source_with_mock_role_manager fixture is separated so each fixture only accesses one level of private fields.

tests/unit/manager/repositories/group/test_group_repository.py

changes/8502.enhance.md

tests/unit/manager/repositories/group/test_group_db_source.py

HyeockJinKim

Please review the conflicts.

Extract all DB session-opening logic from GroupRepository into GroupDBSource, making GroupRepository a thin delegation layer with resilience decorators only. This follows the same pattern applied to UserRepository in BA-4080. Key changes: - Create GroupDBSource class with all DB access methods - GroupRepository delegates to GroupDBSource for all operations - External dependencies (valkey_stat_client, config_provider, storage_manager) passed as method parameters to db_source - Remove dead code _get_group_by_id (no references found) - Remove unnecessary except-reraise for VFolderOperationFailed - Refactor tests to avoid private field chaining by testing GroupDBSource directly where appropriate Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

…older check Replace bare `except: pass` with `log.warning` to aid debugging when mount entry parsing fails during active kernel vfolder mount checks. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Move TestGroupRepositoryDeleteEndpoints to test_group_db_source.py as TestGroupDBSourceDeleteEndpoints, keeping test_group_repository.py focused on GroupRepository public API tests only. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

… fixtures Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings February 2, 2026 02:09

github-actions bot assigned MinJuTur Feb 2, 2026

Copilot started reviewing on behalf of MinJuTur February 2, 2026 02:09 View session

MinJuTur added the Intern-OKR label Feb 2, 2026

Copilot AI reviewed Feb 2, 2026

View reviewed changes

MinJuTur commented Feb 2, 2026

View reviewed changes

src/ai/backend/manager/repositories/group/repository.py Show resolved Hide resolved

MinJuTur commented Feb 2, 2026

View reviewed changes

MinJuTur requested review from HyeockJinKim and jopemachine February 2, 2026 03:56

MinJuTur force-pushed the refactor/BA-4082-introduce-db-source-pattern-in-group-repository branch from 058198f to ddf1ca4 Compare February 2, 2026 04:41

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

jopemachine reviewed Feb 2, 2026

View reviewed changes

tests/unit/manager/repositories/group/test_group_repository.py Outdated Show resolved Hide resolved

MinJuTur requested a review from jopemachine February 2, 2026 08:04

jopemachine reviewed Feb 2, 2026

View reviewed changes

changes/8502.enhance.md Outdated Show resolved Hide resolved

MinJuTur requested a review from jopemachine February 2, 2026 08:21

jopemachine reviewed Feb 2, 2026

View reviewed changes

tests/unit/manager/repositories/group/test_group_db_source.py Outdated Show resolved Hide resolved

jopemachine reviewed Feb 2, 2026

View reviewed changes

tests/unit/manager/repositories/group/test_group_db_source.py Outdated Show resolved Hide resolved

MinJuTur requested a review from jopemachine February 2, 2026 09:09

HyeockJinKim previously approved these changes Feb 3, 2026

View reviewed changes

HyeockJinKim reviewed Feb 3, 2026

View reviewed changes

MinJuTur and others added 3 commits February 3, 2026 04:27

docs(BA-4082): Add changelog for group repository db_source pattern

5d405d6

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

fix(BA-4082): Add warning log for malformed mount entries in group vf…

eb93fd1

…older check Replace bare `except: pass` with `log.warning` to aid debugging when mount entry parsing fails during active kernel vfolder mount checks. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

MinJuTur added this to the 26.2 milestone Feb 3, 2026

MinJuTur and others added 2 commits February 3, 2026 04:34

docs(BA-4082): Align changelog with previous db_source PR convention

d3374c4

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

refactor(BA-4082): Replace dict return types with dataclasses in test…

eb7c707

… fixtures Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

MinJuTur force-pushed the refactor/BA-4082-introduce-db-source-pattern-in-group-repository branch from 3e7a6c2 to eb7c707 Compare February 3, 2026 04:38

MinJuTur requested a review from HyeockJinKim February 3, 2026 04:50

jopemachine added this pull request to the merge queue Feb 3, 2026

Merged via the queue into main with commit a40d0f0 Feb 3, 2026
30 checks passed

MinJuTur dismissed HyeockJinKim’s stale review via eb7c707 February 3, 2026 05:50

jopemachine deleted the refactor/BA-4082-introduce-db-source-pattern-in-group-repository branch February 3, 2026 05:50

		if user_update_mode not in (None, "add", "remove"):
		raise InvalidUserUpdateMode("invalid user_update_mode")

refactor(BA-4082): add DB Source Pattern in Group repository #8502

refactor(BA-4082): add DB Source Pattern in Group repository #8502

Uh oh!

Conversation

MinJuTur commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

MinJuTur Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MinJuTur Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MinJuTur Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MinJuTur Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

MinJuTur Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

MinJuTur Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HyeockJinKim left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MinJuTur commented Feb 2, 2026 •

edited

Loading

MinJuTur Feb 2, 2026 •

edited

Loading

MinJuTur Feb 2, 2026 •

edited

Loading