-
Notifications
You must be signed in to change notification settings - Fork 163
feat(BA-4325): Apply DataLoaders to fair share entity back-reference resolvers #8702
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -239,7 +239,37 @@ GraphQL Resolver → check_admin_only (if admin) → Processor → Service → R | |||||||||||||||||
| - NEVER put optional fields in Scope - use Filter instead | ||||||||||||||||||
| - Scope fields must all be required (no default values, no Optional types) | ||||||||||||||||||
|
|
||||||||||||||||||
| ### Cross-Entity Reference Resolvers | ||||||||||||||||||
|
|
||||||||||||||||||
| When a GQL node references another entity node, use `strawberry.lazy()` to avoid circular imports. Strawberry requires runtime type resolution, so `TYPE_CHECKING` imports alone are insufficient. | ||||||||||||||||||
|
|
||||||||||||||||||
| **Pattern:** | ||||||||||||||||||
| ```python | ||||||||||||||||||
| # 1. TYPE_CHECKING: for static analysis (mypy) | ||||||||||||||||||
| if TYPE_CHECKING: | ||||||||||||||||||
| from ai.backend.manager.api.gql.domain_v2.types.node import DomainV2GQL | ||||||||||||||||||
|
|
||||||||||||||||||
| # 2. Return type: Annotated with strawberry.lazy() for runtime resolution | ||||||||||||||||||
| # 3. Function body: runtime import + DataLoader | ||||||||||||||||||
| async def domain(self, info: Info[StrawberryGQLContext]) -> Annotated[ | ||||||||||||||||||
| DomainV2GQL, | ||||||||||||||||||
| strawberry.lazy("ai.backend.manager.api.gql.domain_v2.types.node"), | ||||||||||||||||||
| ]: | ||||||||||||||||||
| from ai.backend.manager.api.gql.domain_v2.types.node import DomainV2GQL | ||||||||||||||||||
| data = await info.context.data_loaders.domain_loader.load(self.domain_name) | ||||||||||||||||||
|
Comment on lines
+257
to
+259
|
||||||||||||||||||
| ]: | |
| from ai.backend.manager.api.gql.domain_v2.types.node import DomainV2GQL | |
| data = await info.context.data_loaders.domain_loader.load(self.domain_name) | |
| ] | None: | |
| from ai.backend.manager.api.gql.domain_v2.types.node import DomainV2GQL | |
| data = await info.context.data_loaders.domain_loader.load(self.domain_name) | |
| if data is None: | |
| return None |
Copilot
AI
Feb 9, 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 file paths in this new section are inconsistent with earlier “Key Files” paths (e.g., src/ai/backend/manager/...). api/gql/data_loader/data_loaders.py and the example paths look like they’re missing the src/ai/backend/manager/ prefix; please make the path style consistent so readers can locate the files reliably.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Apply DataLoaders to fair share entity back-reference resolvers |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,7 +12,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from strawberry.relay import Connection, Edge, Node, NodeID | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.api.gql.base import OrderDirection, StringFilter | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.api.gql.types import GQLFilter, GQLOrderBy | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.api.gql.types import GQLFilter, GQLOrderBy, StrawberryGQLContext | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.data.fair_share.types import DomainFairShareData | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.repositories.base import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| QueryCondition, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -65,14 +65,20 @@ class DomainFairShareGQL(Node): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async def domain( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| info: Info, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> Annotated[ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DomainV2GQL, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| strawberry.lazy("ai.backend.manager.api.gql.domain_v2.types.node"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.api.gql.domain_v2.fetcher.domain import fetch_domain | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return await fetch_domain(info=info, domain_name=self.domain_name) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| info: Info[StrawberryGQLContext], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Annotated[ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DomainV2GQL, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| strawberry.lazy("ai.backend.manager.api.gql.domain_v2.types.node"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.api.gql.domain_v2.types.node import DomainV2GQL | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| domain_data = await info.context.data_loaders.domain_loader.load(self.domain_name) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if domain_data is None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+69
to
+80
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> ( | |
| Annotated[ | |
| DomainV2GQL, | |
| strawberry.lazy("ai.backend.manager.api.gql.domain_v2.types.node"), | |
| ] | |
| | None | |
| ): | |
| from ai.backend.manager.api.gql.domain_v2.types.node import DomainV2GQL | |
| domain_data = await info.context.data_loaders.domain_loader.load(self.domain_name) | |
| if domain_data is None: | |
| return None | |
| ) -> Annotated[ | |
| DomainV2GQL, | |
| strawberry.lazy("ai.backend.manager.api.gql.domain_v2.types.node"), | |
| ]: | |
| from ai.backend.manager.api.gql.domain_v2.types.node import DomainV2GQL | |
| domain_data = await info.context.data_loaders.domain_loader.load(self.domain_name) | |
| if domain_data is None: | |
| raise KeyError(f"Domain not found: {self.domain_name}") |
Copilot
AI
Feb 9, 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 domain back-reference was previously non-null and would error if the domain didn’t exist via fetch_domain(). Making it nullable (... | None) changes the GraphQL schema/contract. Keep it non-nullable and map a None DataLoader result to DomainNotFound (or the equivalent error from the old fetcher).
| ) -> ( | |
| Annotated[ | |
| DomainV2GQL, | |
| strawberry.lazy("ai.backend.manager.api.gql.domain_v2.types.node"), | |
| ] | |
| | None | |
| ): | |
| from ai.backend.manager.api.gql.domain_v2.types.node import DomainV2GQL | |
| domain_data = await info.context.data_loaders.domain_loader.load(self.domain_name) | |
| if domain_data is None: | |
| return None | |
| ) -> Annotated[ | |
| DomainV2GQL, | |
| strawberry.lazy("ai.backend.manager.api.gql.domain_v2.types.node"), | |
| ]: | |
| from ai.backend.manager.api.gql.domain_v2.types.node import DomainV2GQL | |
| domain_data = await info.context.data_loaders.domain_loader.load(self.domain_name) | |
| if domain_data is None: | |
| raise RuntimeError("Domain not found") |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,7 +13,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||
| from strawberry.relay import Connection, Edge, Node, NodeID | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.api.gql.base import OrderDirection, StringFilter, UUIDFilter | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.api.gql.types import GQLFilter, GQLOrderBy | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.api.gql.types import GQLFilter, GQLOrderBy, StrawberryGQLContext | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.data.fair_share.types import ProjectFairShareData | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.data.group.types import ProjectType | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.repositories.base import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -68,14 +68,20 @@ class ProjectFairShareGQL(Node): | |||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| async def project( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| self, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| info: Info, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> Annotated[ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ProjectV2GQL, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| strawberry.lazy("ai.backend.manager.api.gql.project_v2.types.node"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.api.gql.project_v2.fetcher.project import fetch_project | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| return await fetch_project(info=info, project_id=self.project_id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| info: Info[StrawberryGQLContext], | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| Annotated[ | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ProjectV2GQL, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| strawberry.lazy("ai.backend.manager.api.gql.project_v2.types.node"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| | None | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.api.gql.project_v2.types.node import ProjectV2GQL | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| project_data = await info.context.data_loaders.project_loader.load(self.project_id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| if project_data is None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| return None | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+72
to
+83
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> ( | |
| Annotated[ | |
| ProjectV2GQL, | |
| strawberry.lazy("ai.backend.manager.api.gql.project_v2.types.node"), | |
| ] | |
| | None | |
| ): | |
| from ai.backend.manager.api.gql.project_v2.types.node import ProjectV2GQL | |
| project_data = await info.context.data_loaders.project_loader.load(self.project_id) | |
| if project_data is None: | |
| return None | |
| ) -> Annotated[ | |
| ProjectV2GQL, | |
| strawberry.lazy("ai.backend.manager.api.gql.project_v2.types.node"), | |
| ]: | |
| from ai.backend.manager.api.gql.project_v2.types.node import ProjectV2GQL | |
| project_data = await info.context.data_loaders.project_loader.load(self.project_id) | |
| if project_data is None: | |
| raise RuntimeError( | |
| f"Project not found for fair share record (project_id={self.project_id})" | |
| ) |
Copilot
AI
Feb 9, 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.
Returning None on missing project data will silently null out the back-reference instead of surfacing an error like the old fetch_project() path. Consider raising the appropriate not-found exception when project_loader.load() returns None to preserve prior semantics.
| return None | |
| raise KeyError(f"Project not found for id {self.project_id}") |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -13,7 +13,7 @@ | |||||||||||||||||||||||||||||||||||
| from strawberry.relay import Connection, Edge, Node, NodeID | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.api.gql.base import OrderDirection, StringFilter, UUIDFilter | ||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.api.gql.types import GQLFilter, GQLOrderBy | ||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.api.gql.types import GQLFilter, GQLOrderBy, StrawberryGQLContext | ||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.data.fair_share.types import UserFairShareData | ||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.repositories.base import ( | ||||||||||||||||||||||||||||||||||||
| QueryCondition, | ||||||||||||||||||||||||||||||||||||
|
|
@@ -68,14 +68,20 @@ class UserFairShareGQL(Node): | |||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
| async def user( | ||||||||||||||||||||||||||||||||||||
| self, | ||||||||||||||||||||||||||||||||||||
| info: Info, | ||||||||||||||||||||||||||||||||||||
| ) -> Annotated[ | ||||||||||||||||||||||||||||||||||||
| UserV2GQL, | ||||||||||||||||||||||||||||||||||||
| strawberry.lazy("ai.backend.manager.api.gql.user_v2.types.node"), | ||||||||||||||||||||||||||||||||||||
| ]: | ||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.api.gql.user_v2.fetcher.user import fetch_user | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| return await fetch_user(info=info, user_uuid=self.user_uuid) | ||||||||||||||||||||||||||||||||||||
| info: Info[StrawberryGQLContext], | ||||||||||||||||||||||||||||||||||||
| ) -> ( | ||||||||||||||||||||||||||||||||||||
| Annotated[ | ||||||||||||||||||||||||||||||||||||
| UserV2GQL, | ||||||||||||||||||||||||||||||||||||
| strawberry.lazy("ai.backend.manager.api.gql.user_v2.types.node"), | ||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||
| | None | ||||||||||||||||||||||||||||||||||||
| ): | ||||||||||||||||||||||||||||||||||||
| from ai.backend.manager.api.gql.user_v2.types.node import UserV2GQL | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| user_data = await info.context.data_loaders.user_loader.load(self.user_uuid) | ||||||||||||||||||||||||||||||||||||
| if user_data is None: | ||||||||||||||||||||||||||||||||||||
| return None | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+77
to
+83
|
||||||||||||||||||||||||||||||||||||
| | None | |
| ): | |
| from ai.backend.manager.api.gql.user_v2.types.node import UserV2GQL | |
| user_data = await info.context.data_loaders.user_loader.load(self.user_uuid) | |
| if user_data is None: | |
| return None | |
| ): | |
| from ai.backend.manager.api.gql.user_v2.types.node import UserNotFound, UserV2GQL | |
| user_data = await info.context.data_loaders.user_loader.load(self.user_uuid) | |
| if user_data is None: | |
| raise UserNotFound(str(self.user_uuid)) |
Copilot
AI
Feb 9, 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.
Returning None when the loader can’t find the user changes behavior compared to the previous fetch_user() call (which raises). If you keep the DataLoader approach, translate None into the appropriate not-found exception so clients get a consistent error instead of a null field.
| return None | |
| raise strawberry.exceptions.GraphQLError( | |
| f"User with UUID {self.user_uuid} not found." | |
| ) |
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.
This section says "TYPE_CHECKING imports alone are insufficient" but earlier in the document it states "NEVER use TYPE_CHECKING for Strawberry types"; these statements conflict, and the codebase uses TYPE_CHECKING for cross-entity node types alongside
strawberry.lazy(). Consider rewording to clarify that TYPE_CHECKING is fine for static typing, but runtime schema generation must rely onstrawberry.lazy()/ forward references (and adjust the earlier rule accordingly).