Skip to content

Commit

Permalink
[Serve] Add default app name (ray-project#34260)
Browse files Browse the repository at this point in the history
Add default name for applications deploy.
Fix a bug for not setting the deployment name correctly for function deployment.
Fix unit tests because of the default application name change.

Signed-off-by: Jack He <[email protected]>
  • Loading branch information
sihanwang41 authored and ProjectsByJackHe committed May 4, 2023
1 parent ef07f0a commit 4e6a968
Show file tree
Hide file tree
Showing 28 changed files with 323 additions and 173 deletions.
2 changes: 1 addition & 1 deletion python/ray/serve/_private/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
DEFAULT_GRPC_PORT = 9000

#: Default Serve application name
SERVE_DEFAULT_APP_NAME = ""
SERVE_DEFAULT_APP_NAME = "default"

#: Separator between app name and deployment name when we prepend
#: the app name to each deployment name. This prepending is currently
Expand Down
8 changes: 0 additions & 8 deletions python/ray/serve/_private/deployment_function_node.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import inspect
from typing import Any, Callable, Dict, List, Union

from ray.dag.dag_node import DAGNode
Expand Down Expand Up @@ -35,13 +34,6 @@ def __init__(
]
deployment_shell = schema_to_deployment(deployment_schema)

# Prefer user specified name to override the generated one.
if (
inspect.isfunction(func_body)
and deployment_shell.name != func_body.__name__
):
self._deployment_name = deployment_shell.name

# Set the route prefix, prefer the one user supplied,
# otherwise set it to /deployment_name
if (
Expand Down
13 changes: 12 additions & 1 deletion python/ray/serve/_private/deployment_graph_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ def build(ray_dag_root_node: DAGNode, name: str = None) -> List[Deployment]:
should be executable via `ray_dag_root_node.execute(user_input)`
and should have `InputNode` in it.
name: Application name,. If provided, formatting all the deployment name to
{name}_{deployment_name}
{name}_{deployment_name}, if not provided, the deployment name won't be
updated.
Returns:
deployments: All deployments needed for an e2e runnable serve pipeline,
Expand Down Expand Up @@ -273,6 +274,16 @@ def replace_with_handle(node):
dag_node._body.__annotations__["return"]
)

# Set the deployment name if the user provides.
if "deployment_schema" in dag_node._bound_other_args_to_resolve:
schema = dag_node._bound_other_args_to_resolve["deployment_schema"]
if (
inspect.isfunction(dag_node._body)
and schema.name != dag_node._body.__name__
):
deployment_name = schema.name

# Update the deployment name if the application name provided.
if name:
deployment_name = name + DEPLOYMENT_NAME_PREFIX_SEPARATOR + deployment_name

Expand Down
5 changes: 3 additions & 2 deletions python/ray/serve/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ def run(


@PublicAPI(stability="alpha")
def build(target: Application, name: str = SERVE_DEFAULT_APP_NAME) -> BuiltApplication:
def build(target: Application, name: str = None) -> BuiltApplication:
"""Builds a Serve application into a static, built application.
Resolves the provided Application object into a list of deployments.
Expand All @@ -562,7 +562,8 @@ def build(target: Application, name: str = SERVE_DEFAULT_APP_NAME) -> BuiltAppli
Args:
target: The Serve application to run consisting of one or more
deployments.
name: The name of the Serve application.
name: The name of the Serve application. When name is not provided, the
deployment name won't be updated. (SINGLE_APP use case.)
Returns:
The static built Serve application.
Expand Down
5 changes: 2 additions & 3 deletions python/ray/serve/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from ray.serve.config import DeploymentMode
from ray.serve._private.utils import DEFAULT, dict_keys_snake_to_camel_case
from ray.util.annotations import DeveloperAPI, PublicAPI
from ray.serve._private.constants import SERVE_DEFAULT_APP_NAME


def _route_prefix_format(cls, v):
Expand Down Expand Up @@ -304,9 +305,7 @@ class ServeApplicationSchema(BaseModel, extra=Extra.forbid):
"""

name: str = Field(
# TODO(cindy): eventually we should set the default app name to a non-empty
# string and forbid empty app names.
default="",
default=SERVE_DEFAULT_APP_NAME,
description=(
"Application name, the name should be unique within the serve instance"
),
Expand Down
3 changes: 2 additions & 1 deletion python/ray/serve/tests/test_advanced.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import ray
from ray import serve
from ray._private.test_utils import SignalActor
from ray.serve._private.constants import SERVE_DEFAULT_APP_NAME


def test_serve_forceful_shutdown(serve_instance):
Expand All @@ -16,7 +17,7 @@ def sleeper():

handle = serve.run(sleeper.bind())
ref = handle.remote()
sleeper.delete()
serve.delete(SERVE_DEFAULT_APP_NAME)

with pytest.raises(ray.exceptions.RayActorError):
ray.get(ref)
Expand Down
30 changes: 8 additions & 22 deletions python/ray/serve/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
from ray.serve.drivers import DAGDriver
from ray.serve.exceptions import RayServeException
from ray.serve._private.api import call_app_builder_with_args_if_necessary
from ray.serve._private.constants import (
SERVE_DEFAULT_APP_NAME,
DEPLOYMENT_NAME_PREFIX_SEPARATOR,
)


@serve.deployment()
Expand Down Expand Up @@ -423,27 +427,6 @@ def __call__(self, *args):
assert ray.get(ingress_handle.remote()) == "got f"


def test_run_delete_old_deployments(serve_instance):
"""Check that serve.run() can remove all old deployments"""

@serve.deployment(name="f", route_prefix="/test1")
def f():
return "got f"

@serve.deployment(name="g", route_prefix="/test2")
def g():
return "got g"

ingress_handle = serve.run(f.bind())
assert ray.get(ingress_handle.remote()) == "got f"

ingress_handle = serve.run(g.bind())
assert ray.get(ingress_handle.remote()) == "got g"

assert "g" in serve.list_deployments()
assert "f" not in serve.list_deployments()


class TestSetOptions:
def test_set_options_basic(self):
@serve.deployment(
Expand Down Expand Up @@ -574,7 +557,10 @@ def g():

serve.run(g.bind())
deployment_info = ray.get(controller._all_running_replicas.remote())
assert "g" in deployment_info
assert (
f"{SERVE_DEFAULT_APP_NAME}{DEPLOYMENT_NAME_PREFIX_SEPARATOR}g"
in deployment_info
)

@serve.deployment
def f():
Expand Down
Loading

0 comments on commit 4e6a968

Please sign in to comment.