-
Notifications
You must be signed in to change notification settings - Fork 164
feat(BA-4117): Add Resource group scoped fair share query APIs #8504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Implement resource group scoped fair share query APIs for domain,
project, and user levels:
GraphQL:
- rg_domain_fair_share, rg_domain_fair_shares
- rg_project_fair_share, rg_project_fair_shares
- rg_user_fair_share, rg_user_fair_shares
REST API:
- GET/POST /rg/{resource_group}/domains/{domain_name}
- GET/POST /rg/{resource_group}/domains/{domain_name}/projects/{project_id}
- GET/POST /rg/{resource_group}/domains/{domain_name}/projects/{project_id}/users/{user_uuid}
Client SDK & CLI:
- rg_get_domain_fair_share, rg_search_domain_fair_shares
- rg_get_project_fair_share, rg_search_project_fair_shares
- rg_get_user_fair_share, rg_search_user_fair_shares
- fair-share rg-domain/rg-project/rg-user commands
Also includes unit tests for GraphQL resolvers and REST handlers.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-authored-by: octodog <mu001@lablup.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds resource-group (RG) scoped fair-share query APIs across GraphQL, REST, client SDK, and CLI, with accompanying schema/docs updates and unit tests.
Changes:
- Implemented GraphQL RG-scoped resolvers for domain/project/user fair-share reads and listings.
- Added REST endpoints for RG-scoped get/search operations and wired them into the app router + OpenAPI.
- Extended client SDK + CLI with RG-scoped fair-share commands and added unit tests.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/manager/api/gql/fair_share/test_rg_user_resolver.py | Adds unit tests for rgUserFairShare / rgUserFairShares GraphQL resolvers |
| tests/unit/manager/api/gql/fair_share/test_rg_project_resolver.py | Adds unit tests for rgProjectFairShare / rgProjectFairShares GraphQL resolvers |
| tests/unit/manager/api/gql/fair_share/test_rg_domain_resolver.py | Adds unit tests for rgDomainFairShare / rgDomainFairShares GraphQL resolvers |
| tests/unit/manager/api/fair_share/test_rg_handler.py | Adds unit tests intended to cover RG-scoped REST handlers |
| tests/unit/manager/api/fair_share/BUILD | Registers fair-share REST unit tests in build config |
| src/ai/backend/manager/api/gql/fair_share/resolver/user.py | Implements RG-scoped user fair-share GraphQL resolvers |
| src/ai/backend/manager/api/gql/fair_share/resolver/project.py | Implements RG-scoped project fair-share GraphQL resolvers |
| src/ai/backend/manager/api/gql/fair_share/resolver/domain.py | Implements RG-scoped domain fair-share GraphQL resolvers |
| src/ai/backend/manager/api/gql/fair_share/fetcher/domain.py | Adds RG-scoped domain fair-share fetcher |
| src/ai/backend/manager/api/gql/fair_share/fetcher/init.py | Exposes new RG-scoped domain fetcher |
| src/ai/backend/manager/api/fair_share/handler.py | Adds RG-scoped REST handlers + routes |
| src/ai/backend/common/dto/manager/fair_share/request.py | Introduces RG-scoped path parameter models |
| src/ai/backend/common/dto/manager/fair_share/init.py | Exports RG-scoped request models |
| src/ai/backend/client/func/fair_share.py | Adds client SDK functions for RG-scoped get/search calls |
| src/ai/backend/client/cli/fair_share/commands.py | Adds fair-share rg-domain/rg-project/rg-user CLI commands |
| docs/manager/rest-reference/openapi.json | Documents new RG-scoped REST endpoints |
| docs/manager/graphql-reference/v2-schema.graphql | Updates GraphQL schema docs for RG-scoped APIs + nullability changes |
| docs/manager/graphql-reference/supergraph.graphql | Updates federated supergraph schema docs similarly |
| changes/8504.feature.md | Adds changelog entry for RG-scoped fair-share query APIs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| processors = info.context.processors | ||
| action_result = await processors.fair_share.get_domain_fair_share.wait_for_complete( | ||
| GetDomainFairShareAction( | ||
| resource_group=scope.resource_group, | ||
| domain_name=domain_name, | ||
| ) | ||
| ) | ||
|
|
||
| return DomainFairShareGQL.from_dataclass(action_result.data) |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
action_result.data is used unconditionally while the resolver return type is now non-null (DomainFairShareGQL). If wait_for_complete() can still yield data=None (the previous implementation had an explicit None guard), from_dataclass(None) will raise at runtime and violate the GraphQL non-null contract. Either (1) restore a nullable return type and keep the None behavior, or (2) explicitly raise a domain-not-found style exception when action_result.data is None so the non-null schema stays correct.
|
|
||
| return ProjectFairShareGQL.from_dataclass(action_result.data) |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rg_project_fair_share receives scope.domain_name, but the domain is not used to constrain/validate the fetched record. This can produce confusing behavior where the path/scope domain does not match the returned object's domain_name. Consider validating action_result.data.domain_name == scope.domain_name (and raising a not-found error when mismatched), or extend the action/service API to accept the domain as part of the lookup so the scope is actually enforced.
| return ProjectFairShareGQL.from_dataclass(action_result.data) | |
| data = action_result.data | |
| if data.domain_name != scope.domain_name: | |
| raise web.HTTPNotFound() | |
| return ProjectFairShareGQL.from_dataclass(data) |
| processors = info.context.processors | ||
| action_result = await processors.fair_share.get_user_fair_share.wait_for_complete( | ||
| GetUserFairShareAction( | ||
| resource_group=scope.resource_group, | ||
| project_id=uuid.UUID(scope.project_id), | ||
| user_uuid=user_uuid, | ||
| ) | ||
| ) | ||
|
|
||
| return UserFairShareGQL.from_dataclass(action_result.data) |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same pattern as other resolvers: action_result.data is dereferenced unconditionally after changing the return type to non-null (UserFairShareGQL). If the action ever returns data=None, this will raise and become an internal GraphQL error. Prefer an explicit not-found exception (keeping the schema non-null) or revert to a nullable GraphQL return type and keep prior None handling.
| return DomainFairShareConnection( | ||
| edges=edges, | ||
| page_info=PageInfo( | ||
| has_next_page=len(action_result.items) > 0 and (first is not None or limit is not None), | ||
| has_previous_page=(offset or 0) > 0 or last is not None, | ||
| start_cursor=edges[0].cursor if edges else None, | ||
| end_cursor=edges[-1].cursor if edges else None, | ||
| ), | ||
| count=action_result.total_count, | ||
| ) |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
has_next_page can become true on the last page because it only checks that at least one item was returned and that first/limit was provided. This will mislead Relay-style pagination clients. Compute has_next_page based on whether there are more results beyond the current window (e.g., using total_count vs (offset + len(items)) for offset/limit, or by fetching first+1 items and trimming, depending on how PaginationOptions/querier is implemented).
| await processors.fair_share.get_domain_fair_share.wait_for_complete( | ||
| GetDomainFairShareAction( | ||
| resource_group="default", | ||
| domain_name="test-domain", | ||
| ) | ||
| ) | ||
|
|
||
| # Verify the action was called with correct params | ||
| processors.fair_share.get_domain_fair_share.wait_for_complete.assert_called_once() |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests don’t exercise the new REST handler methods (rg_get_* / rg_search_*) at all; they only call the mocked processor method directly, so they would pass even if the handler code/routes were broken. To meaningfully test handler logic, invoke the actual handler (e.g., call FairShareHandler.rg_get_domain_fair_share(...) with PathParam/BodyParam wrappers, or use an aiohttp test client against the registered routes) and assert the resulting APIResponse payload and status code.
| class RGDomainFairSharePathParam(BaseRequestModel): | ||
| """Path parameters for RG-scoped domain fair share get.""" | ||
|
|
||
| resource_group: str = Field(description="Scaling group name") |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The resource_group field description says “Scaling group name”, which is inconsistent with the API naming (“resource group”) and the client/CLI docstrings (“Resource group name”). Update the Field descriptions for RG-scoped path params to use consistent terminology to avoid confusing API consumers.
Implement resource group scoped fair share query APIs for domain, project, and user levels:
GraphQL:
REST API:
Client SDK & CLI:
Also includes unit tests for GraphQL resolvers and REST handlers.
Co-Authored-By: Claude Opus 4.5 noreply@anthropic.com
resolves #NNN (BA-MMM)
Checklist: (if applicable)
ai.backend.testdocsdirectory