Skip to content

Conversation

@seedspirit
Copy link
Contributor

resolves #8354 (BA-4108)

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

@seedspirit seedspirit self-assigned this Jan 28, 2026
@github-actions github-actions bot added size:L 100~500 LoC comp:manager Related to Manager component labels Jan 28, 2026
seedspirit and others added 2 commits January 28, 2026 10:22
…ide class

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@seedspirit seedspirit marked this pull request as ready for review January 28, 2026 01:24
Copilot AI review requested due to automatic review settings January 28, 2026 01:24
@seedspirit seedspirit added this to the 26.1 milestone Jan 28, 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 fixes a bug where the initial_delay field in health check configurations was being passed as None when missing from YAML, preventing Pydantic from applying its default value of 60.0. The fix replaces manual object construction with Pydantic's model_validate method, which properly applies field defaults.

Changes:

  • Replaced manual ModelHealthCheck construction with model_validate() to properly handle missing optional fields
  • Added comprehensive test coverage for health check configurations with various field combinations
  • Updated changelog to document the fix

Reviewed changes

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

File Description
src/ai/backend/manager/registry.py Replaced manual construction of ModelHealthCheck with model_validate() to properly apply Pydantic defaults for missing fields
tests/unit/manager/test_registry.py Added comprehensive test suite for health check configurations, including test cases for missing initial_delay field and other optional fields
changes/8389.fix.md Added changelog entry documenting the fix

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

@@ -0,0 +1 @@
Use `model_validate` for health check info to apply Pydantic defaults, preventing pydantic validation error when `initial_delay` field is nullable
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The description says "when initial_delay field is nullable" but this is inaccurate. The issue is that initial_delay is NOT nullable (it's typed as float, not float | None), which is why passing None prevented Pydantic from using its default value. Consider rephrasing to: "preventing pydantic validation error when initial_delay field is missing from the YAML"

Suggested change
Use `model_validate` for health check info to apply Pydantic defaults, preventing pydantic validation error when `initial_delay` field is nullable
Use `model_validate` for health check info to apply Pydantic defaults, preventing pydantic validation error when `initial_delay` field is missing from the YAML

Copilot uses AI. Check for mistakes.
@HyeockJinKim HyeockJinKim added this pull request to the merge queue Jan 28, 2026
Merged via the queue into main with commit 6d5e536 Jan 28, 2026
30 checks passed
@HyeockJinKim HyeockJinKim deleted the fix/BA-4108 branch January 28, 2026 02:17
lablup-octodog pushed a commit that referenced this pull request Jan 28, 2026
…antic defaults (#8389)

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Backported-from: main (26.1)
Backported-to: 26.1
Backport-of: 8389
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.

get_health_check_info passes None to initial_delay when field is missing from model definition YAML

3 participants