Skip to content

Conversation

@jopemachine
Copy link
Member

@jopemachine jopemachine commented Jan 26, 2026

resolves #8248 (BA-4036)

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 AI review requested due to automatic review settings January 26, 2026 04:14
@github-actions github-actions bot added the size:L 100~500 LoC label Jan 26, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces BEP-1038, which defines the implementation plan for ImageV2 GraphQL types as part of the Strawberry GraphQL migration (BEP-1010). The proposal provides a comprehensive specification for implementing image-related GraphQL types with improved structure, RBAC integration, and cleaner separation of concerns.

Changes:

  • Added main BEP document defining the implementation strategy for ImageV2 GQL types
  • Added detailed type specifications document listing all types to be implemented
  • Added document identifying legacy types to skip
  • Added document specifying fields/types to defer until related Node types are implemented

Reviewed changes

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

File Description
proposals/BEP-1038-image-v2-gql-implementation.md Main BEP document providing overview, type structure hierarchy, and implementation checklist
proposals/BEP-1038/types-to-include.md Detailed specifications of all types to implement, including sub-info types, info types, and main types with field definitions
proposals/BEP-1038/types-to-skip.md Documents legacy Image type that should not be implemented, with justification
proposals/BEP-1038/types-to-defer.md Specifies registry field that should remain as primitive string until ContainerRegistryNode is implemented

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

row: ImageRow,
*,
permissions: list[ImagePermission] | None = None,
) -> Self:
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of Self type hint (line 107) requires Python 3.11+, but BEP-1010 specifies Python 3.9+ as the runtime environment. For Python 3.9-3.10 compatibility, consider using from typing import Self (available via typing_extensions) or using a string literal "ImageV2GQL" as the return type annotation.

Suggested change
) -> Self:
) -> "ImageV2GQL":

Copilot uses AI. Check for mistakes.
---

# BEP-1038: ImageV2 GQL Implementation

Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the BEP Writing Guide (proposals/README.md line 58), BEP documents should include a "Related Issues" section that links to JIRA/GitHub issues. The PR description mentions BA-4036, but the BEP document itself lacks a "Related Issues" section. Consider adding this section to align with the BEP guidelines.

Suggested change
## Related Issues
- [BA-4036](https://jira.backend.ai/browse/BA-4036)

Copilot uses AI. Check for mistakes.
class ImageResourceLimitGQL:
key: str
min: str
max: str | None
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type hints use PEP 604 union syntax (str | None) and PEP 585 generics (list[str]), which require Python 3.10+. However, BEP-1010 specifies Python 3.9+ as the runtime environment. For Python 3.9 compatibility, use Optional[str] from typing instead of str | None, and List[str] instead of list[str]. If the project has moved to Python 3.10+ as the minimum version, this should be documented in BEP-1010.

Copilot uses AI. Check for mistakes.
## References

- [BEP-1010: GraphQL API Migration to Strawberry](BEP-1010-new-gql.md)
- [BEP-1034: KernelV2 GQL Implementation](BEP-1034-kernel-v2-gql-implementation.md)
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reference to BEP-1034 points to a non-existent file. While BEP-1034 is listed in the proposals/README.md, the file proposals/BEP-1034-kernel-v2-gql-implementation.md does not exist yet. Consider either removing this reference until BEP-1034 is created, or ensuring the referenced file exists before this PR is merged.

Suggested change
- [BEP-1034: KernelV2 GQL Implementation](BEP-1034-kernel-v2-gql-implementation.md)

Copilot uses AI. Check for mistakes.
@jopemachine jopemachine added this to the 26.2 milestone Jan 26, 2026
@jopemachine jopemachine changed the title doc(BA-4036): Add BEP-1038 for ImageV2 GQL Implementation doc(BA-4036): Add BEP-1038 for ImageV2 GQL Implementation Jan 26, 2026
Copy link
Collaborator

@HyeockJinKim HyeockJinKim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider the alias part later and start working. @jopemachine

│ ├── canonical_name
│ ├── namespace
│ ├── architecture
│ └── aliases
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned yesterday, aliases should also be passed as direct fields of ImageV2 via connections or nodes.

@HyeockJinKim HyeockJinKim added this pull request to the merge queue Jan 27, 2026
Merged via the queue into main with commit d5692bf Jan 27, 2026
30 checks passed
@HyeockJinKim HyeockJinKim deleted the BA-4036 branch January 27, 2026 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100~500 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Write BEP for ImageV2 GQL Implementation

3 participants