feat(BA-3487): Implement ScalingGroup Domain Association Actions #7651

jopemachine · 2025-12-31T01:27:54Z

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 implements the AssociateScalingGroupWithDomainAction as part of BA-3487, refactoring the domain association operations from direct SQL queries to the new action/processor pattern. The implementation adds action definitions, service methods, repository operations, and comprehensive test coverage at both service and repository levels. However, the PR only implements the Associate action and refactors only the plural mutation, leaving the singular mutation and the Disassociate action unimplemented despite the original issue requirements.

Implements AssociateScalingGroupWithDomainAction following the action pattern
Refactors AssociateScalingGroupsWithDomain (plural) mutation to use the new action
Adds comprehensive tests for service and repository layers

Reviewed changes

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

Show a summary per file

File	Description
`src/ai/backend/manager/services/scaling_group/actions/associate_with_domain.py`	Defines the action and result classes for associating a scaling group with a domain
`src/ai/backend/manager/services/scaling_group/service.py`	Adds service method to handle the association action
`src/ai/backend/manager/services/scaling_group/processors.py`	Registers the new action processor
`src/ai/backend/manager/repositories/scaling_group/repository.py`	Adds repository method for domain association
`src/ai/backend/manager/repositories/scaling_group/db_source/db_source.py`	Implements database operation for creating the association
`src/ai/backend/manager/repositories/scaling_group/creators.py`	Adds `ScalingGroupForDomainCreatorSpec` for building association rows
`src/ai/backend/manager/models/gql_models/scaling_group.py`	Refactors `AssociateScalingGroupsWithDomain` (plural) mutation to use the new action pattern
`tests/unit/manager/services/scaling_group/test_scaling_group_service.py`	Adds service layer tests for success and error propagation
`tests/unit/manager/repositories/scaling_group/test_scaling_group_repository.py`	Adds repository layer test with fixtures, updates cleanup logic for association table
`changes/7651.feature.md`	Changelog entry for the feature

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

Copilot · 2025-12-31T02:38:53Z

changes/7651.feature.md

@@ -0,0 +1 @@
+Implement `AssociateScalingGroupWithDomainAction`, and repository methods


According to issue #7487, this PR should implement both AssociateScalingGroupWithDomainAction AND DisassociateScalingGroupWithDomainAction. However, only the Associate action has been implemented. The DisassociateScalingGroupWithDomain mutations (both singular and plural variants) still use the old simple_db_mutate pattern and have not been refactored to use the new action pattern.

Copilot · 2025-12-31T02:38:54Z

tests/unit/manager/repositories/scaling_group/test_scaling_group_repository.py

+    async def test_associate_scaling_group_with_domain_success(
+        self,
+        scaling_group_repository: ScalingGroupRepository,
+        db_with_cleanup: ExtendedAsyncSAEngine,
+        sample_scaling_group_for_association: str,
+        sample_domain: str,
+    ) -> None:
+        """Test associating a scaling group with a domain"""
+        creator = Creator(
+            spec=ScalingGroupForDomainCreatorSpec(
+                scaling_group=sample_scaling_group_for_association,
+                domain=sample_domain,
+            )
+        )
+        await scaling_group_repository.associate_scaling_group_with_domain(creator)
+
+        # Verify association
+        async with db_with_cleanup.begin_readonly_session() as db_sess:
+            result = await db_sess.execute(
+                sa.select(ScalingGroupForDomainRow).where(
+                    sa.and_(
+                        ScalingGroupForDomainRow.scaling_group
+                        == sample_scaling_group_for_association,
+                        ScalingGroupForDomainRow.domain == sample_domain,
+                    )
+                )
+            )
+            association = result.scalar_one_or_none()
+            assert association is not None
+            assert association.scaling_group == sample_scaling_group_for_association
+            assert association.domain == sample_domain


Consider adding a test case for the duplicate association scenario at the repository level. While the service layer tests the IntegrityError propagation, it would be valuable to have a repository test that verifies the database unique constraint (uq_sgroup_domain) works as expected when attempting to create a duplicate association. This would test the actual database behavior rather than just mocking it.

Copilot · 2025-12-31T02:38:54Z

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

+    async def associate_scaling_group_with_domain(
+        self,
+        creator: Creator[ScalingGroupForDomainRow],
+    ) -> None:
+        """Associates a single scaling group with a domain."""
+        await self._db_source.associate_scaling_group_with_domain(creator)


The associate_scaling_group_with_domain method is missing the @scaling_group_repository_resilience.apply() decorator that is applied to all other repository methods (create_scaling_group, search_scaling_groups, purge_scaling_group). For consistency and to benefit from the retry and metric policies, this decorator should be added.

Copilot · 2025-12-31T02:38:54Z

src/ai/backend/manager/models/gql_models/scaling_group.py

+        ctx: GraphQueryContext = info.context
+        for scaling_group in scaling_groups:
+            creator = Creator(
+                spec=ScalingGroupForDomainCreatorSpec(
+                    scaling_group=scaling_group,
+                    domain=domain,
+                )
+            )
+            await (
+                ctx.processors.scaling_group.associate_scaling_group_with_domain.wait_for_complete(
+                    AssociateScalingGroupWithDomainAction(creator=creator)
+                )
+            )
+        return cls(ok=True, msg="")


The AssociateScalingGroupWithDomain mutation (singular) should also be refactored to use the new action pattern for consistency. Currently, this mutation still uses the old simple_db_mutate pattern (line 752-756 in the original code), while AssociateScalingGroupsWithDomain (plural) has been refactored to use the new action pattern. Both mutations should use the same implementation approach. The singular mutation should be refactored to use AssociateScalingGroupWithDomainAction just like the plural version now does.

Copilot · 2025-12-31T02:38:54Z

changes/7651.feature.md

@@ -0,0 +1 @@
+Implement `AssociateScalingGroupWithDomainAction`, and repository methods


The changelog file is named 7651.feature.md but references issue #7487 (BA-3487) in the PR description. The changelog filename should match the issue number. Either the filename should be 7487.feature.md, or there's a mismatch between the issue numbers.

seedspirit · 2026-01-05T08:47:09Z

tests/unit/manager/repositories/scaling_group/test_scaling_group_repository.py

+        db_with_cleanup: ExtendedAsyncSAEngine,
+    ) -> AsyncGenerator[str, None]:
+        """Create a sample domain for testing"""
+        domain_name = "test-domain-for-sgroup"


How about making domain name with random uuid to avoid conflict?

Since all test fixtures are isolated, I think there doesn’t seem to be any need to worry about conflicts.

HyeockJinKim

Could you explain in what situations those newly added Repository methods should be used? @jopemachine

jopemachine · 2026-01-06T06:45:08Z

Could you explain in what situations those newly added Repository methods should be used? @jopemachine

There are lots of association, disassociation legacy GQL mutations. I will refactor them with this actions @HyeockJinKim

backend.ai/src/ai/backend/manager/api/gql_legacy/schema.py

Lines 417 to 442 in 9bc6db4

    
           associate_scaling_group_with_domain = AssociateScalingGroupWithDomain.Field() 
        
           associate_scaling_groups_with_domain = AssociateScalingGroupsWithDomain.Field( 
        
               description="Added in 24.03.9" 
        
           ) 
        
           associate_scaling_group_with_user_group = AssociateScalingGroupWithUserGroup.Field() 
        
           associate_scaling_groups_with_user_group = AssociateScalingGroupsWithUserGroup.Field( 
        
               description="Added in 24.03.9" 
        
           ) 
        
           associate_scaling_group_with_keypair = AssociateScalingGroupWithKeyPair.Field() 
        
           associate_scaling_groups_with_keypair = AssociateScalingGroupsWithKeyPair.Field( 
        
               description="Added in 24.03.9" 
        
           ) 
        
           disassociate_scaling_group_with_domain = DisassociateScalingGroupWithDomain.Field() 
        
           disassociate_scaling_groups_with_domain = DisassociateScalingGroupsWithDomain.Field( 
        
               description="Added in 24.03.9" 
        
           ) 
        
           disassociate_scaling_group_with_user_group = DisassociateScalingGroupWithUserGroup.Field() 
        
           disassociate_scaling_groups_with_user_group = DisassociateScalingGroupsWithUserGroup.Field( 
        
               description="Added in 24.03.9" 
        
           ) 
        
           disassociate_scaling_group_with_keypair = DisassociateScalingGroupWithKeyPair.Field() 
        
           disassociate_scaling_groups_with_keypair = DisassociateScalingGroupsWithKeyPair.Field( 
        
               description="Added in 24.03.9" 
        
           ) 
        
           disassociate_all_scaling_groups_with_domain = DisassociateAllScalingGroupsWithDomain.Field() 
        
           disassociate_all_scaling_groups_with_group = DisassociateAllScalingGroupsWithGroup.Field()

jopemachine · 2026-01-06T07:09:43Z

Could you explain in what situations those newly added Repository methods should be used? @jopemachine

There are lots of association, disassociation legacy GQL mutations. I will refactor them with this actions @HyeockJinKim

backend.ai/src/ai/backend/manager/api/gql_legacy/schema.py

Lines 417 to 442 in 9bc6db4

associate_scaling_group_with_domain = AssociateScalingGroupWithDomain.Field()

associate_scaling_groups_with_domain = AssociateScalingGroupsWithDomain.Field(

description="Added in 24.03.9"

)

associate_scaling_group_with_user_group = AssociateScalingGroupWithUserGroup.Field()

associate_scaling_groups_with_user_group = AssociateScalingGroupsWithUserGroup.Field(

description="Added in 24.03.9"

)

associate_scaling_group_with_keypair = AssociateScalingGroupWithKeyPair.Field()

associate_scaling_groups_with_keypair = AssociateScalingGroupsWithKeyPair.Field(

description="Added in 24.03.9"

)

disassociate_scaling_group_with_domain = DisassociateScalingGroupWithDomain.Field()

disassociate_scaling_groups_with_domain = DisassociateScalingGroupsWithDomain.Field(

description="Added in 24.03.9"

)

disassociate_scaling_group_with_user_group = DisassociateScalingGroupWithUserGroup.Field()

disassociate_scaling_groups_with_user_group = DisassociateScalingGroupsWithUserGroup.Field(

description="Added in 24.03.9"

)

disassociate_scaling_group_with_keypair = DisassociateScalingGroupWithKeyPair.Field()

disassociate_scaling_groups_with_keypair = DisassociateScalingGroupsWithKeyPair.Field(

description="Added in 24.03.9"

)

disassociate_all_scaling_groups_with_domain = DisassociateAllScalingGroupsWithDomain.Field()

disassociate_all_scaling_groups_with_group = DisassociateAllScalingGroupsWithGroup.Field()

- Add DisassociateScalingGroupWithDomainAction using Purger pattern - Add ScalingGroupForDomainPurgerSpec for batch deletion - Update db_source, repository, and service layer - Register processor for disassociate action - Update GQL mutations to use action-processor pattern - Add unit tests for repository and service 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

Reverts the GQL mutation changes for associate/disassociate scaling group with domain to use the original simple_db_mutate pattern instead of the action-processor pattern, keeping only the underlying service layer with the action-processor pattern. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>

…ry layer

HyeockJinKim · 2026-01-06T08:50:24Z

tests/unit/manager/services/scaling_group/test_scaling_group_service.py

-    async def test_associate_scaling_group_with_domains_repository_error_propagates(
-        self,
-        scaling_group_service: ScalingGroupService,
-        mock_repository: MagicMock,
-    ) -> None:
-        """Test that repository errors propagate through the service for association"""
-        mock_repository.associate_scaling_group_with_domains = AsyncMock(
-            side_effect=IntegrityError("statement", {}, Exception("Duplicate entry"))
-        )
-
-        bulk_creator: BulkCreator[ScalingGroupForDomainRow] = BulkCreator(
-            specs=[
-                ScalingGroupForDomainCreatorSpec(
-                    scaling_group="test-sgroup",
-                    domain="test-domain",
-                )
-            ]
-        )
-        action = AssociateScalingGroupWithDomainsAction(bulk_creator=bulk_creator)
-
-        with pytest.raises(IntegrityError):
-            await scaling_group_service.associate_scaling_group_with_domains(action)


Why was this deleted?

I removed it because it felt like an obvious assumption.

jopemachine changed the title ~~feat: Implement AssociateScalingGroupWithDomain Action~~ feat(BA-3487): Implement AssociateScalingGroupWithDomain Action Dec 31, 2025

github-actions bot assigned jopemachine Dec 31, 2025

jopemachine added this to the 25.19 milestone Dec 31, 2025

jopemachine force-pushed the feat/BA-3487 branch from 55c7624 to 7f1e51d Compare December 31, 2025 01:48

github-actions bot added size:L 100~500 LoC comp:manager Related to Manager component labels Dec 31, 2025

jopemachine marked this pull request as ready for review December 31, 2025 02:31

jopemachine requested review from HyeockJinKim, Copilot and seedspirit December 31, 2025 02:31

Copilot started reviewing on behalf of jopemachine December 31, 2025 02:32 View session

Copilot AI reviewed Dec 31, 2025

View reviewed changes

jopemachine changed the title ~~feat(BA-3487): Implement AssociateScalingGroupWithDomain Action~~ feat(BA-3487): Implement ScalingGroup Domain Association Actions Dec 31, 2025

github-actions bot added size:XL 500~ LoC and removed size:L 100~500 LoC labels Dec 31, 2025

jopemachine marked this pull request as draft December 31, 2025 06:06

HyeockJinKim force-pushed the main branch 2 times, most recently from 9552aac to 4af738e Compare December 31, 2025 15:41

jopemachine force-pushed the feat/BA-3487 branch from b47a35d to 6425a52 Compare January 5, 2026 08:27

github-actions bot added size:L 100~500 LoC and removed size:XL 500~ LoC labels Jan 5, 2026

jopemachine modified the milestones: 25.19, 26.1 Jan 5, 2026

seedspirit reviewed Jan 5, 2026

View reviewed changes

jopemachine marked this pull request as ready for review January 5, 2026 08:51

jopemachine force-pushed the feat/BA-3487 branch from d61a125 to c60f0d0 Compare January 6, 2026 05:25

github-actions bot added size:XL 500~ LoC and removed size:L 100~500 LoC labels Jan 6, 2026

jopemachine requested a review from seedspirit January 6, 2026 05:47

HyeockJinKim reviewed Jan 6, 2026

View reviewed changes

jopemachine requested a review from HyeockJinKim January 6, 2026 07:09

jopemachine closed this Jan 6, 2026

jopemachine reopened this Jan 6, 2026

jopemachine and others added 10 commits January 6, 2026 17:40

feat: Implement AssociateScalingGroupWithDomain Action

bd9da44

test: Add

6035510

docs: Add news fragment

52f3561

docs: Add news fragment

20b94cb

docs: Add news fragment

7ca78dd

feat: Add check_scaling_group_domain_association_exists into reposito…

f133b69

…ry layer

feat: Make batch job

06059f0

test: Update

91a9e87

jopemachine force-pushed the feat/BA-3487 branch from 15c6cc6 to 91a9e87 Compare January 6, 2026 08:44

fix: Useless test

c4d4801

HyeockJinKim reviewed Jan 6, 2026

View reviewed changes

jopemachine requested a review from HyeockJinKim January 6, 2026 08:53

HyeockJinKim approved these changes Jan 7, 2026

View reviewed changes

HyeockJinKim added this pull request to the merge queue Jan 7, 2026

Merged via the queue into main with commit 2c70e26 Jan 7, 2026
28 checks passed

HyeockJinKim deleted the feat/BA-3487 branch January 7, 2026 06:37

		@@ -0,0 +1 @@
		Implement `AssociateScalingGroupWithDomainAction`, and repository methods

feat(BA-3487): Implement ScalingGroup Domain Association Actions #7651

feat(BA-3487): Implement ScalingGroup Domain Association Actions #7651

Conversation

jopemachine commented Dec 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

seedspirit Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

jopemachine Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

HyeockJinKim left a comment

Choose a reason for hiding this comment

Uh oh!

jopemachine commented Jan 6, 2026

Uh oh!

jopemachine commented Jan 6, 2026

Uh oh!

HyeockJinKim Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

jopemachine Jan 6, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jopemachine commented Dec 31, 2025 •

edited

Loading

jopemachine Jan 6, 2026 •

edited

Loading