[serve] add max_replicas_per_node to /api/serve/applications response#63234
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements the propagation of the max_replicas_per_node field from ReplicaConfig to DeploymentSchema in the _deployment_info_to_schema function, supported by new unit tests. Feedback was provided regarding the handling of None values; the current implementation omits the field when it is None, but if the API needs to explicitly return null to indicate no limit, the type hint should be updated to Optional[int] and the assignment should be made unconditional.
| if info.replica_config.max_replicas_per_node is not None: | ||
| schema.max_replicas_per_node = info.replica_config.max_replicas_per_node |
There was a problem hiding this comment.
The max_replicas_per_node field in DeploymentSchema is defined as an int (line 433), but its description states that None is a valid value for the default (no limit). If info.replica_config.max_replicas_per_node is None, this implementation skips setting the field, leaving it as DEFAULT.VALUE. While this correctly omits the field from the API response when unset, it also means the API can never explicitly return null for this field. If the intention is to allow an explicit null in the response to indicate "no limit", the type hint in DeploymentSchema should be updated to Optional[int] and this assignment should be made unconditional.
Signed-off-by: akyang-anyscale <alexyang@anyscale.com>
akyang-anyscale
force-pushed
the
alexyang/schema-max-per-node
branch
from
d0f9f8b to
df15b79
Compare
|
please add some description to this PR. What architectural defect resulted in this situation seems a bit odd that the context of |
not sure if it's an architectural defect rather than some fields are just omitted today. |
…ray-project#63234) when constructing the `DeploymentSchema` from the `DeploymentInfo`, we were excluding some fields including `max_replicas_per_node` which may be helpful for debugging. this PR adds that field in. Signed-off-by: akyang-anyscale <alexyang@anyscale.com>
…ray-project#63234) when constructing the `DeploymentSchema` from the `DeploymentInfo`, we were excluding some fields including `max_replicas_per_node` which may be helpful for debugging. this PR adds that field in. Signed-off-by: akyang-anyscale <alexyang@anyscale.com>
…ray-project#63234) when constructing the `DeploymentSchema` from the `DeploymentInfo`, we were excluding some fields including `max_replicas_per_node` which may be helpful for debugging. this PR adds that field in. Signed-off-by: akyang-anyscale <alexyang@anyscale.com> Signed-off-by: anindyam1969 <amukherjee@kinetica.com>
…ray-project#63234) when constructing the `DeploymentSchema` from the `DeploymentInfo`, we were excluding some fields including `max_replicas_per_node` which may be helpful for debugging. this PR adds that field in. Signed-off-by: akyang-anyscale <alexyang@anyscale.com>
2 participants
when constructing the
DeploymentSchemafrom theDeploymentInfo, we were excluding some fields includingmax_replicas_per_nodewhich may be helpful for debugging. this PR adds that field in.