-
Notifications
You must be signed in to change notification settings - Fork 177
feat(BA-6242): give custom runtime variant a default health check #11863
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
Merged
+357
−12
Merged
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
01b9bf7
feat(BA-6242): give custom runtime variant a default health check
seedspirit f265132
changelog: add news fragment for PR #11863
seedspirit 567400f
feat(BA-6242): make model-deployment health checks opt-in via enable …
seedspirit f93424a
chore: update api schema dump
seedspirit 6b34a0f
fix(BA-6242): make health-check enable non-nullable in creation input
seedspirit 5176511
chore: update api schema dump
seedspirit 0ac141f
fix(BA-6242): normalize empty health_check in model-definition as exp…
seedspirit 6dddbbe
fix(BA-6242): enable health checks by default for built-in runtime va…
seedspirit File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Add an `enable` flag to model-deployment health checks (default off), making health checks opt-in. The custom runtime variant ships with a disabled default health check, and a model-definition file that declares a health check is treated as enabled. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -170,12 +170,21 @@ class PreStartAction(BaseConfigModel): | |
|
|
||
|
|
||
| class ModelHealthCheck(BaseConfigModel): | ||
| enable: bool = Field( | ||
| default=False, | ||
| description=( | ||
| "Whether the route should be health-checked. When false the route " | ||
| "becomes active immediately and the remaining fields are ignored." | ||
| ), | ||
| examples=[False], | ||
| ) | ||
| interval: float = Field( | ||
| default=10.0, | ||
| description="Interval in seconds between health checks.", | ||
| examples=[10.0], | ||
| ) | ||
| path: str = Field( | ||
| default="/health", | ||
| description="Path to check for health status.", | ||
| examples=["/health"], | ||
| ) | ||
|
|
@@ -351,6 +360,7 @@ def _merge_service_config( | |
| hb, ho = base.health_check, override.health_check | ||
| hs = ho.model_fields_set | ||
| health_check = ModelHealthCheck.model_construct( | ||
| enable=_pick(hb.enable, ho.enable, "enable" in hs), | ||
| interval=_pick(hb.interval, ho.interval, "interval" in hs), | ||
| path=_pick(hb.path, ho.path, "path" in hs), | ||
| max_retries=_pick(hb.max_retries, ho.max_retries, "max_retries" in hs), | ||
|
|
@@ -441,9 +451,8 @@ def merge(self, override: ModelDefinition) -> ModelDefinition: | |
|
|
||
| def health_check_config(self) -> ModelHealthCheck | None: | ||
| for model in self.models: | ||
| if model.service and model.service.health_check: | ||
| if model.service.health_check is not None: | ||
| return model.service.health_check | ||
| if model.service and model.service.health_check and model.service.health_check.enable: | ||
| return model.service.health_check | ||
| return None | ||
|
|
||
| def with_args_appended(self, args: list[str]) -> ModelDefinition: | ||
|
|
@@ -481,6 +490,7 @@ def with_args_appended(self, args: list[str]) -> ModelDefinition: | |
|
|
||
|
|
||
| class ModelHealthCheckDraft(BaseConfigModel): | ||
| enable: bool | None = None | ||
| interval: float | None = None | ||
| path: str | None = None | ||
| max_retries: int | None = None | ||
|
|
@@ -491,8 +501,6 @@ class ModelHealthCheckDraft(BaseConfigModel): | |
| def to_resolved(self) -> ModelHealthCheck: | ||
| # Drop unset (None) fields so the strict type's ``Field(default=...)`` | ||
| # declarations remain the single source of truth for default values. | ||
| # Missing required fields (e.g. ``path``) surface as the strict | ||
| # type's ``BackendAISchemaValidationFailed`` via ``model_validate``. | ||
| return ModelHealthCheck.model_validate(self.model_dump(exclude_none=True)) | ||
|
|
||
|
|
||
|
|
@@ -511,10 +519,9 @@ def _wrap_str_start_command(cls, data: Any) -> Any: | |
| def to_resolved(self) -> ModelServiceConfig: | ||
| # Drop unset (None) scalars so the strict type's ``Field(default=...)`` | ||
| # declarations remain the single source of truth for default values; | ||
| # resolve the nested ``health_check`` draft explicitly so its own | ||
| # required-field check (``path``) fires through its own | ||
| # ``model_validate``. Missing required fields (e.g. ``port``) | ||
| # surface as ``BackendAISchemaValidationFailed``. | ||
| # resolve the nested ``health_check`` draft explicitly. Missing | ||
| # required fields (e.g. ``port``) surface as | ||
| # ``BackendAISchemaValidationFailed``. | ||
| payload = self.model_dump(exclude_none=True, exclude={"health_check"}) | ||
| payload["health_check"] = self.health_check.to_resolved() if self.health_check else None | ||
| return ModelServiceConfig.model_validate(payload) | ||
|
|
@@ -547,6 +554,7 @@ def _merge_health_check_draft( | |
| ) -> ModelHealthCheckDraft: | ||
| s = override.model_fields_set | ||
| return ModelHealthCheckDraft.model_construct( | ||
| enable=_pick(base.enable, override.enable, "enable" in s), | ||
| interval=_pick(base.interval, override.interval, "interval" in s), | ||
| path=_pick(base.path, override.path, "path" in s), | ||
| max_retries=_pick(base.max_retries, override.max_retries, "max_retries" in s), | ||
|
|
@@ -650,6 +658,28 @@ def to_resolved(self) -> ModelDefinition: | |
| "models": [m.to_resolved() for m in (self.models or [])], | ||
| }) | ||
|
|
||
| @classmethod | ||
| def from_file_payload(cls, payload: Mapping[str, Any]) -> ModelDefinitionDraft: | ||
| """Parse a model-definition file into a draft, normalizing the ``health_check`` block.""" | ||
| # Dump by field name so the keys below match regardless of snake/kebab input. | ||
| data = cls.model_validate(dict(payload)).model_dump(exclude_unset=True, by_alias=False) | ||
| for model in data.get("models") or []: | ||
| service = model.get("service") | ||
| if service is None: | ||
| continue | ||
| if "health_check" not in service: | ||
| continue | ||
| health_check = service["health_check"] | ||
| # An empty health_check (null or {}) is an explicit opt-out; disable it so it | ||
| # overrides any enabled baseline instead of inheriting one. | ||
| if not health_check: | ||
| service["health_check"] = {"enable": False} | ||
| continue | ||
| # A non-empty block opts in; default enable to True when unset. | ||
| if health_check.get("enable") is None: | ||
| health_check["enable"] = True | ||
| return cls.model_validate(data) | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this draft resolved? |
||
|
|
||
| def find_config_file(daemon_name: str) -> Path: | ||
| toml_path_from_env = os.environ.get("BACKEND_CONFIG_FILE", None) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.