feat(BA-3486): Implement `ModifyScalingGroup` Action #7522

jopemachine · 2025-12-19T07:11:14Z

resolves #7486 (BA-3486)

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

tests/manager/repositories/scaling_group/test_scaling_group_repository.py

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

+        assert result.metadata.description == "Updated description"
+        assert result.status.is_active is False
+        assert result.status.is_public is False
+        assert result.network.wsproxy_addr == "http://new-wsproxy:5000"


Copilot

Pull request overview

This PR implements the ModifyScalingGroup action following the repository pattern, enabling partial updates to scaling groups using the Updater pattern with OptionalState and TriState for nullable and non-nullable fields respectively.

Adds ModifyScalingGroupAction and corresponding action result classes
Implements ScalingGroupUpdaterSpec with support for partial field updates
Adds repository and service layer methods for updating scaling groups
Includes comprehensive unit tests at both repository and service layers

Reviewed changes

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

Show a summary per file

File	Description
src/ai/backend/manager/services/scaling_group/actions/modify.py	New action class for modifying scaling groups with updater pattern
src/ai/backend/manager/repositories/scaling_group/updaters.py	New updater specification defining updateable fields and their state handling
src/ai/backend/manager/repositories/scaling_group/db_source/db_source.py	Implements database update operation with error handling for non-existent groups
src/ai/backend/manager/repositories/scaling_group/repository.py	Adds repository method wrapper for update operations
src/ai/backend/manager/services/scaling_group/service.py	Adds service layer method for modify operation
src/ai/backend/manager/services/scaling_group/processors.py	Registers modify action processor in the processor package
tests/unit/manager/services/scaling_group/test_scaling_group_service.py	Service layer tests including success and not-found error cases
tests/unit/manager/repositories/scaling_group/test_scaling_group_repository.py	Repository layer tests covering database update operations and error conditions
changes/7522.feature.md	Changelog entry documenting the new feature

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

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

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

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

+    def _create_scaling_group_updater(
+        self,
+        name: str,
+        description: Optional[TriState[str]] = None,
+        is_active: Optional[OptionalState[bool]] = None,
+        is_public: Optional[OptionalState[bool]] = None,
+        wsproxy_addr: Optional[TriState[str]] = None,
+        wsproxy_api_token: Optional[TriState[str]] = None,
+        driver: Optional[OptionalState[str]] = None,
+        driver_opts: Optional[OptionalState[Mapping[str, Any]]] = None,
+        scheduler: Optional[OptionalState[str]] = None,
+        scheduler_opts: Optional[OptionalState[ScalingGroupOpts]] = None,
+        use_host_network: Optional[OptionalState[bool]] = None,
+    ) -> Updater[ScalingGroupRow]:
+        """Create a ScalingGroupUpdaterSpec with the given parameters."""
+        spec = ScalingGroupUpdaterSpec(
+            description=description if description is not None else TriState.nop(),
+            is_active=is_active if is_active is not None else OptionalState.nop(),
+            is_public=is_public if is_public is not None else OptionalState.nop(),
+            wsproxy_addr=wsproxy_addr if wsproxy_addr is not None else TriState.nop(),
+            wsproxy_api_token=(
+                wsproxy_api_token if wsproxy_api_token is not None else TriState.nop()
+            ),
+            driver=driver if driver is not None else OptionalState.nop(),
+            driver_opts=driver_opts if driver_opts is not None else OptionalState.nop(),
+            scheduler=scheduler if scheduler is not None else OptionalState.nop(),
+            scheduler_opts=scheduler_opts if scheduler_opts is not None else OptionalState.nop(),
+            use_host_network=(
+                use_host_network if use_host_network is not None else OptionalState.nop()
+            ),
+        )
+        return Updater(spec=spec, pk_value=name)


This helper method is duplicated between the service test file and the repository test file. The implementation is identical in both locations (lines 340-371 here and lines 82-113 in test_scaling_group_repository.py). Consider extracting this into a shared test utility module to reduce code duplication and ensure consistency across tests.

changes/7522.feature.md

seedspirit · 2026-01-05T08:42:43Z

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

+        new_scheduler_opts = ScalingGroupOpts(
+            allowed_session_types=[SessionTypes.BATCH],
+        )
+        updater = self._create_scaling_group_updater(
+            name=sample_scaling_group_for_update,
+            description=TriState.update("Updated description"),
+            is_active=OptionalState.update(False),
+            is_public=OptionalState.update(False),
+            wsproxy_addr=TriState.update("http://new-wsproxy:5000"),
+            wsproxy_api_token=TriState.update("new-token"),
+            driver=OptionalState.update("docker"),
+            driver_opts=OptionalState.update({"new_opt": "value"}),
+            scheduler=OptionalState.update("drf"),
+            scheduler_opts=OptionalState.update(new_scheduler_opts),
+            use_host_network=OptionalState.update(True),
+        )


I think it can be provided as a fixture

If we extract the inputs into fixtures, it becomes hard to see at a glance what the test is actually verifying, so I think applying fixtures here does not seem like a good idea.

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

HyeockJinKim · 2026-01-06T07:59:29Z

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

+    def _create_scaling_group_updater(
+        self,
+        name: str,
+        description: Optional[TriState[str]] = None,
+        is_active: Optional[OptionalState[bool]] = None,
+        is_public: Optional[OptionalState[bool]] = None,
+        wsproxy_addr: Optional[TriState[str]] = None,
+        wsproxy_api_token: Optional[TriState[str]] = None,
+        driver: Optional[OptionalState[str]] = None,
+        driver_opts: Optional[OptionalState[Mapping[str, Any]]] = None,
+        scheduler: Optional[OptionalState[str]] = None,
+        scheduler_opts: Optional[OptionalState[ScalingGroupOpts]] = None,
+        use_host_network: Optional[OptionalState[bool]] = None,
+    ) -> Updater[ScalingGroupRow]:
+        """Create a ScalingGroupUpdaterSpec with the given parameters."""
+        # Build sub-specs only if any of their fields are provided
+        status_spec: ScalingGroupStatusUpdaterSpec | None = None
+        if is_active is not None or is_public is not None:
+            status_spec = ScalingGroupStatusUpdaterSpec(
+                is_active=is_active if is_active is not None else OptionalState.nop(),
+                is_public=is_public if is_public is not None else OptionalState.nop(),
+            )
+
+        metadata_spec: ScalingGroupMetadataUpdaterSpec | None = None
+        if description is not None:
+            metadata_spec = ScalingGroupMetadataUpdaterSpec(
+                description=description,
+            )
+
+        network_spec: ScalingGroupNetworkConfigUpdaterSpec | None = None
+        if (
+            wsproxy_addr is not None
+            or wsproxy_api_token is not None
+            or use_host_network is not None
+        ):
+            network_spec = ScalingGroupNetworkConfigUpdaterSpec(
+                wsproxy_addr=wsproxy_addr if wsproxy_addr is not None else TriState.nop(),
+                wsproxy_api_token=(
+                    wsproxy_api_token if wsproxy_api_token is not None else TriState.nop()
+                ),
+                use_host_network=(
+                    use_host_network if use_host_network is not None else OptionalState.nop()
+                ),
+            )
+
+        driver_spec: ScalingGroupDriverConfigUpdaterSpec | None = None
+        if driver is not None or driver_opts is not None:
+            driver_spec = ScalingGroupDriverConfigUpdaterSpec(
+                driver=driver if driver is not None else OptionalState.nop(),
+                driver_opts=driver_opts if driver_opts is not None else OptionalState.nop(),
+            )
+
+        scheduler_spec: ScalingGroupSchedulerConfigUpdaterSpec | None = None
+        if scheduler is not None or scheduler_opts is not None:
+            scheduler_spec = ScalingGroupSchedulerConfigUpdaterSpec(
+                scheduler=scheduler if scheduler is not None else OptionalState.nop(),
+                scheduler_opts=scheduler_opts
+                if scheduler_opts is not None
+                else OptionalState.nop(),
+            )
+
+        spec = ScalingGroupUpdaterSpec(
+            status=status_spec,
+            metadata=metadata_spec,
+            network=network_spec,
+            driver=driver_spec,
+            scheduler=scheduler_spec,
+        )
+        return Updater(spec=spec, pk_value=name)


I don’t think we need this method.

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

+                description=TriState.update("Updated description"),
+            ),
+            network=ScalingGroupNetworkConfigUpdaterSpec(
+                wsproxy_addr=TriState.update("http://new-wsproxy:5000"),


github-actions bot assigned jopemachine Dec 19, 2025

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

jopemachine added this to the 25.19 milestone Dec 19, 2025

jopemachine changed the title ~~feat(BA-3486): Implement ModifyScalingGroup Action~~ feat(BA-3486): Implement ModifyScalingGroup Action Dec 19, 2025

github-advanced-security bot found potential problems Dec 19, 2025

View reviewed changes

jopemachine force-pushed the feat/BA-3486 branch from 06dee4b to 92db966 Compare December 31, 2025 01:54

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

github-advanced-security bot found potential problems Dec 31, 2025

View reviewed changes

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

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

Copilot AI review requested due to automatic review settings December 31, 2025 02:33

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

Copilot AI reviewed Dec 31, 2025

View reviewed changes

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

jopemachine added 10 commits January 5, 2026 08:02

feat: Implement ModifyScalingGroup Action

a6a6888

test: Update

03a24f0

docs: Add news fragment

6460faf

test: fix merge conflict

ec98408

misc: fix merge conflict

7aa2de0

test: fix broken CI

b1c5424

misc: Remove useless option

7eab3a4

test: Add more test

6ca24a4

misc: Improve logging

02a34e2

fix: Resolve Broken tests

bb08812

jopemachine force-pushed the feat/BA-3486 branch from d97146e to bb08812 Compare January 5, 2026 08:11

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

seedspirit reviewed Jan 5, 2026

View reviewed changes

HyeockJinKim requested changes Jan 6, 2026

View reviewed changes

src/ai/backend/manager/repositories/scaling_group/updaters.py Outdated Show resolved Hide resolved

feat: Split large updater into multiple updater

25c1e0d

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

jopemachine requested review from HyeockJinKim and seedspirit January 6, 2026 07:31

HyeockJinKim requested changes Jan 6, 2026

View reviewed changes

test: Remove useless _create_scaling_group_updater

196c0fc

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

github-advanced-security bot found potential problems Jan 6, 2026

View reviewed changes

HyeockJinKim approved these changes Jan 6, 2026

View reviewed changes

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

Merged via the queue into main with commit 7e7a21b Jan 6, 2026
28 checks passed

HyeockJinKim deleted the feat/BA-3486 branch January 6, 2026 08:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(BA-3486): Implement `ModifyScalingGroup` Action #7522

feat(BA-3486): Implement `ModifyScalingGroup` Action #7522

jopemachine commented Dec 19, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check warning

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Dec 31, 2025

Uh oh!

Uh oh!

seedspirit Jan 5, 2026

Uh oh!

jopemachine Jan 5, 2026 •

edited

Loading

Uh oh!

Uh oh!

HyeockJinKim Jan 6, 2026

Uh oh!

Check warning

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

feat(BA-3486): Implement ModifyScalingGroup Action #7522

feat(BA-3486): Implement ModifyScalingGroup Action #7522

Conversation

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

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check warning

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Dec 31, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

seedspirit Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

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

HyeockJinKim Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Check warning

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

feat(BA-3486): Implement `ModifyScalingGroup` Action #7522

feat(BA-3486): Implement `ModifyScalingGroup` Action #7522

jopemachine commented Dec 19, 2025 •

edited

Loading

jopemachine Jan 5, 2026 •

edited

Loading