Skip to content

Conversation

@fregataa
Copy link
Member

@fregataa fregataa commented Oct 27, 2025

resolves #6417 (BA-2839)

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

@fregataa fregataa added this to the 25.16 milestone Oct 27, 2025
@fregataa fregataa requested a review from HyeockJinKim October 27, 2025 03:56
@fregataa fregataa self-assigned this Oct 27, 2025
@github-actions github-actions bot added size:M 30~100 LoC comp:manager Related to Manager component labels Oct 27, 2025
Comment on lines 25 to 26
scope_id: ScopeId
entity_id: ObjectId
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's easy to miss this, so it would be good to apply the matters related to scope and entity as abstractmethod.

@fregataa fregataa marked this pull request as ready for review October 27, 2025 11:11
Copilot AI review requested due to automatic review settings October 27, 2025 11:11
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 an RBAC (Role-Based Access Control) entity creator that manages the association between scopes and entities in the system. The implementation provides a reusable base class for creating RBAC-related database entries with proper conflict handling.

  • Introduces RBACCreator as an abstract base class for RBAC entity creation operations
  • Implements automatic scope-entity association creation with conflict resolution
  • Adds logging for integrity constraint violations during RBAC row creation

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

Comment on lines +40 to +51
@abstractmethod
def scope_id(self) -> ScopeId:
raise NotImplementedError

@abstractmethod
def object_id(self) -> ObjectId:
raise NotImplementedError


TCreatedEntity = TypeVar("TCreatedEntity")


Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

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

The IntegrityError exception is caught after .on_conflict_do_nothing() is used, making this catch block unreachable. The on_conflict_do_nothing() clause prevents IntegrityError from being raised for conflicts. Remove the try-except block or remove the on_conflict_do_nothing() clause if you need to handle conflicts explicitly.

Suggested change
@abstractmethod
def scope_id(self) -> ScopeId:
raise NotImplementedError
@abstractmethod
def object_id(self) -> ObjectId:
raise NotImplementedError
TCreatedEntity = TypeVar("TCreatedEntity")
await db_session.execute(
pg_insert(AssociationScopesEntitiesRow)
.values(creator.fields_to_store())
.on_conflict_do_nothing()
)

Copilot uses AI. Check for mistakes.
@fregataa fregataa requested a review from HyeockJinKim October 27, 2025 11:43
@fregataa fregataa marked this pull request as draft October 27, 2025 13:00
@github-actions github-actions bot added size:L 100~500 LoC comp:agent Related to Agent component and removed size:M 30~100 LoC labels Oct 28, 2025
@fregataa fregataa marked this pull request as ready for review October 29, 2025 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:agent Related to Agent component comp:manager Related to Manager component size:L 100~500 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement RBAC Creator pattern

3 participants