refactor(BA-4011): Remove TODOs in `manager/api/resource.py` #8219

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

resolves #8218 (BA-4011)

Remove Legacy TODOs before migrating resource.py

for validator, removed try-catch logic which is only for catching errors for not implemented scailing param validation.
which is now replaced with dict type enforcement.
As agent_watch function has no relation with resource.py and is only reffered from vfolder moved into vfolder with added description for future plan.

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

📚 Documentation preview 📚: https://sorna--8219.org.readthedocs.build/en/8219/

📚 Documentation preview 📚: https://sorna-ko--8219.org.readthedocs.build/ko/8219/

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

+    )
+    watcher_port = 6099 if raw_watcher_port is None else int(raw_watcher_port)
+    # TODO: watcher scheme is assumed to be http
+    addr = yarl.URL(f"http://{agent_ip}:{watcher_port}")


Copilot

Pull request overview

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

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

Copilot · 2026-01-22T09:04:32Z

changes/8219.enhance.md

@@ -0,0 +1 @@
+For resource handler file in manager api, remove TODos with implementation


This news fragment doesn’t follow the repository guideline of being a single well-formed English sentence (see changes/README.md:30), and it contains typos (e.g., TODos). Please rephrase to a clear one-liner (imperative form) and fix spelling/capitalization.

Suggested change

For resource handler file in manager api, remove TODos with implementation

Implement all remaining TODOs in the resource handler file in the Manager API.

Copilot · 2026-01-22T09:04:33Z

docs/manager/rest-reference/openapi.json

                  }
                },
                "required": [
+                  "scaling_group",


The OpenAPI schema marks scaling_group as required, but check-presets has historically accepted requests without it (the client SDK omits it when unset). If the API should remain backward-compatible, update this required list to not require scaling_group (and keep server-side validation consistent).

Suggested change

"scaling_group",

Copilot · 2026-01-22T09:04:33Z

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

+    """
+    Get watcher information.
+
+    :return addr: address of agent watcher (eg: http://127.0.0.1:6009)


The docstring example uses port 6009, but the default watcher port in this function is 6099. Please align the example with the actual default to avoid confusion during ops/debugging.

Suggested change

:return addr: address of agent watcher (eg: http://127.0.0.1:6009)

:return addr: address of agent watcher (eg: http://127.0.0.1:6099)

HyeockJinKim · 2026-01-23T01:27:05Z

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

 @check_api_params(
    t.Dict({
-        t.Key("scaling_group", default=None): t.Null | t.String,
+        t.Key("scaling_group"): t.String,


Modifying the existing schema doesn’t look ideal.
Although there’s a desire to enforce stricter value constraints, it’s not something we can change in the current API.
Introducing a new API for this purpose would be a better option.

Understood
I will look for better way to handle it, while retainig a exsiting api handlers.

Another small question here:

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

Line 388 in 681d882

cors.add(add_route("POST", "/check-presets", check_presets))

is being handled as "POST", although it doesn't effects current state,
best practice i think would be "GET" with parameter querier.

Current plan is adding new handler with "GET" with new function which we can use while transition.

- Add check_presets (GET) with required scaling_group - Keep check_presets_legacy (POST) for backward compatibility - Move get_watcher_info to vfolder.py Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Co-authored-by: octodog <mu001@lablup.com>

fregataa · 2026-01-26T06:25:19Z

docs/manager/rest-reference/openapi.json

    },
    "/resource/check-presets": {
-      "post": {
+      "get": {


Could you check why the open api schema generator changed this?

c229676#diff-51adf4e1db91582bd045aea8a34e9565383e2c437c8cf7aa9eab64aebbdf9b3cR415-R416

- cors.add(add_route("POST", "/check-presets", check_presets)) + cors.add(add_route("GET", "/check-presets", check_presets)) + cors.add(add_route("POST", "/check-presets", check_presets_legacy))

I guess that modifcation of OpenAPI would occured from here.
With intention of soft migration I've added Get handler for legacy Post handler which doesn't make sense As it doesn't modify any state of program.

but i still need to investigate why "post" were deleted instead of coexistencing.

HyeockJinKim

I wasn't suggesting creating a new API for the check preset right now.
If we did that, it would require corresponding changes to the web UI, and if we're going to create a new API, it seems better to design it from scratch and implement it.

kwonkwonn · 2026-01-26T12:27:21Z

that make sense.

I'll just add some comments about discussions we've made and leave it for now.
So we can refer it next time when we can clean this stuff.

Copilot AI review requested due to automatic review settings January 22, 2026 08:37

github-actions bot assigned kwonkwonn Jan 22, 2026

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

Copilot started reviewing on behalf of kwonkwonn January 22, 2026 08:37 View session

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

View reviewed changes

This comment was marked as outdated.

Sign in to view

kwonkwonn force-pushed the refactor/remove-ToDos-resource branch from 60465ae to 9d8efc9 Compare January 22, 2026 08:49

github-actions bot added size:M 30~100 LoC and removed size:XL 500~ LoC labels Jan 22, 2026

kwonkwonn changed the title ~~refactor(BA-4011): Add pydantic DTOs for group registry quota API~~ refactor(BA-4011): Remove TODOs in manager/api/resource.py Jan 22, 2026

github-actions bot added the area:docs Documentations label Jan 22, 2026

kwonkwonn requested a review from Copilot January 22, 2026 08:56

Copilot started reviewing on behalf of kwonkwonn January 22, 2026 08:56 View session

Copilot AI reviewed Jan 22, 2026

View reviewed changes

HyeockJinKim requested changes Jan 23, 2026

View reviewed changes

refactor: implement TODOs in resource handler and add GET check-presets

c229676

- Add check_presets (GET) with required scaling_group - Keep check_presets_legacy (POST) for backward compatibility - Move get_watcher_info to vfolder.py Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

kwonkwonn force-pushed the refactor/remove-ToDos-resource branch from fdee337 to c229676 Compare January 23, 2026 03:26

github-actions bot added size:L 100~500 LoC and removed size:M 30~100 LoC labels Jan 23, 2026

kwonkwonn requested review from HyeockJinKim and fregataa January 23, 2026 03:27

chore: update api schema dump

0dca2d0

Co-authored-by: octodog <mu001@lablup.com>

fregataa reviewed Jan 26, 2026

View reviewed changes

HyeockJinKim requested changes Jan 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(BA-4011): Remove TODOs in `manager/api/resource.py` #8219

refactor(BA-4011): Remove TODOs in `manager/api/resource.py` #8219

Uh oh!

kwonkwonn commented Jan 22, 2026 •

edited by github-actions bot

Loading

Uh oh!

Check warning

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 22, 2026

Uh oh!

Copilot AI Jan 22, 2026

Uh oh!

Copilot AI Jan 22, 2026

Uh oh!

HyeockJinKim Jan 23, 2026

Uh oh!

kwonkwonn Jan 23, 2026 •

edited

Loading

Uh oh!

fregataa Jan 26, 2026

Uh oh!

kwonkwonn Jan 26, 2026 •

edited

Loading

Uh oh!

HyeockJinKim left a comment

Uh oh!

kwonkwonn commented Jan 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		@@ -0,0 +1 @@
		For resource handler file in manager api, remove TODos with implementation No newline at end of file

	For resource handler file in manager api, remove TODos with implementation
	Implement all remaining TODOs in the resource handler file in the Manager API.

	:return addr: address of agent watcher (eg: http://127.0.0.1:6009)
	:return addr: address of agent watcher (eg: http://127.0.0.1:6099)

refactor(BA-4011): Remove TODOs in manager/api/resource.py #8219

Are you sure you want to change the base?

refactor(BA-4011): Remove TODOs in manager/api/resource.py #8219

Uh oh!

Conversation

kwonkwonn commented Jan 22, 2026 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Check warning

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

HyeockJinKim Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

kwonkwonn Jan 23, 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!

fregataa Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

kwonkwonn Jan 26, 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!

HyeockJinKim left a comment

Choose a reason for hiding this comment

Uh oh!

kwonkwonn commented Jan 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

refactor(BA-4011): Remove TODOs in `manager/api/resource.py` #8219

refactor(BA-4011): Remove TODOs in `manager/api/resource.py` #8219

kwonkwonn commented Jan 22, 2026 •

edited by github-actions bot

Loading

kwonkwonn Jan 23, 2026 •

edited

Loading

kwonkwonn Jan 26, 2026 •

edited

Loading