mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-12 08:42:18 +00:00
fix(model-edit): don't clear the required model field via a null patch
Review follow-up: the generalized null-clear also removed litellm_params.model when a caller sent it as an explicit null, leaving a modelless row that breaks deployment reconstruction. Exempt `model` from the clear so a null there is ignored while other nulled params are still cleared. Trim a redundant UI comment the helper already documents.
This commit is contained in:
parent
fbf6f2dcad
commit
cc11737a5f
3 changed files with 29 additions and 9 deletions
|
|
@ -386,14 +386,16 @@ def update_db_model(db_model: Deployment, updated_patch: updateDeployment) -> Pr
|
|||
#
|
||||
# A litellm_params field the caller explicitly sets to null is removed from the
|
||||
# stored params, so the UI can drop a param (e.g. reasoning_effort) instead of
|
||||
# only overwriting it. The cross-blob mirror stays scoped to SPECIAL_MODEL_INFO_PARAMS
|
||||
# (input/output cost per token/character and cache read/write costs), which
|
||||
# Deployment.__init__ mirrors between litellm_params and model_info. The model_info
|
||||
# loop below stays restricted to SPECIAL params so this path cannot null out
|
||||
# privileged model_info fields like team_id or access groups.
|
||||
# only overwriting it. `model` is exempt: it is required to reconstruct the
|
||||
# deployment, so a null there is ignored rather than persisting a modelless row.
|
||||
# The cross-blob mirror stays scoped to SPECIAL_MODEL_INFO_PARAMS (input/output
|
||||
# cost per token/character and cache read/write costs), which Deployment.__init__
|
||||
# mirrors between litellm_params and model_info. The model_info loop below stays
|
||||
# restricted to SPECIAL params so this path cannot null out privileged model_info
|
||||
# fields like team_id or access groups.
|
||||
if updated_patch.litellm_params:
|
||||
for field in updated_patch.litellm_params.model_fields_set:
|
||||
if getattr(updated_patch.litellm_params, field) is None:
|
||||
if field != "model" and getattr(updated_patch.litellm_params, field) is None:
|
||||
merged_litellm_params.pop(field, None)
|
||||
if field in SPECIAL_MODEL_INFO_PARAMS:
|
||||
merged_model_info.pop(field, None)
|
||||
|
|
|
|||
|
|
@ -3296,6 +3296,25 @@ class TestUpdateDBModelClearParam:
|
|||
info = json.loads(result["model_info"])
|
||||
assert info.get("team_id") == "team-keep-me"
|
||||
|
||||
def test_null_model_field_is_not_cleared(self):
|
||||
"""`model` is required to rebuild the deployment, so an explicit null for it
|
||||
must be ignored, not persisted as a modelless (invalid) row. Other nulled
|
||||
params in the same patch are still cleared."""
|
||||
from litellm.proxy.management_endpoints.model_management_endpoints import (
|
||||
update_db_model,
|
||||
)
|
||||
from litellm.types.router import updateLiteLLMParams
|
||||
|
||||
result = update_db_model(
|
||||
db_model=self._db_model(),
|
||||
updated_patch=updateDeployment(
|
||||
litellm_params=updateLiteLLMParams(model=None, reasoning_effort=None)
|
||||
),
|
||||
)
|
||||
params = json.loads(result["litellm_params"])
|
||||
assert params.get("model") == "gpt-5.6-sol"
|
||||
assert "reasoning_effort" not in params
|
||||
|
||||
|
||||
class TestGetModelInfoWithIdBlocked:
|
||||
"""`ProxyConfig.get_model_info_with_id` must propagate the DB-level `blocked`
|
||||
|
|
|
|||
|
|
@ -504,9 +504,8 @@ export default function ModelInfoView({
|
|||
return;
|
||||
}
|
||||
|
||||
// A param the user deleted from the editor is absent from updatedLitellmParams;
|
||||
// send it as an explicit null so the backend clears it, since the PATCH merge is
|
||||
// additive and would otherwise keep the stored value.
|
||||
// The additive PATCH merge keeps omitted keys, so a deleted param needs an
|
||||
// explicit null to actually clear on the backend.
|
||||
for (const removedKey of computeRemovedLitellmParamKeys(
|
||||
localModelData.litellm_params ?? {},
|
||||
updatedLitellmParams,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue