Skip to content
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

[Serve] Enable serve status when proxies disabled #42286

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions python/ray/serve/_private/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
get_all_live_placement_group_names,
get_head_node_id,
)
from ray.serve.config import HTTPOptions, gRPCOptions
from ray.serve.config import HTTPOptions, ProxyLocation, gRPCOptions
from ray.serve.generated.serve_pb2 import (
ActorNameList,
DeploymentArgs,
Expand Down Expand Up @@ -929,10 +929,15 @@ def get_serve_instance_details(self) -> Dict:
# Eventually we want to remove route_prefix from DeploymentSchema.
http_options = HTTPOptionsSchema.parse_obj(http_config.dict(exclude_unset=True))
grpc_options = gRPCOptionsSchema.parse_obj(grpc_config.dict(exclude_unset=True))
proxy_location = (
GeneDer marked this conversation as resolved.
Show resolved Hide resolved
None
if http_config.location is None
else ProxyLocation._from_deployment_mode(http_config.location)
GeneDer marked this conversation as resolved.
Show resolved Hide resolved
)
return ServeInstanceDetails(
target_capacity=self._target_capacity,
controller_info=self._actor_details,
proxy_location=http_config.location,
proxy_location=proxy_location,
http_options=http_options,
grpc_options=grpc_options,
proxies=self.proxy_state_manager.get_proxy_details()
Expand Down
37 changes: 28 additions & 9 deletions python/ray/serve/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,17 +150,36 @@ class ProxyLocation(str, Enum):
EveryNode = "EveryNode"

@classmethod
def _to_deployment_mode(cls, v: Union["ProxyLocation", str]) -> DeploymentMode:
if not isinstance(v, (cls, str)):
raise TypeError(f"Must be a `ProxyLocation` or str, got: {type(v)}.")
elif v == ProxyLocation.Disabled:
def _to_deployment_mode(
cls, proxy_location: Union["ProxyLocation", str]
) -> DeploymentMode:
if isinstance(proxy_location, str):
proxy_location = ProxyLocation(proxy_location)
elif not isinstance(proxy_location, ProxyLocation):
raise TypeError(
f"Must be a `ProxyLocation` or str, got: {type(proxy_location)}."
)

if proxy_location == ProxyLocation.Disabled:
return DeploymentMode.NoServer
elif v == ProxyLocation.HeadOnly:
return DeploymentMode.HeadOnly
elif v == ProxyLocation.EveryNode:
return DeploymentMode.EveryNode
else:
raise ValueError(f"Unrecognized `ProxyLocation`: {v}.")
return DeploymentMode(proxy_location.value)
Comment on lines +153 to +166
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should not change behavior. I refactored this for simplicity's sake.


@classmethod
def _from_deployment_mode(
cls, deployment_mode: Union[DeploymentMode, str]
) -> "ProxyLocation":
if isinstance(deployment_mode, str):
deployment_mode = DeploymentMode(deployment_mode)
elif not isinstance(deployment_mode, DeploymentMode):
raise TypeError(
f"Must be a `DeploymentMode` or str, got: {type(deployment_mode)}."
)

if deployment_mode == DeploymentMode.NoServer:
return ProxyLocation.Disabled
else:
return ProxyLocation(deployment_mode.value)


@PublicAPI(stability="stable")
Expand Down
25 changes: 25 additions & 0 deletions python/ray/serve/tests/unit/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,31 @@ def test_proxy_location_to_deployment_mode():
ProxyLocation._to_deployment_mode({"some_other_obj"})


def test_deployment_mode_to_proxy_location():
assert (
ProxyLocation._from_deployment_mode(DeploymentMode.NoServer)
== ProxyLocation.Disabled
)
assert (
ProxyLocation._from_deployment_mode(DeploymentMode.HeadOnly)
== ProxyLocation.HeadOnly
)
assert (
ProxyLocation._from_deployment_mode(DeploymentMode.EveryNode)
== ProxyLocation.EveryNode
)

assert ProxyLocation._from_deployment_mode("NoServer") == ProxyLocation.Disabled
assert ProxyLocation._from_deployment_mode("HeadOnly") == ProxyLocation.HeadOnly
assert ProxyLocation._from_deployment_mode("EveryNode") == ProxyLocation.EveryNode

with pytest.raises(ValueError):
ProxyLocation._from_deployment_mode("Unknown")
GeneDer marked this conversation as resolved.
Show resolved Hide resolved

with pytest.raises(TypeError):
ProxyLocation._from_deployment_mode({"some_other_obj"})


@pytest.mark.parametrize(
"policy", [None, fake_policy, "ray.serve.tests.unit.test_config:fake_policy"]
)
Expand Down
Loading