Skip to content

Conversation

@seedspirit
Copy link
Contributor

resolves #8468 (BA-4167)

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

@github-actions github-actions bot added size:M 30~100 LoC comp:manager Related to Manager component labels Jan 30, 2026
@seedspirit seedspirit marked this pull request as ready for review January 30, 2026 09:23
Copilot AI review requested due to automatic review settings January 30, 2026 09:23
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

Fixes MissingGreenlet raised by AgentDBSource.get_by_id() when actual_occupied_slots computation touches the kernels relationship in async contexts.

Changes:

  • Switch eager-loading strategy in AgentDBSource.get_by_id() from selectinload to joinedload for AgentRow.kernels.
  • Add unit test coverage to validate get_by_id() can compute actual_occupied_slots without triggering async lazy-loading.
  • Add changelog entry documenting the fix.

Reviewed changes

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

File Description
src/ai/backend/manager/repositories/agent/db_source/db_source.py Changes eager-loading option used by get_by_id() for kernels.
tests/unit/manager/repositories/agent/test_repository.py Adds a new test ensuring get_by_id() loads kernels and computes actual_occupied_slots.
changes/8472.fix.md Adds a changelog note for the MissingGreenlet fix.

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

@seedspirit seedspirit added this to the 25.6 milestone Jan 30, 2026
Comment on lines 917 to 927
async def test_get_by_id_loads_kernels_and_computes_actual_occupied_slots(
self,
db_source: AgentDBSource,
agent_with_kernels: KernelFilteringTestCase,
) -> None:
# Act - This should not raise MissingGreenlet error
agent_data = await db_source.get_by_id(agent_with_kernels.agent_id)

# Assert - Verify actual_occupied_slots is computed correctly
actual_cpu = agent_data.actual_occupied_slots.get("cpu", 0)
assert Decimal(str(actual_cpu)) == agent_with_kernels.expected_actual_occupied_cpu
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to test the repository, but you are only testing the database source.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified it to test the repository. Since test_search_agents_validates_actual_occupied_slots was also testing DBSource, changed it to test the repository as well.

@github-actions github-actions bot added size:L 100~500 LoC and removed size:M 30~100 LoC labels Jan 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AgentDBSource.get_by_id() raises a MissingGreenlet error

3 participants