From cc50e9fd1cdbcbf487405e91a9d1e93500e74638 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 12:01:36 -0800 Subject: [PATCH 01/94] Write import paths in application --- python/ray/serve/application.py | 9 +++++++-- python/ray/serve/pipeline/deployment_node.py | 9 ++------- python/ray/serve/tests/test_application.py | 14 ++++++++++++++ python/ray/serve/utils.py | 9 +++++++++ 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/python/ray/serve/application.py b/python/ray/serve/application.py index 0f666547817d..ead8607f63d5 100644 --- a/python/ray/serve/application.py +++ b/python/ray/serve/application.py @@ -14,7 +14,7 @@ schema_to_serve_application, serve_application_status_to_schema, ) -from ray.serve.utils import logger +from ray.serve.utils import logger, get_deployment_import_path from ray.serve.api import get_deployment_statuses from ray.autoscaler._private.cli_logger import _CliLogger from logging import Logger @@ -62,7 +62,7 @@ def add_deployment(self, deployment: Deployment): "new_deployment" ) - self._deployments[deployment.name] = deployment + self.__setitem__(deployment.name, deployment) def deploy(self, blocking: bool = True): """Atomically deploys the Application's deployments to the Ray cluster. @@ -243,6 +243,9 @@ def __getitem__(self, key: str): def __setitem__(self, key: str, value: Deployment): """Set a deployment by name with dict syntax: app[name]=new_deployment + If the deployment's func_or_class is a function or class (and not an + import path), it overwrites it with that function or class's import path. + Use this to overwrite existing deployments. Args: @@ -258,6 +261,8 @@ def __setitem__(self, key: str, value: Deployment): elif not isinstance(value, Deployment): raise TypeError(f"Got {type(Deployment)} for value. Expected deployment.") + value._func_or_class = get_deployment_import_path(value) + self._deployments[key] = value def __iter__(self): diff --git a/python/ray/serve/pipeline/deployment_node.py b/python/ray/serve/pipeline/deployment_node.py index 26c36d60f950..c156a6b29f43 100644 --- a/python/ray/serve/pipeline/deployment_node.py +++ b/python/ray/serve/pipeline/deployment_node.py @@ -7,6 +7,7 @@ from ray.experimental.dag.constants import DAGNODE_TYPE_KEY from ray.experimental.dag.format_utils import get_dag_node_str from ray.serve.api import Deployment, DeploymentConfig +from ray.serve.utils import get_deployment_import_path class DeploymentNode(DAGNode): @@ -158,13 +159,7 @@ def get_deployment_name(self): return self._deployment.name def get_import_path(self): - if isinstance(self._deployment._func_or_class, str): - # We're processing a deserilized JSON node where import_path - # is dag_node body. - return self._deployment._func_or_class - else: - body = self._deployment._func_or_class.__ray_actor_class__ - return f"{body.__module__}.{body.__qualname__}" + return get_deployment_import_path(self._deployment) def to_json(self, encoder_cls) -> Dict[str, Any]: json_dict = super().to_json_base(encoder_cls, DeploymentNode.__name__) diff --git a/python/ray/serve/tests/test_application.py b/python/ray/serve/tests/test_application.py index 5f321cc01bcc..29287796368a 100644 --- a/python/ray/serve/tests/test_application.py +++ b/python/ray/serve/tests/test_application.py @@ -352,6 +352,20 @@ def test_deploy_from_yaml(self, serve_instance): Application.from_yaml(tmp).to_dict(), app1.to_dict() ) + def test_convert_to_import_path(self, serve_instance): + f = decorated_func.options(name="f") + C = DecoratedClass.options(name="C") + app = Application([f, C]) + + with tempfile.TemporaryFile(mode="w+") as tmp: + app.to_yaml(tmp) + tmp.seek(0) + reconstructed_app = Application.from_yaml(tmp) + + reconstructed_app.deploy() + assert requests.get("http://localhost:8000/f").text == "got decorated func" + assert requests.get("http://localhost:8000/C").text == "got decorated class" + @pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") def test_get_set_item(serve_instance): diff --git a/python/ray/serve/utils.py b/python/ray/serve/utils.py index 3efdee09789b..195124968937 100644 --- a/python/ray/serve/utils.py +++ b/python/ray/serve/utils.py @@ -310,6 +310,15 @@ def msgpack_serialize(obj): return serialized +def get_deployment_import_path(deployment): + if isinstance(deployment._func_or_class, str): + # deployment's func_or_class is already an import path + return deployment._func_or_class + else: + body = deployment._func_or_class.__ray_actor_class__ + return f"{body.__module__}.{body.__qualname__}" + + def parse_import_path(import_path: str): """ Takes in an import_path of form: From f908168a4a332dcc8ce7b4a99b73ac076a37527d Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 12:29:21 -0800 Subject: [PATCH 02/94] Remove __ray_actor_class__ --- python/ray/serve/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ray/serve/utils.py b/python/ray/serve/utils.py index 195124968937..43348c5b6493 100644 --- a/python/ray/serve/utils.py +++ b/python/ray/serve/utils.py @@ -315,7 +315,7 @@ def get_deployment_import_path(deployment): # deployment's func_or_class is already an import path return deployment._func_or_class else: - body = deployment._func_or_class.__ray_actor_class__ + body = deployment._func_or_class return f"{body.__module__}.{body.__qualname__}" From ba85404c3405e2b95628cacbb39b769a07bb4bac Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Thu, 10 Mar 2022 14:30:58 -0600 Subject: [PATCH 03/94] polish helps --- python/ray/serve/scripts.py | 160 +++++++++++------------------------- 1 file changed, 47 insertions(+), 113 deletions(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 6802d8b23cde..15e7e97bd9c5 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -9,7 +9,6 @@ import ray from ray.serve.config import DeploymentMode -from ray._private.utils import import_attr from ray import serve from ray.serve.constants import ( DEFAULT_CHECKPOINT_PATH, @@ -23,6 +22,16 @@ from ray.serve.application import Application from ray.serve.api import Deployment +RAY_INIT_ADDRESS_HELP_STR = ( + "Address to use for ray.init(). Can also be specified " + "using the RAY_ADDRESS environment variable." +) +RAY_DASHBOARD_ADDRESS_HELP_STR = ( + "Address to use to query the Ray dashboard (defaults to " + "http://localhost:8265). Can also be specified using the " + "RAY_ADDRESS environment variable." +) + def _process_args_and_kwargs( args_and_kwargs: Tuple[str], @@ -111,7 +120,7 @@ def cli(): default=os.environ.get("RAY_ADDRESS", "auto"), required=False, type=str, - help='Address of the running Ray cluster to connect to. Defaults to "auto".', + help=RAY_INIT_ADDRESS_HELP_STR, ) @click.option( "--namespace", @@ -181,14 +190,14 @@ def start( ) -@cli.command(help="Shutdown the running Serve instance on the Ray cluster.") +@cli.command(help="Shut down the running Serve instance on the Ray cluster.") @click.option( "--address", "-a", default=os.environ.get("RAY_ADDRESS", "auto"), required=False, type=str, - help='Address of the running Ray cluster to connect to. Defaults to "auto".', + help=RAY_INIT_ADDRESS_HELP_STR, ) @click.option( "--namespace", @@ -208,85 +217,24 @@ def shutdown(address: str, namespace: str): @cli.command( - help=""" -[Experimental] -Create a deployment in running Serve instance. The required argument is the -import path for the deployment: ``my_module.sub_module.file.MyClass``. The -class may or may not be decorated with ``@serve.deployment``. -""", - hidden=True, -) -@click.argument("deployment") -@click.option( - "--address", - "-a", - default=os.environ.get("RAY_ADDRESS", "auto"), - required=False, - type=str, - help='Address of the running Ray cluster to connect to. Defaults to "auto".', -) -@click.option( - "--namespace", - "-n", - default="serve", - required=False, - type=str, - help='Ray namespace to connect to. Defaults to "serve".', -) -@click.option( - "--runtime-env-json", - default=r"{}", - required=False, - type=str, - help=("Runtime environment dictionary to pass into ray.init. Defaults to empty."), -) -@click.option( - "--options-json", - default=r"{}", - required=False, - type=str, - help="JSON string for the deployments options", -) -def create_deployment( - address: str, - namespace: str, - runtime_env_json: str, - deployment: str, - options_json: str, -): - ray.init( - address=address, - namespace=namespace, - runtime_env=json.loads(runtime_env_json), - ) - deployment_cls = import_attr(deployment) - if not isinstance(deployment_cls, Deployment): - deployment_cls = serve.deployment(deployment_cls) - options = json.loads(options_json) - deployment_cls.options(**options).deploy() - - -@cli.command( - short_help="[Experimental] Deploy deployments from a YAML config file.", + short_help="Deploy a Serve app from a YAML config file.", help=( - "Deploys deployment(s) from CONFIG_OR_IMPORT_PATH, which must be either a " - "Serve YAML configuration file path or an import path to " - "a class or function to deploy.\n\n" - "Import paths must be of the form " - '"module.submodule_1...submodule_n.MyClassOrFunction".\n\n' - "Sends a nonblocking request. A successful response only indicates that the " - "request was received successfully. It does not mean the deployments are " - "live. Use `serve info` and `serve status` to check on them. " + "Deploys deployment(s) from a YAML config file.\n\n" + "This call is async; a successful response only indicates that the " + "request was sent to the Ray cluster successfully. It does not mean " + "the the deployments have been deployed/updated.\n\n" + "Use `serve info` to fetch the current config and `serve status` to " + "check the status of the deployments after deploying." ), ) @click.argument("config_file_name") @click.option( "--address", "-a", - default=os.environ.get("RAY_ADDRESS", "http://localhost:8265"), + default=None, required=False, type=str, - help='Address of the Ray dashboard to query. For example, "http://localhost:8265".', + help=RAY_DASHBOARD_ADDRESS_HELP_STR, ) def deploy(config_file_name: str, address: str): @@ -309,21 +257,15 @@ def deploy(config_file_name: str, address: str): @cli.command( - short_help="[Experimental] Run deployments via Serve's Python API.", + short_help="Run a Serve app in a blocking way.", help=( - "Deploys deployment(s) from CONFIG_OR_IMPORT_PATH, which must be either a " - "Serve YAML configuration file path or an import path to " - "a class or function to deploy.\n\n" - "The full command must be of the form:\n" - '"serve run [import path] [optional parameters] -- [arg-1] ... [arg-n] ' - '[kwarg-1]=[kwval-1] ... [kwarg-n]=[kwval-n]"\n\n' - "Deployments via import path may also take in init_args and " - "init_kwargs from any ARGS_AND_KWARGS passed in. Import paths must be " - "of the form:\n" - '"module.submodule_1...submodule_n.MyClassOrFunction".\n\n' - "Blocks after deploying, and logs status periodically. After being killed, " - "this command tears down all deployments it deployed. If there are no " - "deployments left, it also tears down the Serve application." + "Runs the Serve app from the specified YAML config file or import path " + "for a deployment class or function (`my_module.MyClassOrFunction`).\n" + "If deploying via import path, you can pass args and kwargs to the " + "constructor using serve run -- [arg1]...[argn] " + "[--kwarg1=val1]...[--kwargn=valn].\n" + "Blocks after deploying and logs status periodically. If you Ctrl-C " + "this command, it tears down the app." ), ) @click.argument("config_or_import_path") @@ -334,15 +276,15 @@ def deploy(config_file_name: str, address: str): default=None, required=False, help="Path to a local YAML file containing a runtime_env definition. " - "Overrides all runtime_envs specified in a config file.", + "Overrides runtime_envs specified in the config file.", ) @click.option( "--runtime-env-json", type=str, default=None, required=False, - help="JSON-serialized runtime_env dictionary. Overrides all runtime_envs " - "specified in a config file.", + help="JSON-serialized runtime_env dictionary. Overrides runtime_envs " + "specified in the config file.", ) @click.option( "--working-dir", @@ -353,18 +295,16 @@ def deploy(config_file_name: str, address: str): "Directory containing files that your job will run in. Can be a " "local directory or a remote URI to a .zip file (S3, GS, HTTP). " "This overrides the working_dir in --runtime-env if both are " - "specified. Overrides all working_dirs specified in a config file." + "specified. Overrides working_dirs specified in the config file." ), ) @click.option( "--address", "-a", - default="auto", + default=os.environ.get("RAY_ADDRESS", None), required=False, type=str, - help=( - 'Address of the Ray cluster (not the dashboard) to query. Defaults to "auto".' - ), + help=RAY_INIT_ADDRESS_HELP_STR, ) def run( config_or_import_path: str, @@ -374,7 +314,6 @@ def run( working_dir: str, address: str, ): - # Check if path provided is for config or import is_config = pathlib.Path(config_or_import_path).is_file() args, kwargs = _process_args_and_kwargs(args_and_kwargs) @@ -430,11 +369,8 @@ def run( @cli.command( - short_help="[Experimental] Get info about your Serve application's config.", - help=( - "Prints the configurations of all running deployments in the Serve " - "application." - ), + short_help="Get the current config of the running Serve app.", + help=("Prints the configurations of all running deployments in the Serve app."), ) @click.option( "--address", @@ -442,7 +378,7 @@ def run( default=os.environ.get("RAY_ADDRESS", "http://localhost:8265"), required=False, type=str, - help='Address of the Ray dashboard to query. For example, "http://localhost:8265".', + help=RAY_DASHBOARD_ADDRESS_HELP_STR, ) @click.option( "--json_format", @@ -461,13 +397,13 @@ def info(address: str, json_format=bool): @cli.command( - short_help="[Experimental] Get your Serve application's status.", + short_help="Get the current status of the Serve app.", help=( - "Prints status information about all deployments in the Serve application.\n\n" + "Prints status information about all deployments in the Serve app.\n\n" "Deployments may be:\n\n" - "- HEALTHY: all replicas are acting normally and passing their health checks.\n" + "- HEALTHY: all replicas are acting normally and passing their health checks.\n\n" "- UNHEALTHY: at least one replica is not acting normally and may not be " - "passing its health check.\n" + "passing its health check.\n\n" "- UPDATING: the deployment is updating." ), ) @@ -477,7 +413,7 @@ def info(address: str, json_format=bool): default=os.environ.get("RAY_ADDRESS", "http://localhost:8265"), required=False, type=str, - help='Address of the Ray dashboard to query. For example, "http://localhost:8265".', + help=RAY_DASHBOARD_ADDRESS_HELP_STR, ) def status(address: str): @@ -487,10 +423,8 @@ def status(address: str): @cli.command( - short_help=( - "[EXPERIMENTAL] Deletes all running deployments in the Serve application." - ), - help="Deletes all running deployments in the Serve application.", + short_help=("Deletes all running deployments in the Serve app."), + help="Deletes all running deployments in the Serve app.", ) @click.option( "--address", @@ -498,7 +432,7 @@ def status(address: str): default=os.environ.get("RAY_ADDRESS", "http://localhost:8265"), required=False, type=str, - help='Address of the Ray dashboard to query. For example, "http://localhost:8265".', + help=RAY_DASHBOARD_ADDRESS_HELP_STR, ) @click.option("--yes", "-y", is_flag=True, help="Bypass confirmation prompt.") def delete(address: str, yes: bool): From 9fa9f8e8213a8d609f58c9bf404269b01744ca7b Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 12:37:06 -0800 Subject: [PATCH 04/94] Revise get_deployment_import_path code --- python/ray/serve/utils.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/python/ray/serve/utils.py b/python/ray/serve/utils.py index 43348c5b6493..aa82ee8e6d20 100644 --- a/python/ray/serve/utils.py +++ b/python/ray/serve/utils.py @@ -311,11 +311,14 @@ def msgpack_serialize(obj): def get_deployment_import_path(deployment): - if isinstance(deployment._func_or_class, str): + """Gets the import path for deployment's func_or_class.""" + + body = deployment._func_or_class + + if isinstance(body, str): # deployment's func_or_class is already an import path - return deployment._func_or_class + return body else: - body = deployment._func_or_class return f"{body.__module__}.{body.__qualname__}" From 4f098b7f76cbd3ea574f21d59ce534191e9f7908 Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Thu, 10 Mar 2022 14:50:59 -0600 Subject: [PATCH 05/94] remove test --- python/ray/serve/tests/test_cli.py | 43 ------------------------------ 1 file changed, 43 deletions(-) diff --git a/python/ray/serve/tests/test_cli.py b/python/ray/serve/tests/test_cli.py index cdd295825dbf..c28eef2a4502 100644 --- a/python/ray/serve/tests/test_cli.py +++ b/python/ray/serve/tests/test_cli.py @@ -250,49 +250,6 @@ def __call__(self, inp): return (self.value + self.increment - self.decrement) * self.multiplier -@serve.deployment -class DecoratedA(A): - pass - - -@pytest.mark.parametrize("class_name", ["A", "DecoratedA"]) -def test_create_deployment(ray_start_stop, tmp_working_dir, class_name): # noqa: F811 - subprocess.check_output(["serve", "start"]) - subprocess.check_output( - [ - "serve", - "create-deployment", - f"ray.serve.tests.test_cli.{class_name}", - "--runtime-env-json", - json.dumps( - { - "working_dir": tmp_working_dir, - } - ), - "--options-json", - json.dumps( - { - "name": "B", - "init_args": [42], - "init_kwargs": {"increment": 10}, - "num_replicas": 2, - "user_config": {"decrement": 5}, - "ray_actor_options": { - "runtime_env": { - "env_vars": { - "SERVE_TEST_MULTIPLIER": "2", - }, - } - }, - } - ), - ] - ) - resp = requests.get("http://127.0.0.1:8000/B") - resp.raise_for_status() - assert resp.text == "94", resp.text - - @pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") def test_deploy(ray_start_stop): # Deploys some valid config files and checks that the deployments work From 004b2ec978dae289c730326a27f0b73d57196850 Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Thu, 10 Mar 2022 14:59:38 -0600 Subject: [PATCH 06/94] fix --- python/ray/serve/scripts.py | 5 +++-- python/ray/serve/tests/test_cli.py | 28 ++++++++++++++++++++++++---- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 15e7e97bd9c5..3ce7a43cf59d 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -231,7 +231,7 @@ def shutdown(address: str, namespace: str): @click.option( "--address", "-a", - default=None, + default=os.environ.get("RAY_ADDRESS", "http://localhost:8265"), required=False, type=str, help=RAY_DASHBOARD_ADDRESS_HELP_STR, @@ -401,7 +401,8 @@ def info(address: str, json_format=bool): help=( "Prints status information about all deployments in the Serve app.\n\n" "Deployments may be:\n\n" - "- HEALTHY: all replicas are acting normally and passing their health checks.\n\n" + "- HEALTHY: all replicas are acting normally and passing their " + "health checks.\n\n" "- UNHEALTHY: at least one replica is not acting normally and may not be " "passing its health check.\n\n" "- UPDATING: the deployment is updating." diff --git a/python/ray/serve/tests/test_cli.py b/python/ray/serve/tests/test_cli.py index c28eef2a4502..7d7d0c037a8f 100644 --- a/python/ray/serve/tests/test_cli.py +++ b/python/ray/serve/tests/test_cli.py @@ -457,7 +457,7 @@ def test_run_basic(ray_start_stop): os.path.dirname(__file__), "test_config_files", "two_deployments.yaml" ) - p = subprocess.Popen(["serve", "run", config_file_name]) + p = subprocess.Popen(["serve", "run", "--address=auto", config_file_name]) wait_for_condition(lambda: ping_endpoint("one") == "2", timeout=10) wait_for_condition( lambda: ping_endpoint("shallow") == "Hello shallow world!", timeout=10 @@ -469,7 +469,9 @@ def test_run_basic(ray_start_stop): assert ping_endpoint("shallow") == "connection error" # Deploy via import path - p = subprocess.Popen(["serve", "run", "ray.serve.tests.test_cli.parrot"]) + p = subprocess.Popen( + ["serve", "run", "--address=auto", "ray.serve.tests.test_cli.parrot"] + ) wait_for_condition( lambda: ping_endpoint("parrot", params="?sound=squawk") == "squawk", timeout=10 ) @@ -501,6 +503,7 @@ def test_run_init_args_kwargs(ray_start_stop): [ "serve", "run", + "--address=auto", "ray.serve.tests.test_cli.Macaw", "--", "green", @@ -518,6 +521,7 @@ def test_run_init_args_kwargs(ray_start_stop): [ "serve", "run", + "--address=auto", "ray.serve.tests.test_cli.Macaw", "--", "green", @@ -540,7 +544,16 @@ def test_run_init_args_kwargs(ray_start_stop): with pytest.raises(subprocess.CalledProcessError): subprocess.check_output( - ["serve", "run", config_file_name, "--", "green", "--name", "Molly"] + [ + "serve", + "run", + "--address=auto", + config_file_name, + "--", + "green", + "--name", + "Molly", + ] ) @@ -548,7 +561,9 @@ def test_run_init_args_kwargs(ray_start_stop): def test_run_simultaneous(ray_start_stop): # Test that two serve run processes can run simultaneously - p1 = subprocess.Popen(["serve", "run", "ray.serve.tests.test_cli.parrot"]) + p1 = subprocess.Popen( + ["serve", "run", "--address=auto", "ray.serve.tests.test_cli.parrot"] + ) wait_for_condition( lambda: ping_endpoint("parrot", params="?sound=squawk") == "squawk", timeout=10 ) @@ -557,6 +572,7 @@ def test_run_simultaneous(ray_start_stop): [ "serve", "run", + "--address=auto", "ray.serve.tests.test_cli.Macaw", "--", "green", @@ -593,6 +609,7 @@ def test_run_runtime_env(ray_start_stop): [ "serve", "run", + "--address=auto", "test_cli.Macaw", "--working-dir", os.path.dirname(__file__), @@ -610,6 +627,7 @@ def test_run_runtime_env(ray_start_stop): [ "serve", "run", + "--address=auto", os.path.join( os.path.dirname(__file__), "test_config_files", "scarlet.yaml" ), @@ -628,6 +646,7 @@ def test_run_runtime_env(ray_start_stop): [ "serve", "run", + "--address=auto", "test_module.test.one", "--working-dir", "https://github.com/shrekris-anyscale/test_module/archive/HEAD.zip", @@ -642,6 +661,7 @@ def test_run_runtime_env(ray_start_stop): [ "serve", "run", + "--address=auto", os.path.join( os.path.dirname(__file__), "test_config_files", From ff040325987e6bc3897384b4a8325a6e4fad1a46 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 13:23:06 -0800 Subject: [PATCH 07/94] Process ray actors --- python/ray/serve/tests/test_application.py | 11 ++++++++++- python/ray/serve/utils.py | 7 +++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/python/ray/serve/tests/test_application.py b/python/ray/serve/tests/test_application.py index 29287796368a..57fffea0358c 100644 --- a/python/ray/serve/tests/test_application.py +++ b/python/ray/serve/tests/test_application.py @@ -270,6 +270,13 @@ class DecoratedClass: def __call__(self, req=None): return "got decorated class" +# Ray decorated class +@serve.deployment(max_concurrent_queries=17) +@ray.remote +class RayDecorated: + def __call__(self, req=None): + return "got ray class" + def compare_specified_options(deployments1: Dict, deployments2: Dict): """ @@ -355,7 +362,8 @@ def test_deploy_from_yaml(self, serve_instance): def test_convert_to_import_path(self, serve_instance): f = decorated_func.options(name="f") C = DecoratedClass.options(name="C") - app = Application([f, C]) + R = RayDecorated.option(name="R") + app = Application([f, C, R]) with tempfile.TemporaryFile(mode="w+") as tmp: app.to_yaml(tmp) @@ -365,6 +373,7 @@ def test_convert_to_import_path(self, serve_instance): reconstructed_app.deploy() assert requests.get("http://localhost:8000/f").text == "got decorated func" assert requests.get("http://localhost:8000/C").text == "got decorated class" + assert requests.get("http://localhost:8000/R").text == "got ray class" @pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") diff --git a/python/ray/serve/utils.py b/python/ray/serve/utils.py index aa82ee8e6d20..e7b81ce600f6 100644 --- a/python/ray/serve/utils.py +++ b/python/ray/serve/utils.py @@ -318,8 +318,11 @@ def get_deployment_import_path(deployment): if isinstance(body, str): # deployment's func_or_class is already an import path return body - else: - return f"{body.__module__}.{body.__qualname__}" + elif hasattr(body.__ray_actor_class__): + # If decorated with @ray.remote, get the class or function inside + body = body.__ray_actor_class__ + + return f"{body.__module__}.{body.__qualname__}" def parse_import_path(import_path: str): From 47846e9c01f9f4ef9c4c50dca7484b0dc1600977 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 13:36:29 -0800 Subject: [PATCH 08/94] Remove invalid test --- python/ray/serve/tests/test_application.py | 11 +---------- python/ray/serve/utils.py | 4 ++-- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/python/ray/serve/tests/test_application.py b/python/ray/serve/tests/test_application.py index 57fffea0358c..29287796368a 100644 --- a/python/ray/serve/tests/test_application.py +++ b/python/ray/serve/tests/test_application.py @@ -270,13 +270,6 @@ class DecoratedClass: def __call__(self, req=None): return "got decorated class" -# Ray decorated class -@serve.deployment(max_concurrent_queries=17) -@ray.remote -class RayDecorated: - def __call__(self, req=None): - return "got ray class" - def compare_specified_options(deployments1: Dict, deployments2: Dict): """ @@ -362,8 +355,7 @@ def test_deploy_from_yaml(self, serve_instance): def test_convert_to_import_path(self, serve_instance): f = decorated_func.options(name="f") C = DecoratedClass.options(name="C") - R = RayDecorated.option(name="R") - app = Application([f, C, R]) + app = Application([f, C]) with tempfile.TemporaryFile(mode="w+") as tmp: app.to_yaml(tmp) @@ -373,7 +365,6 @@ def test_convert_to_import_path(self, serve_instance): reconstructed_app.deploy() assert requests.get("http://localhost:8000/f").text == "got decorated func" assert requests.get("http://localhost:8000/C").text == "got decorated class" - assert requests.get("http://localhost:8000/R").text == "got ray class" @pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") diff --git a/python/ray/serve/utils.py b/python/ray/serve/utils.py index e7b81ce600f6..c782844ca16d 100644 --- a/python/ray/serve/utils.py +++ b/python/ray/serve/utils.py @@ -318,8 +318,8 @@ def get_deployment_import_path(deployment): if isinstance(body, str): # deployment's func_or_class is already an import path return body - elif hasattr(body.__ray_actor_class__): - # If decorated with @ray.remote, get the class or function inside + elif hasattr(body, "__ray_actor_class__"): + # If ActorClass, get the class or function inside body = body.__ray_actor_class__ return f"{body.__module__}.{body.__qualname__}" From 4e4d363fae9ca1e0a35faf2611ae4d985d3fa59b Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 13:48:15 -0800 Subject: [PATCH 09/94] Convert to import paths on write instead of on definition --- python/ray/serve/application.py | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/python/ray/serve/application.py b/python/ray/serve/application.py index ead8607f63d5..3e6ec216cd1f 100644 --- a/python/ray/serve/application.py +++ b/python/ray/serve/application.py @@ -62,7 +62,7 @@ def add_deployment(self, deployment: Deployment): "new_deployment" ) - self.__setitem__(deployment.name, deployment) + self._deployments[deployment.name] = deployment def deploy(self, blocking: bool = True): """Atomically deploys the Application's deployments to the Ray cluster. @@ -148,10 +148,15 @@ def to_dict(self) -> Dict: This dictionary adheres to the Serve REST API schema. It can be deployed via the Serve REST API. + If any deployment's func_or_class is a function or class (and not an + import path), it overwrites it with that function or class's import path. + Returns: Dict: The Application's deployments formatted in a dictionary. """ + self._convert_to_import_paths() + return serve_application_to_schema(self._deployments.values()).dict() @classmethod @@ -184,6 +189,9 @@ def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: This file is formatted as a Serve YAML config file. It can be deployed via the Serve CLI. + If any deployment's func_or_class is a function or class (and not an + import path), it overwrites it with that function or class's import path. + Args: f (Optional[TextIO]): A pointer to the file where the YAML should be written. @@ -193,6 +201,8 @@ def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: yaml.safe_dump(). Returned only if no file pointer is passed in. """ + self._convert_to_import_paths() + deployment_dict = serve_application_to_schema(self._deployments.values()).dict() if f: @@ -228,6 +238,16 @@ def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": schema = ServeApplicationSchema.parse_obj(deployments_json) return cls(schema_to_serve_application(schema)) + def _convert_to_import_paths(self): + """Convert all func_or_classes to import paths. + + If any deployment's func_or_class is a function or class (and not an + import path), it overwrites it with that function or class's import path. + """ + + for deployment in self._deployments.values(): + deployment._func_or_class = get_deployment_import_path(deployment) + def __getitem__(self, key: str): """Fetch a deployment by name using dict syntax: app["name"] @@ -243,9 +263,6 @@ def __getitem__(self, key: str): def __setitem__(self, key: str, value: Deployment): """Set a deployment by name with dict syntax: app[name]=new_deployment - If the deployment's func_or_class is a function or class (and not an - import path), it overwrites it with that function or class's import path. - Use this to overwrite existing deployments. Args: @@ -261,8 +278,6 @@ def __setitem__(self, key: str, value: Deployment): elif not isinstance(value, Deployment): raise TypeError(f"Got {type(Deployment)} for value. Expected deployment.") - value._func_or_class = get_deployment_import_path(value) - self._deployments[key] = value def __iter__(self): From 622c27371568009b561e6a6e07ec654d6d738dda Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 14:57:37 -0800 Subject: [PATCH 10/94] Do not mutate deployments --- python/ray/serve/application.py | 8 +++++--- python/ray/serve/tests/test_application.py | 5 +---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/python/ray/serve/application.py b/python/ray/serve/application.py index 3e6ec216cd1f..5cb93de3fa51 100644 --- a/python/ray/serve/application.py +++ b/python/ray/serve/application.py @@ -31,7 +31,7 @@ class Application: def __init__(self, deployments: List[Deployment] = None): deployments = deployments or [] - self._deployments = dict() + self._deployments: Dict[str, Deployment] = dict() for d in deployments: self.add_deployment(d) @@ -245,8 +245,10 @@ def _convert_to_import_paths(self): import path), it overwrites it with that function or class's import path. """ - for deployment in self._deployments.values(): - deployment._func_or_class = get_deployment_import_path(deployment) + for name, deployment in self._deployments.items(): + self._deployments[name] = deployment.options( + func_or_class=get_deployment_import_path(deployment) + ) def __getitem__(self, key: str): """Fetch a deployment by name using dict syntax: app["name"] diff --git a/python/ray/serve/tests/test_application.py b/python/ray/serve/tests/test_application.py index 29287796368a..827984921ae6 100644 --- a/python/ray/serve/tests/test_application.py +++ b/python/ray/serve/tests/test_application.py @@ -357,10 +357,7 @@ def test_convert_to_import_path(self, serve_instance): C = DecoratedClass.options(name="C") app = Application([f, C]) - with tempfile.TemporaryFile(mode="w+") as tmp: - app.to_yaml(tmp) - tmp.seek(0) - reconstructed_app = Application.from_yaml(tmp) + reconstructed_app = Application.from_yaml(app.to_yaml()) reconstructed_app.deploy() assert requests.get("http://localhost:8000/f").text == "got decorated func" From 746efdc9687637c90ca5e4dcdeea3203ea39cdba Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 15:11:37 -0800 Subject: [PATCH 11/94] Add skeleton for serve build --- python/ray/serve/scripts.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index e5ddc41a1375..3b5293210180 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -441,3 +441,16 @@ def delete(address: str, yes: bool): cli_logger.newline() cli_logger.success("\nSent delete request successfully!\n") cli_logger.newline() + + +@cli.command( + short_help="Build a Serve config using an Application", + help=( + "Imports the Serve app stored at IMPORT_PATH, and writes a config " + "file to the FILE_PATH." + ), +) +@click.argument("import_path") +@click.argument("file_path") +def build(import_path: str, file_path: str): + pass From 4c9d77b8780a58ed372ea32c39676412e806f3b0 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 15:27:32 -0800 Subject: [PATCH 12/94] Replace __main__ as the module name --- python/ray/serve/application.py | 2 +- python/ray/serve/utils.py | 20 +++++++++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/python/ray/serve/application.py b/python/ray/serve/application.py index 5cb93de3fa51..9f4f0f32eb98 100644 --- a/python/ray/serve/application.py +++ b/python/ray/serve/application.py @@ -247,7 +247,7 @@ def _convert_to_import_paths(self): for name, deployment in self._deployments.items(): self._deployments[name] = deployment.options( - func_or_class=get_deployment_import_path(deployment) + func_or_class=get_deployment_import_path(deployment, replace_main=True) ) def __getitem__(self, key: str): diff --git a/python/ray/serve/utils.py b/python/ray/serve/utils.py index c782844ca16d..6a221274952a 100644 --- a/python/ray/serve/utils.py +++ b/python/ray/serve/utils.py @@ -11,6 +11,7 @@ import os import traceback from enum import Enum +import __main__ from ray.actor import ActorHandle import requests @@ -310,8 +311,14 @@ def msgpack_serialize(obj): return serialized -def get_deployment_import_path(deployment): - """Gets the import path for deployment's func_or_class.""" +def get_deployment_import_path(deployment, replace_main=False): + """ + Gets the import path for deployment's func_or_class. + + deployment: A deployment object whose import path should be returned + replace_main: If this is True, the function will try to replace __main__ + with __main__'s file name if the deployment's module is __main__ + """ body = deployment._func_or_class @@ -322,7 +329,14 @@ def get_deployment_import_path(deployment): # If ActorClass, get the class or function inside body = body.__ray_actor_class__ - return f"{body.__module__}.{body.__qualname__}" + import_path = f"{body.__module__}.{body.__qualname__}" + + if replace_main: + if import_path.split(".")[0] == "__main__" and hasattr(__main__, "__file__"): + file_name = os.path.basename(__main__.__file__) + import_path = f"{file_name.split('.')[0]}.{import_path.split('.')[1]}" + + return import_path def parse_import_path(import_path: str): From bcd9a21931acfe1f57993f0ebbf3f713955df120 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 15:32:33 -0800 Subject: [PATCH 13/94] Implement serve build --- python/ray/serve/scripts.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 3b5293210180..454d814f50b6 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -21,6 +21,7 @@ from ray.autoscaler._private.cli_logger import cli_logger from ray.serve.application import Application from ray.serve.api import Deployment +from ray._private.utils import import_attr RAY_INIT_ADDRESS_HELP_STR = ( "Address to use for ray.init(). Can also be specified " @@ -453,4 +454,6 @@ def delete(address: str, yes: bool): @click.argument("import_path") @click.argument("file_path") def build(import_path: str, file_path: str): - pass + app: Application = import_attr(import_path) + with open(file_path, "w+") as f: + app.to_yaml(f) From 8f749b403592413a720c98854cb7feaca0f12b84 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 15:48:10 -0800 Subject: [PATCH 14/94] Make test for serve build --- python/ray/serve/tests/test_cli.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/python/ray/serve/tests/test_cli.py b/python/ray/serve/tests/test_cli.py index 7d7d0c037a8f..b28221b29b70 100644 --- a/python/ray/serve/tests/test_cli.py +++ b/python/ray/serve/tests/test_cli.py @@ -6,6 +6,7 @@ import signal import pytest import requests +from tempfile import NamedTemporaryFile import ray from ray import serve @@ -13,6 +14,7 @@ from ray._private.test_utils import wait_for_condition from ray.dashboard.optional_utils import RAY_INTERNAL_DASHBOARD_NAMESPACE from ray.serve.scripts import _process_args_and_kwargs, _configure_runtime_env +from ray.serve.application import Application def ping_endpoint(endpoint: str, params: str = ""): @@ -682,5 +684,28 @@ def test_run_runtime_env(ray_start_stop): p.wait() +if sys.platform != "win32": + app_config_file_name = os.path.join( + os.path.dirname(__file__), "test_config_files", "two_deployments.yaml" + ) + with open(app_config_file_name, "r") as f: + test_build_app = Application.from_yaml(f) + + +@pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") +def test_build(ray_start_stop): + f = NamedTemporaryFile(mode="w", delete=False) + + subprocess.check_output( + ["serve", "build", "ray.serve.tests.test_cli.test_build_app", f.name] + ) + subprocess.check_output(["serve", "deploy", f.name]) + + assert requests.get("http://localhost:8000/shallow").text == "Hello shallow world!" + assert requests.get("http://localhost:8000/one").text == "2" + + os.unlink(f.name) + + if __name__ == "__main__": sys.exit(pytest.main(["-v", "-s", __file__])) From 4ba2f2199f8a5fce1c727890bd5b43f3a4748994 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 15:59:12 -0800 Subject: [PATCH 15/94] Allow deployments to be build with serve build --- python/ray/serve/scripts.py | 14 ++++++++++---- python/ray/serve/tests/test_cli.py | 11 ++++++++++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 454d814f50b6..d5fbf668bb87 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -4,7 +4,7 @@ import os import pathlib import click -from typing import Tuple, List, Dict +from typing import Tuple, List, Dict, Union import argparse import ray @@ -447,13 +447,19 @@ def delete(address: str, yes: bool): @cli.command( short_help="Build a Serve config using an Application", help=( - "Imports the Serve app stored at IMPORT_PATH, and writes a config " - "file to the FILE_PATH." + "Imports the Serve app or deployment stored at IMPORT_PATH, and " + "writes a config file to the FILE_PATH." ), ) @click.argument("import_path") @click.argument("file_path") def build(import_path: str, file_path: str): - app: Application = import_attr(import_path) + app_or_deployment: Union[Application, Deployment] = import_attr(import_path) + + if isinstance(app_or_deployment, Deployment): + app = Application([app_or_deployment]) + else: + app = app_or_deployment + with open(file_path, "w+") as f: app.to_yaml(f) diff --git a/python/ray/serve/tests/test_cli.py b/python/ray/serve/tests/test_cli.py index b28221b29b70..d2be3045dc53 100644 --- a/python/ray/serve/tests/test_cli.py +++ b/python/ray/serve/tests/test_cli.py @@ -693,9 +693,10 @@ def test_run_runtime_env(ray_start_stop): @pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") -def test_build(ray_start_stop): +def test_build(ray_start_stop, import_path): f = NamedTemporaryFile(mode="w", delete=False) + # Build an app subprocess.check_output( ["serve", "build", "ray.serve.tests.test_cli.test_build_app", f.name] ) @@ -704,6 +705,14 @@ def test_build(ray_start_stop): assert requests.get("http://localhost:8000/shallow").text == "Hello shallow world!" assert requests.get("http://localhost:8000/one").text == "2" + # Build a deployment + subprocess.check_output( + ["serve", "build", "ray.serve.tests.test_cli.parrot", f.name] + ) + subprocess.check_output(["serve", "deploy", f.name]) + + assert requests.get("http://localhost:8000/parrot?sound=squawk").text == "squawk" + os.unlink(f.name) From 92981a8b7a2102ee7406a945cc98bede3e7fa502 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 15:59:52 -0800 Subject: [PATCH 16/94] Remove unnecessary parameter --- python/ray/serve/tests/test_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ray/serve/tests/test_cli.py b/python/ray/serve/tests/test_cli.py index d2be3045dc53..2981fe5a6a0c 100644 --- a/python/ray/serve/tests/test_cli.py +++ b/python/ray/serve/tests/test_cli.py @@ -693,7 +693,7 @@ def test_run_runtime_env(ray_start_stop): @pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") -def test_build(ray_start_stop, import_path): +def test_build(ray_start_stop): f = NamedTemporaryFile(mode="w", delete=False) # Build an app From 76196f25493d1e58dbf9483fd3f9cc0a6eca48b5 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 16:04:52 -0800 Subject: [PATCH 17/94] Fix build test --- python/ray/serve/tests/test_cli.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/python/ray/serve/tests/test_cli.py b/python/ray/serve/tests/test_cli.py index 2981fe5a6a0c..bf866a79f653 100644 --- a/python/ray/serve/tests/test_cli.py +++ b/python/ray/serve/tests/test_cli.py @@ -684,6 +684,10 @@ def test_run_runtime_env(ray_start_stop): p.wait() +@serve.deployment +def global_f(*args): + return "wonderful world" + if sys.platform != "win32": app_config_file_name = os.path.join( os.path.dirname(__file__), "test_config_files", "two_deployments.yaml" @@ -691,7 +695,6 @@ def test_run_runtime_env(ray_start_stop): with open(app_config_file_name, "r") as f: test_build_app = Application.from_yaml(f) - @pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") def test_build(ray_start_stop): f = NamedTemporaryFile(mode="w", delete=False) @@ -707,11 +710,11 @@ def test_build(ray_start_stop): # Build a deployment subprocess.check_output( - ["serve", "build", "ray.serve.tests.test_cli.parrot", f.name] + ["serve", "build", "ray.serve.tests.test_cli.global_f", f.name] ) subprocess.check_output(["serve", "deploy", f.name]) - assert requests.get("http://localhost:8000/parrot?sound=squawk").text == "squawk" + assert requests.get("http://localhost:8000/global_f").text == "wonderful world" os.unlink(f.name) From beededf31b578482f18a5ae3d2c5fdac123761e4 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 16:05:05 -0800 Subject: [PATCH 18/94] Linter --- python/ray/serve/tests/test_cli.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/ray/serve/tests/test_cli.py b/python/ray/serve/tests/test_cli.py index bf866a79f653..d5a599b83cef 100644 --- a/python/ray/serve/tests/test_cli.py +++ b/python/ray/serve/tests/test_cli.py @@ -688,6 +688,7 @@ def test_run_runtime_env(ray_start_stop): def global_f(*args): return "wonderful world" + if sys.platform != "win32": app_config_file_name = os.path.join( os.path.dirname(__file__), "test_config_files", "two_deployments.yaml" @@ -695,6 +696,7 @@ def global_f(*args): with open(app_config_file_name, "r") as f: test_build_app = Application.from_yaml(f) + @pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") def test_build(ray_start_stop): f = NamedTemporaryFile(mode="w", delete=False) From b2ce9733ab8be23c1724b9058506281ef9b75b19 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 18:44:36 -0800 Subject: [PATCH 19/94] Refactor test_build to not rely on file paths --- python/ray/serve/tests/test_cli.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/python/ray/serve/tests/test_cli.py b/python/ray/serve/tests/test_cli.py index d5a599b83cef..f74c72db8e48 100644 --- a/python/ray/serve/tests/test_cli.py +++ b/python/ray/serve/tests/test_cli.py @@ -689,15 +689,14 @@ def global_f(*args): return "wonderful world" -if sys.platform != "win32": - app_config_file_name = os.path.join( - os.path.dirname(__file__), "test_config_files", "two_deployments.yaml" - ) - with open(app_config_file_name, "r") as f: - test_build_app = Application.from_yaml(f) +test_build_app = Application( + [ + global_f.options(name="f1"), + global_f.options(name="f2"), + ] +) -@pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") def test_build(ray_start_stop): f = NamedTemporaryFile(mode="w", delete=False) @@ -707,8 +706,8 @@ def test_build(ray_start_stop): ) subprocess.check_output(["serve", "deploy", f.name]) - assert requests.get("http://localhost:8000/shallow").text == "Hello shallow world!" - assert requests.get("http://localhost:8000/one").text == "2" + assert requests.get("http://localhost:8000/f1").text == "wonderful world" + assert requests.get("http://localhost:8000/f2").text == "wonderful world" # Build a deployment subprocess.check_output( From fcad902558f5a0983642adf5b9b69dfd8a315b1b Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 10 Mar 2022 21:47:32 -0800 Subject: [PATCH 20/94] Disable test on Windows --- python/ray/serve/tests/test_cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/python/ray/serve/tests/test_cli.py b/python/ray/serve/tests/test_cli.py index f74c72db8e48..b4fbe2e80bf8 100644 --- a/python/ray/serve/tests/test_cli.py +++ b/python/ray/serve/tests/test_cli.py @@ -697,6 +697,7 @@ def global_f(*args): ) +@pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") def test_build(ray_start_stop): f = NamedTemporaryFile(mode="w", delete=False) From 2e566b2022af1c2b7a95ae389eca81be8ed85805 Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Sun, 13 Mar 2022 21:14:34 -0500 Subject: [PATCH 21/94] WIP --- python/ray/serve/api.py | 237 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 230 insertions(+), 7 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 5fb727756f58..53eb2ad486a2 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -13,6 +13,7 @@ Callable, Dict, Optional, + TextIO, Tuple, Type, Union, @@ -44,9 +45,12 @@ SERVE_CONTROLLER_NAME, MAX_CACHED_HANDLES, CONTROLLER_MAX_CONCURRENCY, + DEFAULT_HTTP_HOST, + DEFAULT_HTTP_PORT, ) from ray.serve.controller import ServeController from ray.serve.exceptions import RayServeException +from ray.experimental.dag import ClassNode from ray.serve.handle import RayServeHandle, RayServeSyncHandle from ray.serve.http_util import ASGIHTTPSender, make_fastapi_class_based_view from ray.serve.utils import ( @@ -890,6 +894,23 @@ async def __del__(self): return decorator +@PublicAPI(stability="alpha") +class BoundDeploymentNode(ClassNode): + """Represents a deployment with its bound config options and arguments. + + The bound deployment can be run, deployed, or built to a production config + using serve.run, serve.deploy, and serve.build, respectively. + + A bound deployment can be passed as an argument to other bound deployments + to build a multi-deployment application. When the application is deployed, the + bound deployments passed into a constructor will be converted to + RayServeHandles that can be used to send requests. + """ + + def __init__(self): + raise NotImplementedError() + + @PublicAPI class Deployment: def __init__( @@ -1046,6 +1067,15 @@ def __call__(self): "Use `deployment.deploy() instead.`" ) + @PublicAPI(stability="alpha") + def bind(self, *args, **kwargs) -> BoundDeploymentNode: + """Bind the provided arguments and return a BoundDeploymentNode. + + The returned bound deployment can be deployed or bound to other + deployments to create a multi-deployment application. + """ + raise NotImplementedError() + @PublicAPI def deploy(self, *init_args, _blocking=True, **init_kwargs): """Deploy or update this deployment. @@ -1182,13 +1212,6 @@ def options( _internal=True, ) - def bind(self, *args, **kwargs): - raise AttributeError( - "DAG building API should only be used for @ray.remote decorated " - "class or function, not in serve deployment or library " - "specific API." - ) - def __eq__(self, other): return all( [ @@ -1451,3 +1474,203 @@ def get_deployment_statuses() -> Dict[str, DeploymentStatusInfo]: """ return internal_get_global_client().get_deployment_statuses() + + +class Application: + """A static, pre-built Serve application. + + An application consists of a number of Serve deployments that can send + requests to each other. One of the deployments acts as the "ingress," + meaning that it receives external traffic and is the entrypoint to the + application. + + The config options of each deployment can be modified by accessing the + deployment by name (e.g., app["my_deployment"].update(...)). + + This application object can be written to a config file and later deployed + to production using the Serve CLI or REST API. + """ + + # TODO(edoakes): should this literally just be the REST pydantic schema? + # At the very least, it should be a simple dictionary. + + def __init__(self): + raise NotImplementedError() + + def to_dict(self) -> Dict: + """Returns this Application's deployments as a dictionary. + + This dictionary adheres to the Serve REST API schema. It can be deployed + via the Serve REST API. + + Returns: + Dict: The Application's deployments formatted in a dictionary. + """ + raise NotImplementedError() + + @classmethod + def from_dict(cls, d: Dict) -> "Application": + """Converts a dictionary of deployment data to an Application. + + Takes in a dictionary matching the Serve REST API schema and converts + it to an Application containing those deployments. + + Args: + d (Dict): A dictionary containing the deployments' data that matches + the Serve REST API schema. + + Returns: + Application: a new Application object containing the deployments. + """ + raise NotImplementedError() + + def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: + """Returns this Application's deployments as a YAML string. + + Optionally writes the YAML string to a file as well. To write to a + file, use this pattern: + + with open("file_name.txt", "w") as f: + app.to_yaml(f=f) + + This file is formatted as a Serve YAML config file. It can be deployed + via the Serve CLI. + + Args: + f (Optional[TextIO]): A pointer to the file where the YAML should + be written. + + Returns: + Optional[String]: The deployments' YAML string. The output is from + yaml.safe_dump(). Returned only if no file pointer is passed in. + """ + raise NotImplementedError() + + @classmethod + def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": + """Converts YAML data to deployments for an Application. + + Takes in a string or a file pointer to a file containing deployment + definitions in YAML. These definitions are converted to a new + Application object containing the deployments. + + To read from a file, use the following pattern: + + with open("file_name.txt", "w") as f: + app = app.from_yaml(str_or_file) + + Args: + str_or_file (Union[String, TextIO]): Either a string containing + YAML deployment definitions or a pointer to a file containing + YAML deployment definitions. The YAML format must adhere to the + ServeApplicationSchema JSON Schema defined in + ray.serve.schema. This function works with + Serve YAML config files. + + Returns: + Application: a new Application object containing the deployments. + """ + raise NotImplementedError() + + def __getitem__(self, key: str): + """Fetch a deployment by name using dict syntax: app["name"] + + Raises: + KeyError: if the name is not in this Application. + """ + raise NotImplementedError() + + def __setitem__(self, key: str, value: Deployment): + """Set a deployment by name with dict syntax: app[name]=new_deployment + + Use this to overwrite existing deployments. + + Args: + key (String): name + value (Deployment): the deployment that the name maps to + + Raises: + TypeError: if the key is not a String or the value is not a deployment. + """ + raise NotImplementedError() + + def __iter__(self): + """Iterator over Application's deployments. + + Enables "for deployment in Application" pattern. + """ + raise NotImplementedError() + + def __len__(self): + """Number of deployments in this Application.""" + raise NotImplementedError() + + def __contains__(self, key: str): + """Checks if the key exists in self._deployments.""" + raise NotImplementedError() + + +NodeOrApp = Union[BoundDeploymentNode, Application] + + +@PublicAPI(stability="alpha") +def run( + target: NodeOrApp, + *, + host: str = DEFAULT_HTTP_HOST, + port: int = DEFAULT_HTTP_PORT, + logger: Optional[logging.Logger] = None, +): + """Run a Serve application (blocking). + + Deploys all of the deployments in the application then blocks, + periodically logging the status. Upon a KeyboardInterrupt, the application + will be torn down and all of its deployments deleted. + + Either a BoundDeploymentNode or a pre-built application can be passed in. + If a BoundDeploymentNode is passed in, all of the deployments it depends on + will be deployed. + """ + raise NotImplementedError() + + +@PublicAPI(stability="alpha") +def deploy( + target: NodeOrApp, *, host: str = DEFAULT_HTTP_HOST, port: int = DEFAULT_HTTP_PORT +) -> RayServeHandle: + """Deploy a Serve application async and return a handle to the ingress. + + Deploys all of the deployments in the application and returns a + RayServeHandle to the ingress deployment. + + Either a BoundDeploymentNode or a pre-built application can be passed in. + If a BoundDeploymentNode is passed in, all of the deployments it depends on + will be deployed. + """ + raise NotImplementedError() + + +@PublicAPI(stability="alpha") +def build( + target: BoundDeploymentNode, + *, + host: str = DEFAULT_HTTP_HOST, + port: int = DEFAULT_HTTP_PORT, +) -> Application: + """Builds a Serve application into a static configuration. + + Takes in a BoundDeploymentNode and converts it to a Serve application + consisting of one or more deployments. This is intended to be used for + production scenarios and deployed via the Serve REST API or CLI, so there + are some restrictions placed on the deployments: + 1) All of the deployments must be importable. That is, they cannot be + defined in __main__ or inline defined. The deployments will be + imported in production using the same import path they were here. + 2) All arguments bound to the deployment must be JSON-serializable. + + The returned Application object can be exported to a dictionary or YAML + config. + """ + # TODO(edoakes): this should accept host and port, but we don't + # currently support them in the REST API. + raise NotImplementedError() From 96c65bc766ab23f18051d96262970eda11307a74 Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Mon, 14 Mar 2022 10:31:27 -0500 Subject: [PATCH 22/94] fix --- python/ray/serve/pipeline/tests/test_deployment_node.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/python/ray/serve/pipeline/tests/test_deployment_node.py b/python/ray/serve/pipeline/tests/test_deployment_node.py index ee66bacabe86..b61c76825493 100644 --- a/python/ray/serve/pipeline/tests/test_deployment_node.py +++ b/python/ray/serve/pipeline/tests/test_deployment_node.py @@ -46,14 +46,6 @@ async def get(self): return self.i -def test_disallow_binding_deployments(): - with pytest.raises( - AttributeError, - match="DAG building API should only be used for @ray.remote decorated", - ): - _ = ServeActor.bind(10) - - @pytest.mark.asyncio async def test_simple_deployment_async(serve_instance): """Internal testing only for simple creation and execution. From 12a1e6db91c09fd8a1458f551f362f8b11ba6a7b Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Mon, 14 Mar 2022 10:50:56 -0500 Subject: [PATCH 23/94] Add call graph --- python/ray/serve/api.py | 34 ++++++++++++++++++++++++++++++++++ python/ray/serve/drivers.py | 21 +++++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 python/ray/serve/drivers.py diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 53eb2ad486a2..25b892c94887 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -894,6 +894,37 @@ async def __del__(self): return decorator +@PublicAPI(stability="alpha") +class DeployedCallGraph: + """Resolved from a BoundDeploymentMethodNode at runtime. + + This can be used to call the DAG from a driver deployment to efficiently + orchestrate a multi-deployment pipeline. + """ + + def __init__(self, serialized_dag_json: str): + raise NotImplementedError() + + def remote(self, *args, **kwargs) -> ray.ObjectRef: + """Call the DAG.""" + raise NotImplementedError() + + +@PublicAPI(stability="alpha") +class BoundDeploymentMethodNode(ClassNode): + """Represents a method call on a bound deployment node. + + These method calls can be composed into an optimized call DAG and passed + to a "driver" deployment that will orchestrate the calls at runtime. + + This class cannot be called directly. Instead, when it is bound to a + deployment node, it will be resolved to a DeployedCallGraph at runtime. + """ + + def __init__(self): + raise NotImplementedError() + + @PublicAPI(stability="alpha") class BoundDeploymentNode(ClassNode): """Represents a deployment with its bound config options and arguments. @@ -905,6 +936,9 @@ class BoundDeploymentNode(ClassNode): to build a multi-deployment application. When the application is deployed, the bound deployments passed into a constructor will be converted to RayServeHandles that can be used to send requests. + + Calling deployment.method.bind() will return a BoundDeploymentMethodNode + that can be used to compose an optimized call graph. """ def __init__(self): diff --git a/python/ray/serve/drivers.py b/python/ray/serve/drivers.py new file mode 100644 index 000000000000..a32549df8b61 --- /dev/null +++ b/python/ray/serve/drivers.py @@ -0,0 +1,21 @@ +from typing import Callable, Optional, Union + +import starlette + +from ray import serve +from ray.serve.api import DeployedCallGraph + + +@serve.deployment +class PipelineDriver: + def __init__( + self, + call_graph: DeployedCallGraph, + *, + input_schema: Optional[Union[str, Callable]] = None, + ): + raise NotImplementedError() + + async def __call__(self, request: starlette.requests.Request): + """Parse input schema and pass the result to the call graph.""" + raise NotImplementedError() From 5cbd6ebcc60966aaae7a6b423af2e3c83a4fe80c Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Mon, 14 Mar 2022 11:11:43 -0500 Subject: [PATCH 24/94] Deployment --- python/ray/serve/drivers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ray/serve/drivers.py b/python/ray/serve/drivers.py index a32549df8b61..1fa244b77e1d 100644 --- a/python/ray/serve/drivers.py +++ b/python/ray/serve/drivers.py @@ -7,7 +7,7 @@ @serve.deployment -class PipelineDriver: +class PipelineDriverDeployment: def __init__( self, call_graph: DeployedCallGraph, From 50a47a9851119a4b29b9bf7994ddfe6255fefa37 Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Mon, 14 Mar 2022 12:13:10 -0500 Subject: [PATCH 25/94] update --- python/ray/serve/api.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 25b892c94887..ea0e24dbc18c 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -895,8 +895,8 @@ async def __del__(self): @PublicAPI(stability="alpha") -class DeployedCallGraph: - """Resolved from a BoundDeploymentMethodNode at runtime. +class DAGHandle: + """Resolved from a DeploymentMethodNode at runtime. This can be used to call the DAG from a driver deployment to efficiently orchestrate a multi-deployment pipeline. @@ -911,7 +911,7 @@ def remote(self, *args, **kwargs) -> ray.ObjectRef: @PublicAPI(stability="alpha") -class BoundDeploymentMethodNode(ClassNode): +class DeploymentMethodNode(ClassNode): """Represents a method call on a bound deployment node. These method calls can be composed into an optimized call DAG and passed @@ -926,7 +926,7 @@ def __init__(self): @PublicAPI(stability="alpha") -class BoundDeploymentNode(ClassNode): +class DeploymentNode(ClassNode): """Represents a deployment with its bound config options and arguments. The bound deployment can be run, deployed, or built to a production config @@ -937,7 +937,7 @@ class BoundDeploymentNode(ClassNode): bound deployments passed into a constructor will be converted to RayServeHandles that can be used to send requests. - Calling deployment.method.bind() will return a BoundDeploymentMethodNode + Calling deployment.method.bind() will return a DeploymentMethodNode that can be used to compose an optimized call graph. """ @@ -1102,8 +1102,8 @@ def __call__(self): ) @PublicAPI(stability="alpha") - def bind(self, *args, **kwargs) -> BoundDeploymentNode: - """Bind the provided arguments and return a BoundDeploymentNode. + def bind(self, *args, **kwargs) -> DeploymentNode: + """Bind the provided arguments and return a DeploymentNode. The returned bound deployment can be deployed or bound to other deployments to create a multi-deployment application. @@ -1644,7 +1644,7 @@ def __contains__(self, key: str): raise NotImplementedError() -NodeOrApp = Union[BoundDeploymentNode, Application] +NodeOrApp = Union[DeploymentNode, Application] @PublicAPI(stability="alpha") @@ -1661,8 +1661,8 @@ def run( periodically logging the status. Upon a KeyboardInterrupt, the application will be torn down and all of its deployments deleted. - Either a BoundDeploymentNode or a pre-built application can be passed in. - If a BoundDeploymentNode is passed in, all of the deployments it depends on + Either a DeploymentNode or a pre-built application can be passed in. + If a DeploymentNode is passed in, all of the deployments it depends on will be deployed. """ raise NotImplementedError() @@ -1677,8 +1677,8 @@ def deploy( Deploys all of the deployments in the application and returns a RayServeHandle to the ingress deployment. - Either a BoundDeploymentNode or a pre-built application can be passed in. - If a BoundDeploymentNode is passed in, all of the deployments it depends on + Either a DeploymentNode or a pre-built application can be passed in. + If a DeploymentNode is passed in, all of the deployments it depends on will be deployed. """ raise NotImplementedError() @@ -1686,14 +1686,14 @@ def deploy( @PublicAPI(stability="alpha") def build( - target: BoundDeploymentNode, + target: DeploymentNode, *, host: str = DEFAULT_HTTP_HOST, port: int = DEFAULT_HTTP_PORT, ) -> Application: """Builds a Serve application into a static configuration. - Takes in a BoundDeploymentNode and converts it to a Serve application + Takes in a DeploymentNode and converts it to a Serve application consisting of one or more deployments. This is intended to be used for production scenarios and deployed via the Serve REST API or CLI, so there are some restrictions placed on the deployments: From 9a1be0fcd67063a8c0c3695ff1b553b4b1318b85 Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Mon, 14 Mar 2022 12:13:54 -0500 Subject: [PATCH 26/94] remove host/port --- python/ray/serve/api.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index ea0e24dbc18c..6ad600efdf72 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1685,12 +1685,7 @@ def deploy( @PublicAPI(stability="alpha") -def build( - target: DeploymentNode, - *, - host: str = DEFAULT_HTTP_HOST, - port: int = DEFAULT_HTTP_PORT, -) -> Application: +def build(target: DeploymentNode) -> Application: """Builds a Serve application into a static configuration. Takes in a DeploymentNode and converts it to a Serve application From b6f5b83fdc97f9943f4204af5975b2d7a969a7ca Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Mon, 14 Mar 2022 11:48:57 -0700 Subject: [PATCH 27/94] Port Application implementation to API --- python/ray/serve/api.py | 89 ++++++++++++++++++++++++++++++++--------- 1 file changed, 69 insertions(+), 20 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 6ad600efdf72..419768143256 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -6,6 +6,7 @@ import random import re import time +import yaml from dataclasses import dataclass from functools import wraps from typing import ( @@ -65,6 +66,12 @@ from ray.util.annotations import PublicAPI import ray from ray import cloudpickle +from ray.serve.schema import ( + ServeApplicationSchema, + serve_application_to_schema, + schema_to_serve_application, + serve_application_status_to_schema, +) _INTERNAL_REPLICA_CONTEXT = None @@ -1529,7 +1536,11 @@ class Application: # At the very least, it should be a simple dictionary. def __init__(self): - raise NotImplementedError() + deployments = deployments or [] + + self._deployments = dict() + for d in deployments: + self._add_deployment(d) def to_dict(self) -> Dict: """Returns this Application's deployments as a dictionary. @@ -1540,7 +1551,8 @@ def to_dict(self) -> Dict: Returns: Dict: The Application's deployments formatted in a dictionary. """ - raise NotImplementedError() + + return serve_application_to_schema(self._deployments.values()).dict() @classmethod def from_dict(cls, d: Dict) -> "Application": @@ -1556,7 +1568,9 @@ def from_dict(cls, d: Dict) -> "Application": Returns: Application: a new Application object containing the deployments. """ - raise NotImplementedError() + + schema = ServeApplicationSchema.parse_obj(d) + return cls(schema_to_serve_application(schema)) def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: """Returns this Application's deployments as a YAML string. @@ -1578,7 +1592,12 @@ def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: Optional[String]: The deployments' YAML string. The output is from yaml.safe_dump(). Returned only if no file pointer is passed in. """ - raise NotImplementedError() + + deployment_dict = serve_application_to_schema(self._deployments.values()).dict() + + if f: + yaml.safe_dump(deployment_dict, f, default_flow_style=False) + return yaml.safe_dump(deployment_dict, default_flow_style=False) @classmethod def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": @@ -1604,44 +1623,74 @@ def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": Returns: Application: a new Application object containing the deployments. """ - raise NotImplementedError() + + deployments_json = yaml.safe_load(str_or_file) + schema = ServeApplicationSchema.parse_obj(deployments_json) + return cls(schema_to_serve_application(schema)) - def __getitem__(self, key: str): - """Fetch a deployment by name using dict syntax: app["name"] + def _add_deployment(self, deployment: Deployment): + """Adds the deployment to this Serve application. + + Validates that a deployment with the same name doesn't already exist. + To overwrite an existing deployment, use index notation. For example: + + app[deployment_name] = deployment + + Args: + deployment (Deployment): deployment to add to this Application. Raises: - KeyError: if the name is not in this Application. + ValueError: If a deployment with deployment.name already exists in + this Application. """ - raise NotImplementedError() - def __setitem__(self, key: str, value: Deployment): - """Set a deployment by name with dict syntax: app[name]=new_deployment + if not isinstance(deployment, Deployment): + raise TypeError(f"Got {type(deployment)}. Expected deployment.") + elif deployment.name in self._deployments: + raise ValueError( + f'Deployment with name "{deployment.name}" already ' + "exists in this application. To overwrite this " + "deployment, use attribute or index notation. " + "For example:\n\napp.deployment_name = " + "new_deployment" + ) - Use this to overwrite existing deployments. + self._deployments[deployment.name] = deployment + + def _get_deployments(self) -> List[Deployment]: + """Gets a list of this Application's deployments.""" - Args: - key (String): name - value (Deployment): the deployment that the name maps to + return self._deployments.values() + + def __getitem__(self, key: str): + """Fetch a deployment by name using dict syntax: app["name"] Raises: - TypeError: if the key is not a String or the value is not a deployment. + KeyError: if the name is not in this Application. """ - raise NotImplementedError() + + if key in self._deployments: + return self._deployments[key] + else: + raise KeyError(f'Serve application does not contain a "{key}" deployment.') def __iter__(self): """Iterator over Application's deployments. Enables "for deployment in Application" pattern. """ - raise NotImplementedError() + + return iter(self._deployments.values()) def __len__(self): """Number of deployments in this Application.""" - raise NotImplementedError() + + return len(self._deployments) def __contains__(self, key: str): """Checks if the key exists in self._deployments.""" - raise NotImplementedError() + + return key in self._deployments NodeOrApp = Union[DeploymentNode, Application] From 8368747778baaa56ce0aeedcd5f5a210c421138c Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Mon, 14 Mar 2022 12:15:31 -0700 Subject: [PATCH 28/94] Implement serve run and deploy --- python/ray/serve/api.py | 99 ++++++++++++++++++++++++++++++++++------- 1 file changed, 83 insertions(+), 16 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 419768143256..438e92938af4 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -7,6 +7,7 @@ import re import time import yaml +import sys from dataclasses import dataclass from functools import wraps from typing import ( @@ -46,8 +47,6 @@ SERVE_CONTROLLER_NAME, MAX_CACHED_HANDLES, CONTROLLER_MAX_CONCURRENCY, - DEFAULT_HTTP_HOST, - DEFAULT_HTTP_PORT, ) from ray.serve.controller import ServeController from ray.serve.exceptions import RayServeException @@ -72,6 +71,7 @@ schema_to_serve_application, serve_application_status_to_schema, ) +from ray.serve.pipeline.api import extract_deployments_from_serve_dag _INTERNAL_REPLICA_CONTEXT = None @@ -1535,7 +1535,7 @@ class Application: # TODO(edoakes): should this literally just be the REST pydantic schema? # At the very least, it should be a simple dictionary. - def __init__(self): + def __init__(self, deployments: List[Deployment]=None): deployments = deployments or [] self._deployments = dict() @@ -1551,7 +1551,7 @@ def to_dict(self) -> Dict: Returns: Dict: The Application's deployments formatted in a dictionary. """ - + return serve_application_to_schema(self._deployments.values()).dict() @classmethod @@ -1568,7 +1568,7 @@ def from_dict(cls, d: Dict) -> "Application": Returns: Application: a new Application object containing the deployments. """ - + schema = ServeApplicationSchema.parse_obj(d) return cls(schema_to_serve_application(schema)) @@ -1592,7 +1592,7 @@ def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: Optional[String]: The deployments' YAML string. The output is from yaml.safe_dump(). Returned only if no file pointer is passed in. """ - + deployment_dict = serve_application_to_schema(self._deployments.values()).dict() if f: @@ -1623,7 +1623,7 @@ def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": Returns: Application: a new Application object containing the deployments. """ - + deployments_json = yaml.safe_load(str_or_file) schema = ServeApplicationSchema.parse_obj(deployments_json) return cls(schema_to_serve_application(schema)) @@ -1656,7 +1656,7 @@ def _add_deployment(self, deployment: Deployment): ) self._deployments[deployment.name] = deployment - + def _get_deployments(self) -> List[Deployment]: """Gets a list of this Application's deployments.""" @@ -1668,7 +1668,7 @@ def __getitem__(self, key: str): Raises: KeyError: if the name is not in this Application. """ - + if key in self._deployments: return self._deployments[key] else: @@ -1684,12 +1684,12 @@ def __iter__(self): def __len__(self): """Number of deployments in this Application.""" - + return len(self._deployments) def __contains__(self, key: str): """Checks if the key exists in self._deployments.""" - + return key in self._deployments @@ -1700,8 +1700,7 @@ def __contains__(self, key: str): def run( target: NodeOrApp, *, - host: str = DEFAULT_HTTP_HOST, - port: int = DEFAULT_HTTP_PORT, + http_options: Union[Dict, HTTPOptions] = None, logger: Optional[logging.Logger] = None, ): """Run a Serve application (blocking). @@ -1714,12 +1713,34 @@ def run( If a DeploymentNode is passed in, all of the deployments it depends on will be deployed. """ - raise NotImplementedError() + + try: + deployments = _get_deployments_from_target(target) + + deploy(target, http_options=http_options, blocking=True) + + logger.info("\nDeployed successfully!\n") + + while True: + statuses = serve_application_status_to_schema( + get_deployment_statuses() + ).json(indent=4) + logger.info(f"{statuses}") + time.sleep(10) + + except KeyboardInterrupt: + logger.info("Got SIGINT (KeyboardInterrupt). Removing deployments.") + for deployment in deployments.values(): + deployment.delete() + if len(list_deployments()) == 0: + logger.info("No deployments left. Shutting down Serve.") + shutdown() + sys.exit() @PublicAPI(stability="alpha") def deploy( - target: NodeOrApp, *, host: str = DEFAULT_HTTP_HOST, port: int = DEFAULT_HTTP_PORT + target: NodeOrApp, *, http_options: Union[Dict, HTTPOptions] = None, blocking=True ) -> RayServeHandle: """Deploy a Serve application async and return a handle to the ingress. @@ -1730,7 +1751,38 @@ def deploy( If a DeploymentNode is passed in, all of the deployments it depends on will be deployed. """ - raise NotImplementedError() + + deployments = _get_deployments_from_target(target) + + # TODO (shrekris-anyscale): validate ingress + + if len(deployments) == 0: + return + + parameter_group = [] + + for deployment in deployments.values(): + + deployment_parameters = { + "name": deployment._name, + "func_or_class": deployment._func_or_class, + "init_args": deployment.init_args, + "init_kwargs": deployment.init_kwargs, + "ray_actor_options": deployment._ray_actor_options, + "config": deployment._config, + "version": deployment._version, + "prev_version": deployment._prev_version, + "route_prefix": deployment.route_prefix, + "url": deployment.url, + } + + parameter_group.append(deployment_parameters) + + start(detached=True, http_options=http_options) + + internal_get_global_client().deploy_group(parameter_group, _blocking=blocking) + + # TODO (shrekris-anyscale): return handle to ingress deployment @PublicAPI(stability="alpha") @@ -1752,3 +1804,18 @@ def build(target: DeploymentNode) -> Application: # TODO(edoakes): this should accept host and port, but we don't # currently support them in the REST API. raise NotImplementedError() + + +def _get_deployments_from_target(target: NodeOrApp) -> List[Deployment]: + """Validates target and gets its deployments.""" + + if isinstance(target, Application): + return target._get_deployments() + elif isinstance(target, DeploymentNode): + return extract_deployments_from_serve_dag(target) + else: + raise TypeError( + "Expected a DeploymentNode or " + "Application as target. Got unexpected type " + f'"{type(target)}" instead.' + ) From 587cc4ec5d978ffa1d27c7339bc6718e4a751b1c Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Mon, 14 Mar 2022 12:19:08 -0700 Subject: [PATCH 29/94] Make deploy and run public APIs --- python/ray/serve/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/python/ray/serve/__init__.py b/python/ray/serve/__init__.py index df474240ce57..d77bbd526f0d 100644 --- a/python/ray/serve/__init__.py +++ b/python/ray/serve/__init__.py @@ -7,6 +7,8 @@ deployment, get_deployment, list_deployments, + deploy, + run, ) from ray.serve.batching import batch from ray.serve.config import HTTPOptions @@ -33,4 +35,6 @@ "deployment", "get_deployment", "list_deployments", + "deploy", + "run", ] From 08fb88a618c506ef8e11a963128af63017671340 Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Mon, 14 Mar 2022 14:53:16 -0500 Subject: [PATCH 30/94] WIP --- python/ray/serve/scripts.py | 130 ++------------------ python/ray/serve/tests/test_cli.py | 186 ----------------------------- 2 files changed, 13 insertions(+), 303 deletions(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index e5ddc41a1375..9eaed5724cce 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -8,6 +8,7 @@ import argparse import ray +from ray._private.utils import import_attr from ray.serve.config import DeploymentMode from ray import serve from ray.serve.constants import ( @@ -33,81 +34,6 @@ ) -def _process_args_and_kwargs( - args_and_kwargs: Tuple[str], -) -> Tuple[List[str], Dict[str, str]]: - """ - Takes in a Tuple of strings. Any string prepended with "--" is considered a - keyword. Keywords must be formatted as --keyword=value or --keyword value. - All other strings are considered args. All args must come before kwargs. - - For example: - - ("argval1", "argval2", "--kwarg1", "kwval1", "--kwarg2", "kwval2",) - - becomes - - args = ["argval1", "argval2"] - kwargs = {"kwarg1": "kwval1", "kwarg2": "kwval2"} - """ - - if args_and_kwargs is None: - return [], {} - - class ErroringArgumentParser(argparse.ArgumentParser): - """ - ArgumentParser prints and exits upon error. This subclass raises a - ValueError instead. - """ - - def error(self, message): - if message.find("unrecognized arguments") == 0: - # Give clear message when args come between or after kwargs - arg = message[message.find(":") + 2 :] - raise ValueError( - f'Argument "{arg}" was separated from other args by ' - "keyword arguments. Args cannot be separated by " - f"kwargs.\nMessage from parser: {message}" - ) - elif message.endswith("expected one argument"): - # Give clear message when kwargs are undefined - kwarg = message[message.find("--") : message.rfind(":")] - raise ValueError( - f'Got no value for argument "{kwarg}". All ' - "keyword arguments specified must have a value." - f"\nMessage from parser: {message}" - ) - else: - # Raise argparse's error otherwise - raise ValueError(message) - - parser = ErroringArgumentParser() - parser.add_argument("args", nargs="*") - for arg_or_kwarg in args_and_kwargs: - if arg_or_kwarg[:2] == "--": - parser.add_argument(arg_or_kwarg.split("=")[0]) - - args_and_kwargs = vars(parser.parse_args(args_and_kwargs)) - args = args_and_kwargs["args"] - del args_and_kwargs["args"] - return args, args_and_kwargs - - -def _configure_runtime_env(deployment: Deployment, updates: Dict): - """Overwrites deployment's runtime_env with fields in updates. - - Any fields in deployment's runtime_env that aren't in updates stay the - same. - """ - - if deployment.ray_actor_options is None: - deployment._ray_actor_options = {"runtime_env": updates} - else: - current_env = deployment.ray_actor_options.get("runtime_env", {}) - updates.update(current_env) - deployment.ray_actor_options["runtime_env"] = updates - - @click.group(help="[EXPERIMENTAL] CLI for managing Serve instances on a Ray cluster.") def cli(): pass @@ -251,16 +177,12 @@ def deploy(config_file_name: str, address: str): short_help="Run a Serve app in a blocking way.", help=( "Runs the Serve app from the specified YAML config file or import path " - "for a deployment class or function (`my_module.MyClassOrFunction`).\n" - "If deploying via import path, you can pass args and kwargs to the " - "constructor using serve run -- [arg1]...[argn] " - "[--kwarg1=val1]...[--kwargn=valn].\n" + "to a bound deployment node or built application object.\n" "Blocks after deploying and logs status periodically. If you Ctrl-C " "this command, it tears down the app." ), ) @click.argument("config_or_import_path") -@click.argument("args_and_kwargs", required=False, nargs=-1) @click.option( "--runtime-env", type=str, @@ -305,58 +227,32 @@ def run( working_dir: str, address: str, ): - # Check if path provided is for config or import - is_config = pathlib.Path(config_or_import_path).is_file() - args, kwargs = _process_args_and_kwargs(args_and_kwargs) - - # Calculate deployments' runtime env updates requested via args - runtime_env_updates = parse_runtime_env_args( + final_runtime_env = parse_runtime_env_args( runtime_env=runtime_env, runtime_env_json=runtime_env_json, working_dir=working_dir, ) - # Create ray.init()'s runtime_env - if "working_dir" in runtime_env_updates: - ray_runtime_env = {"working_dir": runtime_env_updates.pop("working_dir")} - else: - ray_runtime_env = {} - - if is_config: + app_or_node = None + if pathlib.Path(config_or_import_path).is_file(): config_path = config_or_import_path - # Delay serve.start() to catch invalid inputs without waiting - if len(args) + len(kwargs) > 0: - raise ValueError( - "ARGS_AND_KWARGS cannot be defined for a " - "config file deployment. Please specify the " - "init_args and init_kwargs inside the config file." - ) - - cli_logger.print("Deploying application in config file at " f"{config_path}.") + cli_logger.print(f"Loading app from config file: '{config_path}'.") with open(config_path, "r") as config_file: - app = Application.from_yaml(config_file) - + app_or_node = Application.from_yaml(config_file) else: import_path = config_or_import_path if "." not in import_path: raise ValueError( "Import paths must be of the form " - '"module.submodule_1...submodule_n.MyClassOrFunction".' + "'module.submodule.app_or_bound_deployment'." ) - cli_logger.print(f'Deploying function or class imported from "{import_path}".') - - deployment_name = import_path[import_path.rfind(".") + 1 :] - deployment = serve.deployment(name=deployment_name)(import_path) - - app = Application([deployment.options(init_args=args, init_kwargs=kwargs)]) - - ray.init(address=address, namespace="serve", runtime_env=ray_runtime_env) - - for deployment in app: - _configure_runtime_env(deployment, runtime_env_updates) + cli_logger.print(f"Loading app from import path: '{import_path}'.") + app_or_node = import_attr(import_path) - app.run(logger=cli_logger) + # Setting the runtime_env here will set defaults for the deployments. + ray.init(address=address, namespace="serve", runtime_env=final_runtime_env) + serve.run(app_or_node, host=host, port=port, logger=cli_logger) @cli.command( diff --git a/python/ray/serve/tests/test_cli.py b/python/ray/serve/tests/test_cli.py index 7d7d0c037a8f..485d67a34252 100644 --- a/python/ray/serve/tests/test_cli.py +++ b/python/ray/serve/tests/test_cli.py @@ -29,192 +29,6 @@ def ray_start_stop(): subprocess.check_output(["ray", "stop", "--force"]) -class TestProcessArgsAndKwargs: - def test_valid_args_and_kwargs(self): - args_and_kwargs = ( - "argval1", - "argval2", - "--kwarg1", - "kwval1", - "--kwarg2", - "kwval2", - ) - args, kwargs = _process_args_and_kwargs(args_and_kwargs) - assert args == ["argval1", "argval2"] - assert kwargs == {"kwarg1": "kwval1", "kwarg2": "kwval2"} - - def test_mixed_args_and_kwargs(self): - args_and_kwargs = ( - "argval1", - "--kwarg1", - "kwval1", - "argval2", - "--kwarg2", - "kwval2", - ) - with pytest.raises(ValueError): - _process_args_and_kwargs(args_and_kwargs) - - def test_mixed_kwargs(self): - args_and_kwargs = ( - "argval1", - "argval2", - "--kwarg1==kw==val1", - "--kwarg2", - "kwval2", - "--kwarg3", - "=kwval=3", - "--kwarg4=", - "--kwarg5", - "kwval5", - ) - args, kwargs = _process_args_and_kwargs(args_and_kwargs) - assert args == ["argval1", "argval2"] - assert kwargs == { - "kwarg1": "=kw==val1", - "kwarg2": "kwval2", - "kwarg3": "=kwval=3", - "kwarg4": "", - "kwarg5": "kwval5", - } - - def test_empty_kwarg(self): - args_and_kwargs = ( - "argval1", - "--kwarg1", - "--kwarg2", - "kwval2", - ) - with pytest.raises(ValueError): - _process_args_and_kwargs(args_and_kwargs) - - args_and_kwargs = ("--empty_kwarg_only",) - with pytest.raises(ValueError): - _process_args_and_kwargs(args_and_kwargs) - - def test_empty_equals_kwarg(self): - args_and_kwargs = ( - "argval1", - "--kwarg1=--hello", - "--kwarg2=", - ) - args, kwargs = _process_args_and_kwargs(args_and_kwargs) - assert args == ["argval1"] - assert kwargs == { - "kwarg1": "--hello", - "kwarg2": "", - } - - args_and_kwargs = ("--empty_kwarg_only=",) - args, kwargs = _process_args_and_kwargs(args_and_kwargs) - assert args == [] - assert kwargs == {"empty_kwarg_only": ""} - - def test_only_args(self): - args_and_kwargs = ("argval1", "argval2", "argval3") - args, kwargs = _process_args_and_kwargs(args_and_kwargs) - assert args == ["argval1", "argval2", "argval3"] - assert kwargs == {} - - args_and_kwargs = ("single_arg",) - args, kwargs = _process_args_and_kwargs(args_and_kwargs) - assert args == ["single_arg"] - assert kwargs == {} - - def test_only_kwargs(self): - args_and_kwargs = ( - "--kwarg1", - "kwval1", - "--kwarg2", - "kwval2", - "--kwarg3", - "kwval3", - ) - args, kwargs = _process_args_and_kwargs(args_and_kwargs) - assert args == [] - assert kwargs == {"kwarg1": "kwval1", "kwarg2": "kwval2", "kwarg3": "kwval3"} - - args_and_kwargs = ( - "--single_kwarg", - "single_kwval", - ) - args, kwargs = _process_args_and_kwargs(args_and_kwargs) - assert args == [] - assert kwargs == {"single_kwarg": "single_kwval"} - - def test_empty_args_and_kwargs(self): - for empty_val in [None, ()]: - args, kwargs = _process_args_and_kwargs(empty_val) - assert args == [] - assert kwargs == {} - - -class TestConfigureRuntimeEnv: - @serve.deployment - def f(): - pass - - @pytest.mark.parametrize("ray_actor_options", [None, {}]) - def test_empty_ray_actor_options(self, ray_actor_options): - runtime_env = { - "working_dir": "http://test.com", - "pip": ["requests", "pendulum==2.1.2"], - } - deployment = TestConfigureRuntimeEnv.f.options( - ray_actor_options=ray_actor_options - ) - _configure_runtime_env(deployment, runtime_env) - assert deployment.ray_actor_options["runtime_env"] == runtime_env - - def test_no_overwrite_all_options(self): - old_runtime_env = { - "working_dir": "http://test.com", - "pip": ["requests", "pendulum==2.1.2"], - } - new_runtime_env = { - "working_dir": "http://new.com", - "pip": [], - "env_vars": {"test_var": "test"}, - } - updated_env = { - "working_dir": "http://test.com", - "pip": ["requests", "pendulum==2.1.2"], - "env_vars": {"test_var": "test"}, - } - deployment = TestConfigureRuntimeEnv.f.options( - ray_actor_options={"runtime_env": old_runtime_env} - ) - _configure_runtime_env(deployment, new_runtime_env) - assert deployment.ray_actor_options["runtime_env"] == updated_env - - def test_no_overwrite_some_options(self): - old_runtime_env = { - "working_dir": "http://new.com", - "pip": [], - "env_vars": {"test_var": "test"}, - } - new_runtime_env = { - "working_dir": "http://test.com", - "pip": ["requests", "pendulum==2.1.2"], - } - deployment = TestConfigureRuntimeEnv.f.options( - ray_actor_options={"runtime_env": old_runtime_env} - ) - _configure_runtime_env(deployment, new_runtime_env) - assert deployment.ray_actor_options["runtime_env"] == old_runtime_env - - def test_overwrite_no_options(self): - runtime_env = { - "working_dir": "http://test.com", - "pip": ["requests", "pendulum==2.1.2"], - } - deployment = TestConfigureRuntimeEnv.f.options( - ray_actor_options={"runtime_env": runtime_env} - ) - _configure_runtime_env(deployment, {}) - assert deployment.ray_actor_options["runtime_env"] == runtime_env - - def test_start_shutdown(ray_start_stop): with pytest.raises(subprocess.CalledProcessError): subprocess.check_output(["serve", "shutdown"]) From 30733fe4d4fcd7534c81106bcbf19f9c87ab6e1b Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Mon, 14 Mar 2022 14:59:53 -0500 Subject: [PATCH 31/94] remove deploy from python --- python/ray/serve/api.py | 32 +++++++------------------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 6ad600efdf72..1c9bb8bc19ff 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1644,42 +1644,24 @@ def __contains__(self, key: str): raise NotImplementedError() -NodeOrApp = Union[DeploymentNode, Application] - - @PublicAPI(stability="alpha") def run( - target: NodeOrApp, + target: Union[DeploymentNode, Application], *, + blocking: bool = False, host: str = DEFAULT_HTTP_HOST, port: int = DEFAULT_HTTP_PORT, logger: Optional[logging.Logger] = None, -): - """Run a Serve application (blocking). - - Deploys all of the deployments in the application then blocks, - periodically logging the status. Upon a KeyboardInterrupt, the application - will be torn down and all of its deployments deleted. - - Either a DeploymentNode or a pre-built application can be passed in. - If a DeploymentNode is passed in, all of the deployments it depends on - will be deployed. - """ - raise NotImplementedError() - - -@PublicAPI(stability="alpha") -def deploy( - target: NodeOrApp, *, host: str = DEFAULT_HTTP_HOST, port: int = DEFAULT_HTTP_PORT ) -> RayServeHandle: - """Deploy a Serve application async and return a handle to the ingress. - - Deploys all of the deployments in the application and returns a - RayServeHandle to the ingress deployment. + """Run a Serve application and return a ServeHandle to the ingress. Either a DeploymentNode or a pre-built application can be passed in. If a DeploymentNode is passed in, all of the deployments it depends on will be deployed. + + If blocking=True, this command will block and periodically print the + status of the deployments. On shutdown (KeyboardInterrupt or exit), all + deployments will be torn down. """ raise NotImplementedError() From 637fad2079c60ea044042a9f662f911401d81340 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Mon, 14 Mar 2022 13:01:32 -0700 Subject: [PATCH 32/94] Fix circular imports and update tests --- dashboard/modules/serve/serve_head.py | 4 +- dashboard/modules/serve/tests/test_schema.py | 597 +++++++++++++++++++ python/ray/serve/api.py | 103 +++- python/ray/serve/application.py | 8 +- python/ray/serve/schema.py | 91 +-- python/ray/serve/tests/test_application.py | 20 +- python/ray/serve/tests/test_schema.py | 10 +- 7 files changed, 717 insertions(+), 116 deletions(-) create mode 100644 dashboard/modules/serve/tests/test_schema.py diff --git a/dashboard/modules/serve/serve_head.py b/dashboard/modules/serve/serve_head.py index ce5ee329d0b9..6ed288e558a1 100644 --- a/dashboard/modules/serve/serve_head.py +++ b/dashboard/modules/serve/serve_head.py @@ -6,11 +6,11 @@ from ray import serve from ray.serve.application import Application -from ray.serve.schema import ( +from ray.serve.api import ( + get_deployment_statuses, serve_application_to_schema, serve_application_status_to_schema, ) -from ray.serve.api import get_deployment_statuses logger = logging.getLogger(__name__) logger.setLevel(logging.INFO) diff --git a/dashboard/modules/serve/tests/test_schema.py b/dashboard/modules/serve/tests/test_schema.py new file mode 100644 index 000000000000..0271509d9198 --- /dev/null +++ b/dashboard/modules/serve/tests/test_schema.py @@ -0,0 +1,597 @@ +import sys + +from pydantic import ValidationError + +import pytest + +import requests + +import ray +from ray.serve.schema import ( + RayActorOptionsSchema, + DeploymentSchema, + DeploymentStatusSchema, + ServeApplicationSchema, + ServeApplicationStatusSchema, +) +from ray.util.accelerators.accelerators import NVIDIA_TESLA_V100, NVIDIA_TESLA_P4 +from ray.serve.config import AutoscalingConfig +from ray.serve.common import DeploymentStatus, DeploymentStatusInfo +from ray.serve.api import ( + get_deployment_statuses, + deployment_to_schema, + schema_to_deployment, + serve_application_to_schema, + schema_to_serve_application, + status_info_to_schema, + serve_application_status_to_schema, +) +from ray import serve + + +class TestRayActorOptionsSchema: + def test_valid_ray_actor_options_schema(self): + # Ensure a valid RayActorOptionsSchema can be generated + + ray_actor_options_schema = { + "runtime_env": { + "working_dir": ( + "https://github.com/shrekris-anyscale/" + "test_module/archive/HEAD.zip" + ) + }, + "num_cpus": 0.2, + "num_gpus": 50, + "memory": 3, + "object_store_memory": 64, + "resources": {"custom_asic": 12}, + "accelerator_type": NVIDIA_TESLA_V100, + } + + RayActorOptionsSchema.parse_obj(ray_actor_options_schema) + + def test_ge_zero_ray_actor_options_schema(self): + # Ensure ValidationError is raised when any fields that must be greater + # than zero is set to zero. + + ge_zero_fields = ["num_cpus", "num_gpus", "memory", "object_store_memory"] + for field in ge_zero_fields: + with pytest.raises(ValidationError): + RayActorOptionsSchema.parse_obj({field: -1}) + + def test_runtime_env(self): + # Test different runtime_env configurations + + ray_actor_options_schema = { + "runtime_env": None, + "num_cpus": 0.2, + "num_gpus": 50, + "memory": 3, + "object_store_memory": 64, + "resources": {"custom_asic": 12}, + "accelerator_type": NVIDIA_TESLA_V100, + } + + # ray_actor_options_schema should work as is + RayActorOptionsSchema.parse_obj(ray_actor_options_schema) + + # working_dir and py_modules cannot contain local uris + ray_actor_options_schema["runtime_env"] = { + "working_dir": ".", + "py_modules": [ + "/Desktop/my_project", + ( + "https://github.com/shrekris-anyscale/" + "test_deploy_group/archive/HEAD.zip" + ), + ], + } + + with pytest.raises(ValueError): + RayActorOptionsSchema.parse_obj(ray_actor_options_schema) + + # remote uris should work + ray_actor_options_schema["runtime_env"] = { + "working_dir": ( + "https://github.com/shrekris-anyscale/test_module/archive/HEAD.zip" + ), + "py_modules": [ + ( + "https://github.com/shrekris-anyscale/" + "test_deploy_group/archive/HEAD.zip" + ), + ], + } + + RayActorOptionsSchema.parse_obj(ray_actor_options_schema) + + def test_extra_fields_invalid_ray_actor_options(self): + # Undefined fields should be forbidden in the schema + + ray_actor_options_schema = { + "runtime_env": None, + "num_cpus": None, + "num_gpus": None, + "memory": None, + "object_store_memory": None, + "resources": None, + "accelerator_type": None, + } + + # Schema should be createable with valid fields + RayActorOptionsSchema.parse_obj(ray_actor_options_schema) + + # Schema should raise error when a nonspecified field is included + ray_actor_options_schema["fake_field"] = None + with pytest.raises(ValidationError): + RayActorOptionsSchema.parse_obj(ray_actor_options_schema) + + +class TestDeploymentSchema: + def get_minimal_deployment_schema(self): + # Generate a DeploymentSchema with the fewest possible attributes set + + return { + "name": "deep", + "init_args": None, + "init_kwargs": None, + "import_path": "my_module.MyClass", + "num_replicas": None, + "route_prefix": None, + "max_concurrent_queries": None, + "user_config": None, + "autoscaling_config": None, + "graceful_shutdown_wait_loop_s": None, + "graceful_shutdown_timeout_s": None, + "health_check_period_s": None, + "health_check_timeout_s": None, + "ray_actor_options": { + "runtime_env": None, + "num_cpus": None, + "num_gpus": None, + "memory": None, + "object_store_memory": None, + "resources": None, + "accelerator_type": None, + }, + } + + def test_valid_deployment_schema(self): + # Ensure a valid DeploymentSchema can be generated + + deployment_schema = { + "name": "shallow", + "init_args": [4, "glue"], + "init_kwargs": {"fuel": "diesel"}, + "import_path": "test_env.shallow_import.ShallowClass", + "num_replicas": 2, + "route_prefix": "/shallow", + "max_concurrent_queries": 32, + "user_config": {"threshold": 0.2, "pattern": "rainbow"}, + "autoscaling_config": None, + "graceful_shutdown_wait_loop_s": 17, + "graceful_shutdown_timeout_s": 49, + "health_check_period_s": 11, + "health_check_timeout_s": 11, + "ray_actor_options": { + "runtime_env": { + "working_dir": ( + "https://github.com/shrekris-anyscale/" + "test_module/archive/HEAD.zip" + ), + "py_modules": [ + ( + "https://github.com/shrekris-anyscale/" + "test_deploy_group/archive/HEAD.zip" + ), + ], + }, + "num_cpus": 3, + "num_gpus": 4.2, + "memory": 5, + "object_store_memory": 3, + "resources": {"custom_asic": 8}, + "accelerator_type": NVIDIA_TESLA_P4, + }, + } + + DeploymentSchema.parse_obj(deployment_schema) + + def test_invalid_python_attributes(self): + # Test setting invalid attributes for Python to ensure a validation or + # value error is raised. + + # Python requires an import path + deployment_schema = self.get_minimal_deployment_schema() + deployment_schema["init_args"] = [1, 2] + deployment_schema["init_kwargs"] = {"threshold": 0.5} + del deployment_schema["import_path"] + + with pytest.raises(ValueError, match="must be specified"): + DeploymentSchema.parse_obj(deployment_schema) + + # DeploymentSchema should be generated once import_path is set + deployment_schema["import_path"] = "my_module.MyClass" + DeploymentSchema.parse_obj(deployment_schema) + + # Invalid import_path syntax should raise a ValidationError + invalid_paths = ["", "MyClass", ".", "hello,world"] + for path in invalid_paths: + deployment_schema["import_path"] = path + with pytest.raises(ValidationError): + DeploymentSchema.parse_obj(deployment_schema) + + def test_gt_zero_deployment_schema(self): + # Ensure ValidationError is raised when any fields that must be greater + # than zero is set to zero. + + deployment_schema = self.get_minimal_deployment_schema() + + gt_zero_fields = [ + "num_replicas", + "max_concurrent_queries", + "health_check_period_s", + "health_check_timeout_s", + ] + for field in gt_zero_fields: + deployment_schema[field] = 0 + with pytest.raises(ValidationError): + DeploymentSchema.parse_obj(deployment_schema) + deployment_schema[field] = None + + def test_ge_zero_deployment_schema(self): + # Ensure ValidationError is raised when any fields that must be greater + # than or equal to zero is set to -1. + + deployment_schema = self.get_minimal_deployment_schema() + + ge_zero_fields = [ + "graceful_shutdown_wait_loop_s", + "graceful_shutdown_timeout_s", + ] + + for field in ge_zero_fields: + deployment_schema[field] = -1 + with pytest.raises(ValidationError): + DeploymentSchema.parse_obj(deployment_schema) + deployment_schema[field] = None + + def test_route_prefix(self): + # Ensure that route_prefix is validated + + deployment_schema = self.get_minimal_deployment_schema() + + # route_prefix must start with a "/" + deployment_schema["route_prefix"] = "hello/world" + with pytest.raises(ValueError): + DeploymentSchema.parse_obj(deployment_schema) + + # route_prefix must end with a "/" + deployment_schema["route_prefix"] = "/hello/world/" + with pytest.raises(ValueError): + DeploymentSchema.parse_obj(deployment_schema) + + # route_prefix cannot contain wildcards, meaning it can't have + # "{" or "}" + deployment_schema["route_prefix"] = "/hello/{adjective}/world/" + with pytest.raises(ValueError): + DeploymentSchema.parse_obj(deployment_schema) + + # Ensure a valid route_prefix works + deployment_schema["route_prefix"] = "/hello/wonderful/world" + DeploymentSchema.parse_obj(deployment_schema) + + # Ensure route_prefix of "/" works + deployment_schema["route_prefix"] = "/" + DeploymentSchema.parse_obj(deployment_schema) + + # Ensure route_prefix of None works + deployment_schema["route_prefix"] = None + DeploymentSchema.parse_obj(deployment_schema) + + def test_mutually_exclusive_num_replicas_and_autoscaling_config(self): + # num_replicas and autoscaling_config cannot be set at the same time + deployment_schema = self.get_minimal_deployment_schema() + + deployment_schema["num_replicas"] = 5 + deployment_schema["autoscaling_config"] = None + DeploymentSchema.parse_obj(deployment_schema) + + deployment_schema["num_replicas"] = None + deployment_schema["autoscaling_config"] = AutoscalingConfig().dict() + DeploymentSchema.parse_obj(deployment_schema) + + deployment_schema["num_replicas"] = 5 + deployment_schema["autoscaling_config"] = AutoscalingConfig().dict() + with pytest.raises(ValueError): + DeploymentSchema.parse_obj(deployment_schema) + + def test_extra_fields_invalid_deployment_schema(self): + # Undefined fields should be forbidden in the schema + + deployment_schema = self.get_minimal_deployment_schema() + + # Schema should be createable with valid fields + DeploymentSchema.parse_obj(deployment_schema) + + # Schema should raise error when a nonspecified field is included + deployment_schema["fake_field"] = None + with pytest.raises(ValidationError): + DeploymentSchema.parse_obj(deployment_schema) + + +class TestServeApplicationSchema: + def get_valid_serve_application_schema(self): + return { + "deployments": [ + { + "name": "shallow", + "init_args": [4, "glue"], + "init_kwargs": {"fuel": "diesel"}, + "import_path": "test_env.shallow_import.ShallowClass", + "num_replicas": 2, + "route_prefix": "/shallow", + "max_concurrent_queries": 32, + "user_config": None, + "autoscaling_config": None, + "graceful_shutdown_wait_loop_s": 17, + "graceful_shutdown_timeout_s": 49, + "health_check_period_s": 11, + "health_check_timeout_s": 11, + "ray_actor_options": { + "runtime_env": { + "working_dir": ( + "https://github.com/shrekris-anyscale/" + "test_module/archive/HEAD.zip" + ), + "py_modules": [ + ( + "https://github.com/shrekris-anyscale/" + "test_deploy_group/archive/HEAD.zip" + ), + ], + }, + "num_cpus": 3, + "num_gpus": 4.2, + "memory": 5, + "object_store_memory": 3, + "resources": {"custom_asic": 8}, + "accelerator_type": NVIDIA_TESLA_P4, + }, + }, + { + "name": "deep", + "init_args": None, + "init_kwargs": None, + "import_path": ("test_env.subdir1.subdir2.deep_import.DeepClass"), + "num_replicas": None, + "route_prefix": None, + "max_concurrent_queries": None, + "user_config": None, + "autoscaling_config": None, + "graceful_shutdown_wait_loop_s": None, + "graceful_shutdown_timeout_s": None, + "health_check_period_s": None, + "health_check_timeout_s": None, + "ray_actor_options": { + "runtime_env": None, + "num_cpus": None, + "num_gpus": None, + "memory": None, + "object_store_memory": None, + "resources": None, + "accelerator_type": None, + }, + }, + ] + } + + def test_valid_serve_application_schema(self): + # Ensure a valid ServeApplicationSchema can be generated + + serve_application_schema = self.get_valid_serve_application_schema() + ServeApplicationSchema.parse_obj(serve_application_schema) + + def test_extra_fields_invalid_serve_application_schema(self): + # Undefined fields should be forbidden in the schema + + serve_application_schema = self.get_valid_serve_application_schema() + + # Schema should be createable with valid fields + ServeApplicationSchema.parse_obj(serve_application_schema) + + # Schema should raise error when a nonspecified field is included + serve_application_schema["fake_field"] = None + with pytest.raises(ValidationError): + ServeApplicationSchema.parse_obj(serve_application_schema) + + +class TestDeploymentStatusSchema: + def get_valid_deployment_status_schema(self): + return { + "deployment_1": DeploymentStatusInfo(DeploymentStatus.HEALTHY), + "deployment_2": DeploymentStatusInfo( + DeploymentStatus.UNHEALTHY, "This is an unhealthy deployment." + ), + "deployment_3": DeploymentStatusInfo(DeploymentStatus.UPDATING), + } + + def test_valid_deployment_status_schema(self): + # Ensure valid DeploymentStatusSchemas can be generated + + deployment_status_schemas = self.get_valid_deployment_status_schema() + + for name, status_info in deployment_status_schemas.items(): + status_info_to_schema(name, status_info) + + def test_invalid_status(self): + # Ensure a DeploymentStatusSchema cannot be initialized with an invalid status + + status_info = { + "status": "nonexistent status", + "message": "welcome to nonexistence", + } + with pytest.raises(ValidationError): + status_info_to_schema("deployment name", status_info) + + def test_extra_fields_invalid_deployment_status_schema(self): + # Undefined fields should be forbidden in the schema + + deployment_status_schemas = self.get_valid_deployment_status_schema() + + # Schema should be createable with valid fields + for name, status_info in deployment_status_schemas.items(): + DeploymentStatusSchema( + name=name, status=status_info.status, message=status_info.message + ) + + # Schema should raise error when a nonspecified field is included + for name, status_info in deployment_status_schemas.items(): + with pytest.raises(ValidationError): + DeploymentStatusSchema( + name=name, + status=status_info.status, + message=status_info.message, + fake_field=None, + ) + + +class TestServeApplicationStatusSchema: + def get_valid_serve_application_status_schema(self): + return { + "deployment_1": {"status": "HEALTHY", "message": ""}, + "deployment_2": { + "status": "UNHEALTHY", + "message": "this deployment is deeply unhealthy", + }, + } + + def test_valid_serve_application_status_schema(self): + # Ensure a valid ServeApplicationStatusSchema can be generated + + serve_application_status_schema = ( + self.get_valid_serve_application_status_schema() + ) + serve_application_status_to_schema(serve_application_status_schema) + + def test_extra_fields_invalid_serve_application_status_schema(self): + # Undefined fields should be forbidden in the schema + + serve_application_status_schema = ( + self.get_valid_serve_application_status_schema() + ) + + # Schema should be createable with valid fields + serve_application_status_to_schema(serve_application_status_schema) + + # Schema should raise error when a nonspecified field is included + with pytest.raises(ValidationError): + statuses = [ + status_info_to_schema(name, status_info) + for name, status_info in serve_application_status_schema.items() + ] + ServeApplicationStatusSchema(statuses=statuses, fake_field=None) + + +# This function is defined globally to be accessible via import path +def global_f(): + return "Hello world!" + + +def test_deployment_to_schema_to_deployment(): + @serve.deployment( + num_replicas=3, + route_prefix="/hello", + ray_actor_options={ + "runtime_env": { + "working_dir": ( + "https://github.com/shrekris-anyscale/" + "test_module/archive/HEAD.zip" + ), + "py_modules": [ + ( + "https://github.com/shrekris-anyscale/" + "test_deploy_group/archive/HEAD.zip" + ), + ], + } + }, + ) + def f(): + # The body of this function doesn't matter. It gets replaced by + # global_f() when the import path in f._func_or_class is overwritten. + # This function is used as a convenience to apply the @serve.deployment + # decorator without converting global_f() into a Deployment object. + pass + + f._func_or_class = "ray.serve.tests.test_schema.global_f" + + deployment = schema_to_deployment(deployment_to_schema(f)) + + assert deployment.num_replicas == 3 + assert deployment.route_prefix == "/hello" + assert deployment.ray_actor_options["runtime_env"]["working_dir"] == ( + "https://github.com/shrekris-anyscale/test_module/archive/HEAD.zip" + ) + assert deployment.ray_actor_options["runtime_env"]["py_modules"] == [ + "https://github.com/shrekris-anyscale/test_deploy_group/archive/HEAD.zip", + "https://github.com/shrekris-anyscale/test_module/archive/HEAD.zip", + ] + + serve.start() + deployment.deploy() + assert ray.get(deployment.get_handle().remote()) == "Hello world!" + assert requests.get("http://localhost:8000/hello").text == "Hello world!" + serve.shutdown() + + +def test_serve_application_to_schema_to_serve_application(): + @serve.deployment( + num_replicas=1, + route_prefix="/hello", + ) + def f1(): + # The body of this function doesn't matter. See the comment in + # test_deployment_to_schema_to_deployment. + pass + + @serve.deployment( + num_replicas=2, + route_prefix="/hi", + ) + def f2(): + pass + + f1._func_or_class = "ray.serve.tests.test_schema.global_f" + f2._func_or_class = "ray.serve.tests.test_schema.global_f" + + deployments = schema_to_serve_application(serve_application_to_schema([f1, f2])) + + assert deployments[0].num_replicas == 1 + assert deployments[0].route_prefix == "/hello" + assert deployments[1].num_replicas == 2 + assert deployments[1].route_prefix == "/hi" + + serve.start() + + deployments[0].deploy() + deployments[1].deploy() + assert ray.get(deployments[0].get_handle().remote()) == "Hello world!" + assert requests.get("http://localhost:8000/hello").text == "Hello world!" + assert ray.get(deployments[1].get_handle().remote()) == "Hello world!" + assert requests.get("http://localhost:8000/hi").text == "Hello world!" + + # Check statuses + statuses = serve_application_status_to_schema(get_deployment_statuses()).statuses + deployment_names = {"f1", "f2"} + for deployment_status in statuses: + assert deployment_status.status in {"UPDATING", "HEALTHY"} + assert deployment_status.name in deployment_names + deployment_names.remove(deployment_status.name) + assert len(deployment_names) == 0 + + serve.shutdown() + + +if __name__ == "__main__": + sys.exit(pytest.main(["-v", __file__])) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 438e92938af4..61ca30017c3e 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -66,12 +66,14 @@ import ray from ray import cloudpickle from ray.serve.schema import ( + RayActorOptionsSchema, + DeploymentSchema, + DeploymentStatusSchema, ServeApplicationSchema, - serve_application_to_schema, - schema_to_serve_application, - serve_application_status_to_schema, + ServeApplicationStatusSchema, ) -from ray.serve.pipeline.api import extract_deployments_from_serve_dag + +# from ray.serve.pipeline.api import extract_deployments_from_serve_dag _INTERNAL_REPLICA_CONTEXT = None @@ -1535,7 +1537,7 @@ class Application: # TODO(edoakes): should this literally just be the REST pydantic schema? # At the very least, it should be a simple dictionary. - def __init__(self, deployments: List[Deployment]=None): + def __init__(self, deployments: List[Deployment] = None): deployments = deployments or [] self._deployments = dict() @@ -1812,10 +1814,99 @@ def _get_deployments_from_target(target: NodeOrApp) -> List[Deployment]: if isinstance(target, Application): return target._get_deployments() elif isinstance(target, DeploymentNode): - return extract_deployments_from_serve_dag(target) + return [] + # return extract_deployments_from_serve_dag(target) else: raise TypeError( "Expected a DeploymentNode or " "Application as target. Got unexpected type " f'"{type(target)}" instead.' ) + + +def deployment_to_schema(d: Deployment) -> DeploymentSchema: + if d.ray_actor_options is not None: + ray_actor_options_schema = RayActorOptionsSchema.parse_obj(d.ray_actor_options) + else: + ray_actor_options_schema = None + + return DeploymentSchema( + name=d.name, + import_path=d.func_or_class, + init_args=d.init_args, + init_kwargs=d.init_kwargs, + num_replicas=d.num_replicas, + route_prefix=d.route_prefix, + max_concurrent_queries=d.max_concurrent_queries, + user_config=d.user_config, + autoscaling_config=d._config.autoscaling_config, + graceful_shutdown_wait_loop_s=d._config.graceful_shutdown_wait_loop_s, + graceful_shutdown_timeout_s=d._config.graceful_shutdown_timeout_s, + health_check_period_s=d._config.health_check_period_s, + health_check_timeout_s=d._config.health_check_timeout_s, + ray_actor_options=ray_actor_options_schema, + ) + + +def schema_to_deployment(s: DeploymentSchema) -> Deployment: + if s.ray_actor_options is None: + ray_actor_options = None + else: + ray_actor_options = s.ray_actor_options.dict(exclude_unset=True) + + return deployment( + name=s.name, + num_replicas=s.num_replicas, + init_args=s.init_args, + init_kwargs=s.init_kwargs, + route_prefix=s.route_prefix, + ray_actor_options=ray_actor_options, + max_concurrent_queries=s.max_concurrent_queries, + _autoscaling_config=s.autoscaling_config, + _graceful_shutdown_wait_loop_s=s.graceful_shutdown_wait_loop_s, + _graceful_shutdown_timeout_s=s.graceful_shutdown_timeout_s, + _health_check_period_s=s.health_check_period_s, + _health_check_timeout_s=s.health_check_timeout_s, + )(s.import_path) + + +def serve_application_to_schema( + deployments: List[Deployment], +) -> ServeApplicationSchema: + schemas = [deployment_to_schema(d) for d in deployments] + return ServeApplicationSchema(deployments=schemas) + + +def schema_to_serve_application(schema: ServeApplicationSchema) -> List[Deployment]: + return [schema_to_deployment(s) for s in schema.deployments] + + +def status_info_to_schema( + deployment_name: str, status_info: Union[DeploymentStatusInfo, Dict] +) -> DeploymentStatusSchema: + if isinstance(status_info, DeploymentStatusInfo): + return DeploymentStatusSchema( + name=deployment_name, status=status_info.status, message=status_info.message + ) + elif isinstance(status_info, dict): + return DeploymentStatusSchema( + name=deployment_name, + status=status_info["status"], + message=status_info["message"], + ) + else: + raise TypeError( + f"Got {type(status_info)} as status_info's " + "type. Expected status_info to be either a " + "DeploymentStatusInfo or a dictionary." + ) + + +def serve_application_status_to_schema( + status_infos: Dict[str, Union[DeploymentStatusInfo, Dict]] +) -> ServeApplicationStatusSchema: + schemas = [ + status_info_to_schema(deployment_name, status_info) + for deployment_name, status_info in status_infos.items() + ] + return ServeApplicationStatusSchema(statuses=schemas) diff --git a/python/ray/serve/application.py b/python/ray/serve/application.py index 0f666547817d..6662f1baecb5 100644 --- a/python/ray/serve/application.py +++ b/python/ray/serve/application.py @@ -8,14 +8,14 @@ Deployment, internal_get_global_client, ) -from ray.serve.schema import ( - ServeApplicationSchema, +from ray.serve.schema import ServeApplicationSchema +from ray.serve.utils import logger +from ray.serve.api import ( + get_deployment_statuses, serve_application_to_schema, schema_to_serve_application, serve_application_status_to_schema, ) -from ray.serve.utils import logger -from ray.serve.api import get_deployment_statuses from ray.autoscaler._private.cli_logger import _CliLogger from logging import Logger diff --git a/python/ray/serve/schema.py b/python/ray/serve/schema.py index cf1b34f59b76..63acb50a94c5 100644 --- a/python/ray/serve/schema.py +++ b/python/ray/serve/schema.py @@ -1,8 +1,7 @@ from pydantic import BaseModel, Field, Extra, root_validator, validator from typing import Union, Tuple, List, Dict from ray._private.runtime_env.packaging import parse_uri -from ray.serve.api import Deployment, deployment -from ray.serve.common import DeploymentStatus, DeploymentStatusInfo +from ray.serve.common import DeploymentStatus from ray.serve.utils import DEFAULT @@ -311,91 +310,3 @@ class DeploymentStatusSchema(BaseModel, extra=Extra.forbid): class ServeApplicationStatusSchema(BaseModel, extra=Extra.forbid): statuses: List[DeploymentStatusSchema] = Field(...) - - -def deployment_to_schema(d: Deployment) -> DeploymentSchema: - if d.ray_actor_options is not None: - ray_actor_options_schema = RayActorOptionsSchema.parse_obj(d.ray_actor_options) - else: - ray_actor_options_schema = None - - return DeploymentSchema( - name=d.name, - import_path=d.func_or_class, - init_args=d.init_args, - init_kwargs=d.init_kwargs, - num_replicas=d.num_replicas, - route_prefix=d.route_prefix, - max_concurrent_queries=d.max_concurrent_queries, - user_config=d.user_config, - autoscaling_config=d._config.autoscaling_config, - graceful_shutdown_wait_loop_s=d._config.graceful_shutdown_wait_loop_s, - graceful_shutdown_timeout_s=d._config.graceful_shutdown_timeout_s, - health_check_period_s=d._config.health_check_period_s, - health_check_timeout_s=d._config.health_check_timeout_s, - ray_actor_options=ray_actor_options_schema, - ) - - -def schema_to_deployment(s: DeploymentSchema) -> Deployment: - if s.ray_actor_options is None: - ray_actor_options = None - else: - ray_actor_options = s.ray_actor_options.dict(exclude_unset=True) - - return deployment( - name=s.name, - num_replicas=s.num_replicas, - init_args=s.init_args, - init_kwargs=s.init_kwargs, - route_prefix=s.route_prefix, - ray_actor_options=ray_actor_options, - max_concurrent_queries=s.max_concurrent_queries, - _autoscaling_config=s.autoscaling_config, - _graceful_shutdown_wait_loop_s=s.graceful_shutdown_wait_loop_s, - _graceful_shutdown_timeout_s=s.graceful_shutdown_timeout_s, - _health_check_period_s=s.health_check_period_s, - _health_check_timeout_s=s.health_check_timeout_s, - )(s.import_path) - - -def serve_application_to_schema( - deployments: List[Deployment], -) -> ServeApplicationSchema: - schemas = [deployment_to_schema(d) for d in deployments] - return ServeApplicationSchema(deployments=schemas) - - -def schema_to_serve_application(schema: ServeApplicationSchema) -> List[Deployment]: - return [schema_to_deployment(s) for s in schema.deployments] - - -def status_info_to_schema( - deployment_name: str, status_info: Union[DeploymentStatusInfo, Dict] -) -> DeploymentStatusSchema: - if isinstance(status_info, DeploymentStatusInfo): - return DeploymentStatusSchema( - name=deployment_name, status=status_info.status, message=status_info.message - ) - elif isinstance(status_info, dict): - return DeploymentStatusSchema( - name=deployment_name, - status=status_info["status"], - message=status_info["message"], - ) - else: - raise TypeError( - f"Got {type(status_info)} as status_info's " - "type. Expected status_info to be either a " - "DeploymentStatusInfo or a dictionary." - ) - - -def serve_application_status_to_schema( - status_infos: Dict[str, Union[DeploymentStatusInfo, Dict]] -) -> ServeApplicationStatusSchema: - schemas = [ - status_info_to_schema(deployment_name, status_info) - for deployment_name, status_info in status_infos.items() - ] - return ServeApplicationStatusSchema(statuses=schemas) diff --git a/python/ray/serve/tests/test_application.py b/python/ray/serve/tests/test_application.py index 5f321cc01bcc..f5ae25247156 100644 --- a/python/ray/serve/tests/test_application.py +++ b/python/ray/serve/tests/test_application.py @@ -8,7 +8,7 @@ import ray from ray import serve -from ray.serve.application import Application +from ray.serve.api import Application from ray._private.test_utils import wait_for_condition @@ -24,8 +24,8 @@ def __call__(self, *args): def test_add_deployment_valid(self): app = Application() - app.add_deployment(self.f) - app.add_deployment(self.C) + app._add_deployment(self.f) + app._add_deployment(self.C) assert len(app) == 2 assert "f" in app @@ -34,8 +34,8 @@ def test_add_deployment_valid(self): def test_add_deployment_repeat_name(self): with pytest.raises(ValueError): app = Application() - app.add_deployment(self.f) - app.add_deployment(self.C.options(name="f")) + app._add_deployment(self.f) + app._add_deployment(self.C.options(name="f")) with pytest.raises(ValueError): Application([self.C, self.f.options(name="C")]) @@ -70,7 +70,7 @@ def deploy_and_check_responses( the client to wait until the deployments finish deploying. """ - Application(deployments).deploy(blocking=blocking) + serve.deploy(target=Application(deployments), blocking=blocking) def check_all_deployed(): try: @@ -143,7 +143,7 @@ async def request_echo(self, echo: str): MutualHandles.options(name=deployment_name, init_args=(handle_name,)) ) - Application(deployments).deploy(blocking=True) + serve.deploy(target=Application(deployments), blocking=True) for deployment in deployments: assert (ray.get(deployment.get_handle().remote("hello"))) == "hello" @@ -308,7 +308,7 @@ def test_deploy_from_dict(self, serve_instance): compare_specified_options(config_dict, app_dict) - app.deploy() + serve.deploy(app) assert ( requests.get("http://localhost:8000/shallow").text == "Hello shallow world!" @@ -334,7 +334,7 @@ def test_deploy_from_yaml(self, serve_instance): compare_specified_options(app1.to_dict(), app2.to_dict()) # Check that deployment works - app1.deploy() + serve.deploy(app1) assert ( requests.get("http://localhost:8000/shallow").text == "Hello shallow world!" ) @@ -354,7 +354,7 @@ def test_deploy_from_yaml(self, serve_instance): @pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") -def test_get_set_item(serve_instance): +def test_get_item(serve_instance): config_file_name = os.path.join( os.path.dirname(__file__), "test_config_files", "two_deployments.yaml" ) diff --git a/python/ray/serve/tests/test_schema.py b/python/ray/serve/tests/test_schema.py index 37cc7b9d4d4a..a038010c2dd8 100644 --- a/python/ray/serve/tests/test_schema.py +++ b/python/ray/serve/tests/test_schema.py @@ -13,6 +13,12 @@ DeploymentStatusSchema, ServeApplicationSchema, ServeApplicationStatusSchema, +) +from ray.util.accelerators.accelerators import NVIDIA_TESLA_V100, NVIDIA_TESLA_P4 +from ray.serve.config import AutoscalingConfig +from ray.serve.common import DeploymentStatus, DeploymentStatusInfo +from ray.serve.api import ( + get_deployment_statuses, deployment_to_schema, schema_to_deployment, serve_application_to_schema, @@ -20,10 +26,6 @@ status_info_to_schema, serve_application_status_to_schema, ) -from ray.util.accelerators.accelerators import NVIDIA_TESLA_V100, NVIDIA_TESLA_P4 -from ray.serve.config import AutoscalingConfig -from ray.serve.common import DeploymentStatus, DeploymentStatusInfo -from ray.serve.api import get_deployment_statuses from ray import serve From 7eaef418419e5e9e06b1d1461b9200fb678f6298 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Mon, 14 Mar 2022 13:10:46 -0700 Subject: [PATCH 33/94] Make application pass tests --- python/ray/serve/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 61ca30017c3e..ef3ba9413db9 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1763,7 +1763,7 @@ def deploy( parameter_group = [] - for deployment in deployments.values(): + for deployment in deployments: deployment_parameters = { "name": deployment._name, From c7b0797ce4a1c8193ce43d6bb26c78a66f2e1e3d Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Mon, 14 Mar 2022 13:11:48 -0700 Subject: [PATCH 34/94] Port application users to api --- dashboard/modules/serve/serve_head.py | 2 +- python/ray/serve/scripts.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dashboard/modules/serve/serve_head.py b/dashboard/modules/serve/serve_head.py index 6ed288e558a1..fb339aaaa3a5 100644 --- a/dashboard/modules/serve/serve_head.py +++ b/dashboard/modules/serve/serve_head.py @@ -5,7 +5,7 @@ import ray.dashboard.optional_utils as optional_utils from ray import serve -from ray.serve.application import Application +from ray.serve.api import Application from ray.serve.api import ( get_deployment_statuses, serve_application_to_schema, diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index e5ddc41a1375..c1e35a369193 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -19,7 +19,7 @@ from ray.dashboard.modules.dashboard_sdk import parse_runtime_env_args from ray.dashboard.modules.serve.sdk import ServeSubmissionClient from ray.autoscaler._private.cli_logger import cli_logger -from ray.serve.application import Application +from ray.serve.api import Application from ray.serve.api import Deployment RAY_INIT_ADDRESS_HELP_STR = ( From e41aec58d8dd337b75d514d22d3fd771fbea4be7 Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Mon, 14 Mar 2022 15:14:13 -0500 Subject: [PATCH 35/94] fix --- python/ray/serve/scripts.py | 64 +++++++++++++++++------------- python/ray/serve/tests/test_cli.py | 1 - 2 files changed, 36 insertions(+), 29 deletions(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 9eaed5724cce..01ee49a25737 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -4,8 +4,6 @@ import os import pathlib import click -from typing import Tuple, List, Dict -import argparse import ray from ray._private.utils import import_attr @@ -21,7 +19,6 @@ from ray.dashboard.modules.serve.sdk import ServeSubmissionClient from ray.autoscaler._private.cli_logger import cli_logger from ray.serve.application import Application -from ray.serve.api import Deployment RAY_INIT_ADDRESS_HELP_STR = ( "Address to use for ray.init(). Can also be specified " @@ -107,7 +104,7 @@ def start( ) -@cli.command(help="Shut down the running Serve instance on the Ray cluster.") +@cli.command(help="Shut down the running Serve app on the Ray cluster.") @click.option( "--address", "-a", @@ -153,22 +150,23 @@ def shutdown(address: str, namespace: str): type=str, help=RAY_DASHBOARD_ADDRESS_HELP_STR, ) +@click.option( + "--blocking/--non-blocking", + default=False, +) def deploy(config_file_name: str, address: str): - with open(config_file_name, "r") as config_file: config = yaml.safe_load(config_file) - # Schematize config to validate format + # Schematize config to validate format. ServeApplicationSchema.parse_obj(config) - ServeSubmissionClient(address).deploy_application(config) cli_logger.newline() cli_logger.success( "\nSent deploy request successfully!\n " - "* Use `serve status` to check your deployments' statuses.\n " - "* Use `serve info` to see your running Serve " - "application's configuration.\n" + "* Use `serve status` to check deployments' statuses.\n " + "* Use `serve config` to see the running app's config.\n" ) cli_logger.newline() @@ -219,13 +217,35 @@ def deploy(config_file_name: str, address: str): type=str, help=RAY_INIT_ADDRESS_HELP_STR, ) +@click.option( + "--host", + "-h", + default=DEFAULT_HTTP_HOST, + required=False, + type=str, + help=f"Host for HTTP server to listen on. Defaults to {DEFAULT_HTTP_HOST}.", +) +@click.option( + "--port", + "-p", + default=DEFAULT_HTTP_PORT, + required=False, + type=int, + help=f"Port for HTTP servers to listen on. Defaults to {DEFAULT_HTTP_PORT}.", +) +@click.option( + "--blocking/--non-blocking", + default=True, +) def run( config_or_import_path: str, - args_and_kwargs: Tuple[str], runtime_env: str, runtime_env_json: str, working_dir: str, address: str, + host: str, + port: int, + blocking: bool, ): final_runtime_env = parse_runtime_env_args( runtime_env=runtime_env, @@ -256,8 +276,7 @@ def run( @cli.command( - short_help="Get the current config of the running Serve app.", - help=("Prints the configurations of all running deployments in the Serve app."), + help="Get the current config of the running Serve app.", ) @click.option( "--address", @@ -267,24 +286,15 @@ def run( type=str, help=RAY_DASHBOARD_ADDRESS_HELP_STR, ) -@click.option( - "--json_format", - "-j", - is_flag=True, - help="Print info as json. If omitted, info is printed as YAML.", -) -def info(address: str, json_format=bool): +def config(address: str, json_format=bool): app_info = ServeSubmissionClient(address).get_info() if app_info is not None: - if json_format: - print(json.dumps(app_info, indent=4)) - else: - print(yaml.dump(app_info)) + print(yaml.dump(app_info)) @cli.command( - short_help="Get the current status of the Serve app.", + short_help="Get the current status of the running Serve app.", help=( "Prints status information about all deployments in the Serve app.\n\n" "Deployments may be:\n\n" @@ -310,8 +320,7 @@ def status(address: str): @cli.command( - short_help=("Deletes all running deployments in the Serve app."), - help="Deletes all running deployments in the Serve app.", + help="Deletes all deployments in the Serve app.", ) @click.option( "--address", @@ -323,7 +332,6 @@ def status(address: str): ) @click.option("--yes", "-y", is_flag=True, help="Bypass confirmation prompt.") def delete(address: str, yes: bool): - if not yes: click.confirm( f"\nThis will shutdown the Serve application at address " diff --git a/python/ray/serve/tests/test_cli.py b/python/ray/serve/tests/test_cli.py index 485d67a34252..b508dc43699e 100644 --- a/python/ray/serve/tests/test_cli.py +++ b/python/ray/serve/tests/test_cli.py @@ -12,7 +12,6 @@ from ray.tests.conftest import tmp_working_dir # noqa: F401, E501 from ray._private.test_utils import wait_for_condition from ray.dashboard.optional_utils import RAY_INTERNAL_DASHBOARD_NAMESPACE -from ray.serve.scripts import _process_args_and_kwargs, _configure_runtime_env def ping_endpoint(endpoint: str, params: str = ""): From 8de474f5153386d92e8bcb96a2c1578b3f00deac Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Mon, 14 Mar 2022 15:15:00 -0500 Subject: [PATCH 36/94] remove blocking from python --- python/ray/serve/api.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 1c9bb8bc19ff..af79bb42df73 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1648,7 +1648,6 @@ def __contains__(self, key: str): def run( target: Union[DeploymentNode, Application], *, - blocking: bool = False, host: str = DEFAULT_HTTP_HOST, port: int = DEFAULT_HTTP_PORT, logger: Optional[logging.Logger] = None, @@ -1658,10 +1657,6 @@ def run( Either a DeploymentNode or a pre-built application can be passed in. If a DeploymentNode is passed in, all of the deployments it depends on will be deployed. - - If blocking=True, this command will block and periodically print the - status of the deployments. On shutdown (KeyboardInterrupt or exit), all - deployments will be torn down. """ raise NotImplementedError() From ae8331d961ffcee3e13dff5a57b442837badb5f1 Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Mon, 14 Mar 2022 15:16:04 -0500 Subject: [PATCH 37/94] remove logger --- python/ray/serve/api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index af79bb42df73..2cb470c7a389 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1650,7 +1650,6 @@ def run( *, host: str = DEFAULT_HTTP_HOST, port: int = DEFAULT_HTTP_PORT, - logger: Optional[logging.Logger] = None, ) -> RayServeHandle: """Run a Serve application and return a ServeHandle to the ingress. From ce284b2ffc313dab5c19c937be43c77bccfb6015 Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Mon, 14 Mar 2022 15:24:17 -0500 Subject: [PATCH 38/94] update applciation interface --- python/ray/serve/api.py | 76 +++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 49 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 2cb470c7a389..e6c63e353c63 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1510,6 +1510,18 @@ def get_deployment_statuses() -> Dict[str, DeploymentStatusInfo]: return internal_get_global_client().get_deployment_statuses() +class ImmutableDeploymentDict(dict): + def __init__(self, deployments: List[Deployment]): + raise NotImplementedError() + + def __setitem__(self, *args): + """Not allowed. Modify deployment options using set_options instead.""" + raise RuntimeError( + "Setting deployments in a built app is not allowed. Modify the " + 'options using app["deployment"].set_options instead.' + ) + + class Application: """A static, pre-built Serve application. @@ -1518,17 +1530,20 @@ class Application: meaning that it receives external traffic and is the entrypoint to the application. - The config options of each deployment can be modified by accessing the - deployment by name (e.g., app["my_deployment"].update(...)). + The ingress deployment can be accessed via app.ingress and a dictionary of + all deployments can be accessed via app.deployments. + + The config options of each deployment can be modified using set_options: + app.deployments["name"].set_options(...). This application object can be written to a config file and later deployed to production using the Serve CLI or REST API. """ - # TODO(edoakes): should this literally just be the REST pydantic schema? - # At the very least, it should be a simple dictionary. + def __init__(self, ingress: Deployment, deployments: List[Deployment]): + raise NotImplementedError() - def __init__(self): + def deployments(self) -> ImmutableDeploymentDict: raise NotImplementedError() def to_dict(self) -> Dict: @@ -1544,22 +1559,22 @@ def to_dict(self) -> Dict: @classmethod def from_dict(cls, d: Dict) -> "Application": - """Converts a dictionary of deployment data to an Application. + """Converts a dictionary of deployment data to an application. Takes in a dictionary matching the Serve REST API schema and converts - it to an Application containing those deployments. + it to an application containing those deployments. Args: d (Dict): A dictionary containing the deployments' data that matches the Serve REST API schema. Returns: - Application: a new Application object containing the deployments. + Application: a new application object containing the deployments. """ raise NotImplementedError() def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: - """Returns this Application's deployments as a YAML string. + """Returns this application's deployments as a YAML string. Optionally writes the YAML string to a file as well. To write to a file, use this pattern: @@ -1582,11 +1597,11 @@ def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: @classmethod def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": - """Converts YAML data to deployments for an Application. + """Converts YAML data to deployments for an application. Takes in a string or a file pointer to a file containing deployment definitions in YAML. These definitions are converted to a new - Application object containing the deployments. + application object containing the deployments. To read from a file, use the following pattern: @@ -1602,47 +1617,10 @@ def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": Serve YAML config files. Returns: - Application: a new Application object containing the deployments. - """ - raise NotImplementedError() - - def __getitem__(self, key: str): - """Fetch a deployment by name using dict syntax: app["name"] - - Raises: - KeyError: if the name is not in this Application. - """ - raise NotImplementedError() - - def __setitem__(self, key: str, value: Deployment): - """Set a deployment by name with dict syntax: app[name]=new_deployment - - Use this to overwrite existing deployments. - - Args: - key (String): name - value (Deployment): the deployment that the name maps to - - Raises: - TypeError: if the key is not a String or the value is not a deployment. - """ - raise NotImplementedError() - - def __iter__(self): - """Iterator over Application's deployments. - - Enables "for deployment in Application" pattern. + Application: a new application object containing the deployments. """ raise NotImplementedError() - def __len__(self): - """Number of deployments in this Application.""" - raise NotImplementedError() - - def __contains__(self, key: str): - """Checks if the key exists in self._deployments.""" - raise NotImplementedError() - @PublicAPI(stability="alpha") def run( From fb5b549685cad53ff98d1f3487d165960c5bbb1e Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Mon, 14 Mar 2022 13:25:40 -0700 Subject: [PATCH 39/94] Update tests --- python/ray/serve/tests/test_application.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/python/ray/serve/tests/test_application.py b/python/ray/serve/tests/test_application.py index a5f60f81c4fa..349ce03df4c3 100644 --- a/python/ray/serve/tests/test_application.py +++ b/python/ray/serve/tests/test_application.py @@ -70,7 +70,7 @@ def deploy_and_check_responses( the client to wait until the deployments finish deploying. """ - deploy_group(target=Application(deployments), blocking=blocking) + deploy_group(Application(deployments)._get_deployments(), blocking=blocking) def check_all_deployed(): try: @@ -143,7 +143,7 @@ async def request_echo(self, echo: str): MutualHandles.options(name=deployment_name, init_args=(handle_name,)) ) - deploy_group(target=Application(deployments), blocking=True) + deploy_group(Application(deployments)._get_deployments(), blocking=True) for deployment in deployments: assert (ray.get(deployment.get_handle().remote("hello"))) == "hello" @@ -308,7 +308,7 @@ def test_deploy_from_dict(self, serve_instance): compare_specified_options(config_dict, app_dict) - deploy_group(app) + deploy_group(app._get_deployments()) assert ( requests.get("http://localhost:8000/shallow").text == "Hello shallow world!" @@ -334,7 +334,7 @@ def test_deploy_from_yaml(self, serve_instance): compare_specified_options(app1.to_dict(), app2.to_dict()) # Check that deployment works - deploy_group(app1) + deploy_group(app1._get_deployments) assert ( requests.get("http://localhost:8000/shallow").text == "Hello shallow world!" ) From 65afc77de953d4e10953725dd293c87a7fc653b7 Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Mon, 14 Mar 2022 15:27:28 -0500 Subject: [PATCH 40/94] properties --- python/ray/serve/api.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index e6c63e353c63..a91f84a3e3ed 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1543,6 +1543,11 @@ class Application: def __init__(self, ingress: Deployment, deployments: List[Deployment]): raise NotImplementedError() + @property + def ingress(self) -> Deployment: + raise NotImplementedError() + + @property def deployments(self) -> ImmutableDeploymentDict: raise NotImplementedError() From c58dc4f97b74635c6ae711d498059ad9b6c9c762 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Mon, 14 Mar 2022 13:27:46 -0700 Subject: [PATCH 41/94] Include parentheses --- python/ray/serve/tests/test_application.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ray/serve/tests/test_application.py b/python/ray/serve/tests/test_application.py index 349ce03df4c3..4cd677cd4272 100644 --- a/python/ray/serve/tests/test_application.py +++ b/python/ray/serve/tests/test_application.py @@ -334,7 +334,7 @@ def test_deploy_from_yaml(self, serve_instance): compare_specified_options(app1.to_dict(), app2.to_dict()) # Check that deployment works - deploy_group(app1._get_deployments) + deploy_group(app1._get_deployments()) assert ( requests.get("http://localhost:8000/shallow").text == "Hello shallow world!" ) From 9d03faf1f89d649cba4a23dc60f07a08a1599455 Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Mon, 14 Mar 2022 15:28:37 -0500 Subject: [PATCH 42/94] fix err --- python/ray/serve/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index a91f84a3e3ed..36d5aad2e892 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1518,7 +1518,7 @@ def __setitem__(self, *args): """Not allowed. Modify deployment options using set_options instead.""" raise RuntimeError( "Setting deployments in a built app is not allowed. Modify the " - 'options using app["deployment"].set_options instead.' + 'options using app.deployments["deployment"].set_options instead.' ) From a71e17463f3082552617f80204124a99214f731a Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Mon, 14 Mar 2022 15:55:21 -0500 Subject: [PATCH 43/94] fix --- python/ray/serve/scripts.py | 41 ++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 01ee49a25737..0b5b50cbb67c 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -1,14 +1,16 @@ #!/usr/bin/env python +import click import json -import yaml import os import pathlib -import click +import time +import yaml import ray from ray._private.utils import import_attr from ray.serve.config import DeploymentMode from ray import serve +from ray.serve import get_deployment_statuses from ray.serve.constants import ( DEFAULT_CHECKPOINT_PATH, DEFAULT_HTTP_HOST, @@ -150,10 +152,6 @@ def shutdown(address: str, namespace: str): type=str, help=RAY_DASHBOARD_ADDRESS_HELP_STR, ) -@click.option( - "--blocking/--non-blocking", - default=False, -) def deploy(config_file_name: str, address: str): with open(config_file_name, "r") as config_file: config = yaml.safe_load(config_file) @@ -172,12 +170,11 @@ def deploy(config_file_name: str, address: str): @cli.command( - short_help="Run a Serve app in a blocking way.", + short_help="Run a Serve app.", help=( - "Runs the Serve app from the specified YAML config file or import path " - "to a bound deployment node or built application object.\n" - "Blocks after deploying and logs status periodically. If you Ctrl-C " - "this command, it tears down the app." + "Runs the Serve app from the specified import path or YAML config.\n" + "By default, this will block and periodically log status. If you " + "Ctrl-C the command, it will tear down the app." ), ) @click.argument("config_or_import_path") @@ -187,15 +184,15 @@ def deploy(config_file_name: str, address: str): default=None, required=False, help="Path to a local YAML file containing a runtime_env definition. " - "Overrides runtime_envs specified in the config file.", + "This will be passed to ray.init() as the default for deployments.", ) @click.option( "--runtime-env-json", type=str, default=None, required=False, - help="JSON-serialized runtime_env dictionary. Overrides runtime_envs " - "specified in the config file.", + help="JSON-serialized runtime_env dictionary. This will be passed to " + "ray.init() as the default for deployments.", ) @click.option( "--working-dir", @@ -206,7 +203,8 @@ def deploy(config_file_name: str, address: str): "Directory containing files that your job will run in. Can be a " "local directory or a remote URI to a .zip file (S3, GS, HTTP). " "This overrides the working_dir in --runtime-env if both are " - "specified. Overrides working_dirs specified in the config file." + "specified. This will be passed to ray.init() as the default for " + "deployments." ), ) @click.option( @@ -272,7 +270,18 @@ def run( # Setting the runtime_env here will set defaults for the deployments. ray.init(address=address, namespace="serve", runtime_env=final_runtime_env) - serve.run(app_or_node, host=host, port=port, logger=cli_logger) + + try: + serve.run(app_or_node, host=host, port=port) + cli_logger.success("Application deployed successfully.") + if blocking: + cli_logger.print("Watching deployment statuses:\n") + while True: + print(get_deployment_statuses()) + time.sleep(10) + except KeyboardInterrupt: + cli_logger.info("Got KeyboardInterrupt), shutting down...") + serve.shutdown() @cli.command( From 0588458a1a0bb4388496c3635f0c7de380776a11 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Mon, 14 Mar 2022 14:07:03 -0700 Subject: [PATCH 44/94] Use new api --- dashboard/modules/serve/serve_head.py | 5 +- dashboard/modules/serve/tests/test_schema.py | 597 ------------------- python/ray/serve/api.py | 2 +- python/ray/serve/scripts.py | 2 +- 4 files changed, 5 insertions(+), 601 deletions(-) delete mode 100644 dashboard/modules/serve/tests/test_schema.py diff --git a/dashboard/modules/serve/serve_head.py b/dashboard/modules/serve/serve_head.py index fb339aaaa3a5..10c5f8644007 100644 --- a/dashboard/modules/serve/serve_head.py +++ b/dashboard/modules/serve/serve_head.py @@ -5,8 +5,9 @@ import ray.dashboard.optional_utils as optional_utils from ray import serve -from ray.serve.api import Application from ray.serve.api import ( + Application, + deploy_group, get_deployment_statuses, serve_application_to_schema, serve_application_status_to_schema, @@ -53,7 +54,7 @@ async def delete_serve_application(self, req: Request) -> Response: @optional_utils.init_ray_and_catch_exceptions(connect_to_serve=True) async def put_all_deployments(self, req: Request) -> Response: app = Application.from_dict(await req.json()) - app.deploy(blocking=False) + deploy_group(app._get_deployments(), blocking=False) new_names = set() for deployment in app: diff --git a/dashboard/modules/serve/tests/test_schema.py b/dashboard/modules/serve/tests/test_schema.py deleted file mode 100644 index 0271509d9198..000000000000 --- a/dashboard/modules/serve/tests/test_schema.py +++ /dev/null @@ -1,597 +0,0 @@ -import sys - -from pydantic import ValidationError - -import pytest - -import requests - -import ray -from ray.serve.schema import ( - RayActorOptionsSchema, - DeploymentSchema, - DeploymentStatusSchema, - ServeApplicationSchema, - ServeApplicationStatusSchema, -) -from ray.util.accelerators.accelerators import NVIDIA_TESLA_V100, NVIDIA_TESLA_P4 -from ray.serve.config import AutoscalingConfig -from ray.serve.common import DeploymentStatus, DeploymentStatusInfo -from ray.serve.api import ( - get_deployment_statuses, - deployment_to_schema, - schema_to_deployment, - serve_application_to_schema, - schema_to_serve_application, - status_info_to_schema, - serve_application_status_to_schema, -) -from ray import serve - - -class TestRayActorOptionsSchema: - def test_valid_ray_actor_options_schema(self): - # Ensure a valid RayActorOptionsSchema can be generated - - ray_actor_options_schema = { - "runtime_env": { - "working_dir": ( - "https://github.com/shrekris-anyscale/" - "test_module/archive/HEAD.zip" - ) - }, - "num_cpus": 0.2, - "num_gpus": 50, - "memory": 3, - "object_store_memory": 64, - "resources": {"custom_asic": 12}, - "accelerator_type": NVIDIA_TESLA_V100, - } - - RayActorOptionsSchema.parse_obj(ray_actor_options_schema) - - def test_ge_zero_ray_actor_options_schema(self): - # Ensure ValidationError is raised when any fields that must be greater - # than zero is set to zero. - - ge_zero_fields = ["num_cpus", "num_gpus", "memory", "object_store_memory"] - for field in ge_zero_fields: - with pytest.raises(ValidationError): - RayActorOptionsSchema.parse_obj({field: -1}) - - def test_runtime_env(self): - # Test different runtime_env configurations - - ray_actor_options_schema = { - "runtime_env": None, - "num_cpus": 0.2, - "num_gpus": 50, - "memory": 3, - "object_store_memory": 64, - "resources": {"custom_asic": 12}, - "accelerator_type": NVIDIA_TESLA_V100, - } - - # ray_actor_options_schema should work as is - RayActorOptionsSchema.parse_obj(ray_actor_options_schema) - - # working_dir and py_modules cannot contain local uris - ray_actor_options_schema["runtime_env"] = { - "working_dir": ".", - "py_modules": [ - "/Desktop/my_project", - ( - "https://github.com/shrekris-anyscale/" - "test_deploy_group/archive/HEAD.zip" - ), - ], - } - - with pytest.raises(ValueError): - RayActorOptionsSchema.parse_obj(ray_actor_options_schema) - - # remote uris should work - ray_actor_options_schema["runtime_env"] = { - "working_dir": ( - "https://github.com/shrekris-anyscale/test_module/archive/HEAD.zip" - ), - "py_modules": [ - ( - "https://github.com/shrekris-anyscale/" - "test_deploy_group/archive/HEAD.zip" - ), - ], - } - - RayActorOptionsSchema.parse_obj(ray_actor_options_schema) - - def test_extra_fields_invalid_ray_actor_options(self): - # Undefined fields should be forbidden in the schema - - ray_actor_options_schema = { - "runtime_env": None, - "num_cpus": None, - "num_gpus": None, - "memory": None, - "object_store_memory": None, - "resources": None, - "accelerator_type": None, - } - - # Schema should be createable with valid fields - RayActorOptionsSchema.parse_obj(ray_actor_options_schema) - - # Schema should raise error when a nonspecified field is included - ray_actor_options_schema["fake_field"] = None - with pytest.raises(ValidationError): - RayActorOptionsSchema.parse_obj(ray_actor_options_schema) - - -class TestDeploymentSchema: - def get_minimal_deployment_schema(self): - # Generate a DeploymentSchema with the fewest possible attributes set - - return { - "name": "deep", - "init_args": None, - "init_kwargs": None, - "import_path": "my_module.MyClass", - "num_replicas": None, - "route_prefix": None, - "max_concurrent_queries": None, - "user_config": None, - "autoscaling_config": None, - "graceful_shutdown_wait_loop_s": None, - "graceful_shutdown_timeout_s": None, - "health_check_period_s": None, - "health_check_timeout_s": None, - "ray_actor_options": { - "runtime_env": None, - "num_cpus": None, - "num_gpus": None, - "memory": None, - "object_store_memory": None, - "resources": None, - "accelerator_type": None, - }, - } - - def test_valid_deployment_schema(self): - # Ensure a valid DeploymentSchema can be generated - - deployment_schema = { - "name": "shallow", - "init_args": [4, "glue"], - "init_kwargs": {"fuel": "diesel"}, - "import_path": "test_env.shallow_import.ShallowClass", - "num_replicas": 2, - "route_prefix": "/shallow", - "max_concurrent_queries": 32, - "user_config": {"threshold": 0.2, "pattern": "rainbow"}, - "autoscaling_config": None, - "graceful_shutdown_wait_loop_s": 17, - "graceful_shutdown_timeout_s": 49, - "health_check_period_s": 11, - "health_check_timeout_s": 11, - "ray_actor_options": { - "runtime_env": { - "working_dir": ( - "https://github.com/shrekris-anyscale/" - "test_module/archive/HEAD.zip" - ), - "py_modules": [ - ( - "https://github.com/shrekris-anyscale/" - "test_deploy_group/archive/HEAD.zip" - ), - ], - }, - "num_cpus": 3, - "num_gpus": 4.2, - "memory": 5, - "object_store_memory": 3, - "resources": {"custom_asic": 8}, - "accelerator_type": NVIDIA_TESLA_P4, - }, - } - - DeploymentSchema.parse_obj(deployment_schema) - - def test_invalid_python_attributes(self): - # Test setting invalid attributes for Python to ensure a validation or - # value error is raised. - - # Python requires an import path - deployment_schema = self.get_minimal_deployment_schema() - deployment_schema["init_args"] = [1, 2] - deployment_schema["init_kwargs"] = {"threshold": 0.5} - del deployment_schema["import_path"] - - with pytest.raises(ValueError, match="must be specified"): - DeploymentSchema.parse_obj(deployment_schema) - - # DeploymentSchema should be generated once import_path is set - deployment_schema["import_path"] = "my_module.MyClass" - DeploymentSchema.parse_obj(deployment_schema) - - # Invalid import_path syntax should raise a ValidationError - invalid_paths = ["", "MyClass", ".", "hello,world"] - for path in invalid_paths: - deployment_schema["import_path"] = path - with pytest.raises(ValidationError): - DeploymentSchema.parse_obj(deployment_schema) - - def test_gt_zero_deployment_schema(self): - # Ensure ValidationError is raised when any fields that must be greater - # than zero is set to zero. - - deployment_schema = self.get_minimal_deployment_schema() - - gt_zero_fields = [ - "num_replicas", - "max_concurrent_queries", - "health_check_period_s", - "health_check_timeout_s", - ] - for field in gt_zero_fields: - deployment_schema[field] = 0 - with pytest.raises(ValidationError): - DeploymentSchema.parse_obj(deployment_schema) - deployment_schema[field] = None - - def test_ge_zero_deployment_schema(self): - # Ensure ValidationError is raised when any fields that must be greater - # than or equal to zero is set to -1. - - deployment_schema = self.get_minimal_deployment_schema() - - ge_zero_fields = [ - "graceful_shutdown_wait_loop_s", - "graceful_shutdown_timeout_s", - ] - - for field in ge_zero_fields: - deployment_schema[field] = -1 - with pytest.raises(ValidationError): - DeploymentSchema.parse_obj(deployment_schema) - deployment_schema[field] = None - - def test_route_prefix(self): - # Ensure that route_prefix is validated - - deployment_schema = self.get_minimal_deployment_schema() - - # route_prefix must start with a "/" - deployment_schema["route_prefix"] = "hello/world" - with pytest.raises(ValueError): - DeploymentSchema.parse_obj(deployment_schema) - - # route_prefix must end with a "/" - deployment_schema["route_prefix"] = "/hello/world/" - with pytest.raises(ValueError): - DeploymentSchema.parse_obj(deployment_schema) - - # route_prefix cannot contain wildcards, meaning it can't have - # "{" or "}" - deployment_schema["route_prefix"] = "/hello/{adjective}/world/" - with pytest.raises(ValueError): - DeploymentSchema.parse_obj(deployment_schema) - - # Ensure a valid route_prefix works - deployment_schema["route_prefix"] = "/hello/wonderful/world" - DeploymentSchema.parse_obj(deployment_schema) - - # Ensure route_prefix of "/" works - deployment_schema["route_prefix"] = "/" - DeploymentSchema.parse_obj(deployment_schema) - - # Ensure route_prefix of None works - deployment_schema["route_prefix"] = None - DeploymentSchema.parse_obj(deployment_schema) - - def test_mutually_exclusive_num_replicas_and_autoscaling_config(self): - # num_replicas and autoscaling_config cannot be set at the same time - deployment_schema = self.get_minimal_deployment_schema() - - deployment_schema["num_replicas"] = 5 - deployment_schema["autoscaling_config"] = None - DeploymentSchema.parse_obj(deployment_schema) - - deployment_schema["num_replicas"] = None - deployment_schema["autoscaling_config"] = AutoscalingConfig().dict() - DeploymentSchema.parse_obj(deployment_schema) - - deployment_schema["num_replicas"] = 5 - deployment_schema["autoscaling_config"] = AutoscalingConfig().dict() - with pytest.raises(ValueError): - DeploymentSchema.parse_obj(deployment_schema) - - def test_extra_fields_invalid_deployment_schema(self): - # Undefined fields should be forbidden in the schema - - deployment_schema = self.get_minimal_deployment_schema() - - # Schema should be createable with valid fields - DeploymentSchema.parse_obj(deployment_schema) - - # Schema should raise error when a nonspecified field is included - deployment_schema["fake_field"] = None - with pytest.raises(ValidationError): - DeploymentSchema.parse_obj(deployment_schema) - - -class TestServeApplicationSchema: - def get_valid_serve_application_schema(self): - return { - "deployments": [ - { - "name": "shallow", - "init_args": [4, "glue"], - "init_kwargs": {"fuel": "diesel"}, - "import_path": "test_env.shallow_import.ShallowClass", - "num_replicas": 2, - "route_prefix": "/shallow", - "max_concurrent_queries": 32, - "user_config": None, - "autoscaling_config": None, - "graceful_shutdown_wait_loop_s": 17, - "graceful_shutdown_timeout_s": 49, - "health_check_period_s": 11, - "health_check_timeout_s": 11, - "ray_actor_options": { - "runtime_env": { - "working_dir": ( - "https://github.com/shrekris-anyscale/" - "test_module/archive/HEAD.zip" - ), - "py_modules": [ - ( - "https://github.com/shrekris-anyscale/" - "test_deploy_group/archive/HEAD.zip" - ), - ], - }, - "num_cpus": 3, - "num_gpus": 4.2, - "memory": 5, - "object_store_memory": 3, - "resources": {"custom_asic": 8}, - "accelerator_type": NVIDIA_TESLA_P4, - }, - }, - { - "name": "deep", - "init_args": None, - "init_kwargs": None, - "import_path": ("test_env.subdir1.subdir2.deep_import.DeepClass"), - "num_replicas": None, - "route_prefix": None, - "max_concurrent_queries": None, - "user_config": None, - "autoscaling_config": None, - "graceful_shutdown_wait_loop_s": None, - "graceful_shutdown_timeout_s": None, - "health_check_period_s": None, - "health_check_timeout_s": None, - "ray_actor_options": { - "runtime_env": None, - "num_cpus": None, - "num_gpus": None, - "memory": None, - "object_store_memory": None, - "resources": None, - "accelerator_type": None, - }, - }, - ] - } - - def test_valid_serve_application_schema(self): - # Ensure a valid ServeApplicationSchema can be generated - - serve_application_schema = self.get_valid_serve_application_schema() - ServeApplicationSchema.parse_obj(serve_application_schema) - - def test_extra_fields_invalid_serve_application_schema(self): - # Undefined fields should be forbidden in the schema - - serve_application_schema = self.get_valid_serve_application_schema() - - # Schema should be createable with valid fields - ServeApplicationSchema.parse_obj(serve_application_schema) - - # Schema should raise error when a nonspecified field is included - serve_application_schema["fake_field"] = None - with pytest.raises(ValidationError): - ServeApplicationSchema.parse_obj(serve_application_schema) - - -class TestDeploymentStatusSchema: - def get_valid_deployment_status_schema(self): - return { - "deployment_1": DeploymentStatusInfo(DeploymentStatus.HEALTHY), - "deployment_2": DeploymentStatusInfo( - DeploymentStatus.UNHEALTHY, "This is an unhealthy deployment." - ), - "deployment_3": DeploymentStatusInfo(DeploymentStatus.UPDATING), - } - - def test_valid_deployment_status_schema(self): - # Ensure valid DeploymentStatusSchemas can be generated - - deployment_status_schemas = self.get_valid_deployment_status_schema() - - for name, status_info in deployment_status_schemas.items(): - status_info_to_schema(name, status_info) - - def test_invalid_status(self): - # Ensure a DeploymentStatusSchema cannot be initialized with an invalid status - - status_info = { - "status": "nonexistent status", - "message": "welcome to nonexistence", - } - with pytest.raises(ValidationError): - status_info_to_schema("deployment name", status_info) - - def test_extra_fields_invalid_deployment_status_schema(self): - # Undefined fields should be forbidden in the schema - - deployment_status_schemas = self.get_valid_deployment_status_schema() - - # Schema should be createable with valid fields - for name, status_info in deployment_status_schemas.items(): - DeploymentStatusSchema( - name=name, status=status_info.status, message=status_info.message - ) - - # Schema should raise error when a nonspecified field is included - for name, status_info in deployment_status_schemas.items(): - with pytest.raises(ValidationError): - DeploymentStatusSchema( - name=name, - status=status_info.status, - message=status_info.message, - fake_field=None, - ) - - -class TestServeApplicationStatusSchema: - def get_valid_serve_application_status_schema(self): - return { - "deployment_1": {"status": "HEALTHY", "message": ""}, - "deployment_2": { - "status": "UNHEALTHY", - "message": "this deployment is deeply unhealthy", - }, - } - - def test_valid_serve_application_status_schema(self): - # Ensure a valid ServeApplicationStatusSchema can be generated - - serve_application_status_schema = ( - self.get_valid_serve_application_status_schema() - ) - serve_application_status_to_schema(serve_application_status_schema) - - def test_extra_fields_invalid_serve_application_status_schema(self): - # Undefined fields should be forbidden in the schema - - serve_application_status_schema = ( - self.get_valid_serve_application_status_schema() - ) - - # Schema should be createable with valid fields - serve_application_status_to_schema(serve_application_status_schema) - - # Schema should raise error when a nonspecified field is included - with pytest.raises(ValidationError): - statuses = [ - status_info_to_schema(name, status_info) - for name, status_info in serve_application_status_schema.items() - ] - ServeApplicationStatusSchema(statuses=statuses, fake_field=None) - - -# This function is defined globally to be accessible via import path -def global_f(): - return "Hello world!" - - -def test_deployment_to_schema_to_deployment(): - @serve.deployment( - num_replicas=3, - route_prefix="/hello", - ray_actor_options={ - "runtime_env": { - "working_dir": ( - "https://github.com/shrekris-anyscale/" - "test_module/archive/HEAD.zip" - ), - "py_modules": [ - ( - "https://github.com/shrekris-anyscale/" - "test_deploy_group/archive/HEAD.zip" - ), - ], - } - }, - ) - def f(): - # The body of this function doesn't matter. It gets replaced by - # global_f() when the import path in f._func_or_class is overwritten. - # This function is used as a convenience to apply the @serve.deployment - # decorator without converting global_f() into a Deployment object. - pass - - f._func_or_class = "ray.serve.tests.test_schema.global_f" - - deployment = schema_to_deployment(deployment_to_schema(f)) - - assert deployment.num_replicas == 3 - assert deployment.route_prefix == "/hello" - assert deployment.ray_actor_options["runtime_env"]["working_dir"] == ( - "https://github.com/shrekris-anyscale/test_module/archive/HEAD.zip" - ) - assert deployment.ray_actor_options["runtime_env"]["py_modules"] == [ - "https://github.com/shrekris-anyscale/test_deploy_group/archive/HEAD.zip", - "https://github.com/shrekris-anyscale/test_module/archive/HEAD.zip", - ] - - serve.start() - deployment.deploy() - assert ray.get(deployment.get_handle().remote()) == "Hello world!" - assert requests.get("http://localhost:8000/hello").text == "Hello world!" - serve.shutdown() - - -def test_serve_application_to_schema_to_serve_application(): - @serve.deployment( - num_replicas=1, - route_prefix="/hello", - ) - def f1(): - # The body of this function doesn't matter. See the comment in - # test_deployment_to_schema_to_deployment. - pass - - @serve.deployment( - num_replicas=2, - route_prefix="/hi", - ) - def f2(): - pass - - f1._func_or_class = "ray.serve.tests.test_schema.global_f" - f2._func_or_class = "ray.serve.tests.test_schema.global_f" - - deployments = schema_to_serve_application(serve_application_to_schema([f1, f2])) - - assert deployments[0].num_replicas == 1 - assert deployments[0].route_prefix == "/hello" - assert deployments[1].num_replicas == 2 - assert deployments[1].route_prefix == "/hi" - - serve.start() - - deployments[0].deploy() - deployments[1].deploy() - assert ray.get(deployments[0].get_handle().remote()) == "Hello world!" - assert requests.get("http://localhost:8000/hello").text == "Hello world!" - assert ray.get(deployments[1].get_handle().remote()) == "Hello world!" - assert requests.get("http://localhost:8000/hi").text == "Hello world!" - - # Check statuses - statuses = serve_application_status_to_schema(get_deployment_statuses()).statuses - deployment_names = {"f1", "f2"} - for deployment_status in statuses: - assert deployment_status.status in {"UPDATING", "HEALTHY"} - assert deployment_status.name in deployment_names - deployment_names.remove(deployment_status.name) - assert len(deployment_names) == 0 - - serve.shutdown() - - -if __name__ == "__main__": - sys.exit(pytest.main(["-v", __file__])) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index a231c55bfdff..b870598a8c3d 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1734,7 +1734,7 @@ def run( except KeyboardInterrupt: logger.info("Got SIGINT (KeyboardInterrupt). Removing deployments.") - for deployment in deployments.values(): + for deployment in deployments: deployment.delete() if len(list_deployments()) == 0: logger.info("No deployments left. Shutting down Serve.") diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index c1e35a369193..d9a3bddcd14c 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -356,7 +356,7 @@ def run( for deployment in app: _configure_runtime_env(deployment, runtime_env_updates) - app.run(logger=cli_logger) + serve.run(app, logger=cli_logger) @cli.command( From fc54ed8f2c0eff87b154404098fc21f2cb83220a Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Mon, 14 Mar 2022 16:09:32 -0500 Subject: [PATCH 45/94] fix nits --- python/ray/serve/api.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 36d5aad2e892..e584542118b0 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -50,7 +50,7 @@ ) from ray.serve.controller import ServeController from ray.serve.exceptions import RayServeException -from ray.experimental.dag import ClassNode +from ray.experimental.dag import DAGNode from ray.serve.handle import RayServeHandle, RayServeSyncHandle from ray.serve.http_util import ASGIHTTPSender, make_fastapi_class_based_view from ray.serve.utils import ( @@ -911,7 +911,7 @@ def remote(self, *args, **kwargs) -> ray.ObjectRef: @PublicAPI(stability="alpha") -class DeploymentMethodNode(ClassNode): +class DeploymentMethodNode(DAGNode): """Represents a method call on a bound deployment node. These method calls can be composed into an optimized call DAG and passed @@ -926,7 +926,7 @@ def __init__(self): @PublicAPI(stability="alpha") -class DeploymentNode(ClassNode): +class DeploymentNode(DAGNode): """Represents a deployment with its bound config options and arguments. The bound deployment can be run, deployed, or built to a production config From 96b8fb3cdf25d37b21281bcd95f45026372bc09b Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Mon, 14 Mar 2022 16:12:06 -0500 Subject: [PATCH 46/94] fix bad import --- python/ray/serve/drivers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/ray/serve/drivers.py b/python/ray/serve/drivers.py index 1fa244b77e1d..c8c8ff1abf2e 100644 --- a/python/ray/serve/drivers.py +++ b/python/ray/serve/drivers.py @@ -3,19 +3,19 @@ import starlette from ray import serve -from ray.serve.api import DeployedCallGraph +from ray.serve.api import DAGHandle @serve.deployment class PipelineDriverDeployment: def __init__( self, - call_graph: DeployedCallGraph, + dag_handle: DAGHandle, *, input_schema: Optional[Union[str, Callable]] = None, ): raise NotImplementedError() async def __call__(self, request: starlette.requests.Request): - """Parse input schema and pass the result to the call graph.""" + """Parse input schema and pass the result to the DAG handle.""" raise NotImplementedError() From 022068b1ad3db6d93704d514885532112b5869a2 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Mon, 14 Mar 2022 15:26:40 -0700 Subject: [PATCH 47/94] Move blocking behavior to CLI --- python/ray/serve/api.py | 32 +++++--------------------------- python/ray/serve/scripts.py | 30 +++++++++++++++++++++++++++--- 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 551affbbbb21..daf87919ec5f 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -7,7 +7,6 @@ import re import time import yaml -import sys from dataclasses import dataclass from functools import wraps from typing import ( @@ -76,8 +75,6 @@ ServeApplicationStatusSchema, ) -# from ray.serve.pipeline.api import extract_deployments_from_serve_dag - _INTERNAL_REPLICA_CONTEXT = None _global_client = None @@ -1702,33 +1699,14 @@ def run( will be deployed. """ - try: - deployments = _get_deployments_from_target(target) - - # TODO (shrekris-anyscale): validate ingress + deployments = _get_deployments_from_target(target) - start(detached=True, http_options={"host": host, "port": port}) - deploy_group(deployments, blocking=True) + # TODO (shrekris-anyscale): validate ingress - # logger.info("\nDeployed successfully!\n") + start(detached=True, http_options={"host": host, "port": port}) + deploy_group(deployments, blocking=True) - while True: - statuses = serve_application_status_to_schema( - get_deployment_statuses() - ).json(indent=4) - logger.info(f"{statuses}") - time.sleep(10) - - # TODO (shrekris-anyscale): return handle to ingress deployment - - except KeyboardInterrupt: - # logger.info("Got SIGINT (KeyboardInterrupt). Removing deployments.") - for deployment in deployments: - deployment.delete() - if len(list_deployments()) == 0: - # logger.info("No deployments left. Shutting down Serve.") - shutdown() - sys.exit() + # TODO (shrekris-anyscale): return handle to ingress deployment def deploy_group(deployments: List[Deployment], *, blocking=True) -> RayServeHandle: diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 32a9c90c06ab..36b6030d04e0 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -4,6 +4,8 @@ import os import pathlib import click +import time +import sys from typing import Tuple, List, Dict import argparse @@ -19,8 +21,12 @@ from ray.dashboard.modules.dashboard_sdk import parse_runtime_env_args from ray.dashboard.modules.serve.sdk import ServeSubmissionClient from ray.autoscaler._private.cli_logger import cli_logger -from ray.serve.api import Application -from ray.serve.api import Deployment +from ray.serve.api import ( + Application, + Deployment, + get_deployment_statuses, + serve_application_status_to_schema, +) RAY_INIT_ADDRESS_HELP_STR = ( "Address to use for ray.init(). Can also be specified " @@ -356,7 +362,25 @@ def run( for deployment in app.deployments: _configure_runtime_env(deployment, runtime_env_updates) - serve.run(app) + try: + serve.run(app) + cli_logger.success("Deployed successfully!\n") + + while True: + statuses = serve_application_status_to_schema( + get_deployment_statuses() + ).json(indent=4) + cli_logger.info(f"{statuses}") + time.sleep(10) + + except KeyboardInterrupt: + cli_logger.info("Got SIGINT (KeyboardInterrupt). Removing deployments.") + for deployment in app.deployments: + deployment.delete() + if len(serve.list_deployments()) == 0: + cli_logger.info("No deployments left. Shutting down Serve.") + serve.shutdown() + sys.exit() @cli.command( From 64a89993f44a2cc5a6c02d4643dc4695044533a7 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Mon, 14 Mar 2022 15:38:28 -0700 Subject: [PATCH 48/94] Remove application.py --- python/ray/serve/application.py | 279 -------------------------------- 1 file changed, 279 deletions(-) delete mode 100644 python/ray/serve/application.py diff --git a/python/ray/serve/application.py b/python/ray/serve/application.py deleted file mode 100644 index 6662f1baecb5..000000000000 --- a/python/ray/serve/application.py +++ /dev/null @@ -1,279 +0,0 @@ -from typing import List, TextIO, Optional, Union, Dict -import yaml -import time -import sys - -from ray import serve -from ray.serve.api import ( - Deployment, - internal_get_global_client, -) -from ray.serve.schema import ServeApplicationSchema -from ray.serve.utils import logger -from ray.serve.api import ( - get_deployment_statuses, - serve_application_to_schema, - schema_to_serve_application, - serve_application_status_to_schema, -) -from ray.autoscaler._private.cli_logger import _CliLogger -from logging import Logger - - -class Application: - """Used to deploy, update, and monitor groups of Serve deployments. - - Args: - deployments (List[Deployment]): The Serve Application is - initialized to contain this group of deployments. - """ - - def __init__(self, deployments: List[Deployment] = None): - deployments = deployments or [] - - self._deployments = dict() - for d in deployments: - self.add_deployment(d) - - def add_deployment(self, deployment: Deployment): - """Adds the deployment to this Serve application. - - Validates that a deployment with the same name doesn't already exist. - To overwrite an existing deployment, use index notation. For example: - - app[deployment_name] = deployment - - Args: - deployment (Deployment): deployment to add to this Application. - - Raises: - ValueError: If a deployment with deployment.name already exists in - this Application. - """ - - if not isinstance(deployment, Deployment): - raise TypeError(f"Got {type(deployment)}. Expected deployment.") - elif deployment.name in self._deployments: - raise ValueError( - f'Deployment with name "{deployment.name}" already ' - "exists in this application. To overwrite this " - "deployment, use attribute or index notation. " - "For example:\n\napp.deployment_name = " - "new_deployment" - ) - - self._deployments[deployment.name] = deployment - - def deploy(self, blocking: bool = True): - """Atomically deploys the Application's deployments to the Ray cluster. - - The Application's deployments can carry handles to one another. - - Args: - blocking (bool): If True, this function only returns after the - deployment is finished. If False, this function returns - immediately after requesting the deployment. - """ - - if len(self._deployments) == 0: - return - - parameter_group = [] - - for deployment in self._deployments.values(): - if not isinstance(deployment, Deployment): - raise TypeError( - f"deploy_group only accepts Deployments, but got unexpected " - f"type {type(deployment)}." - ) - - deployment_parameters = { - "name": deployment._name, - "func_or_class": deployment._func_or_class, - "init_args": deployment.init_args, - "init_kwargs": deployment.init_kwargs, - "ray_actor_options": deployment._ray_actor_options, - "config": deployment._config, - "version": deployment._version, - "prev_version": deployment._prev_version, - "route_prefix": deployment.route_prefix, - "url": deployment.url, - } - - parameter_group.append(deployment_parameters) - - return internal_get_global_client().deploy_group( - parameter_group, _blocking=blocking - ) - - def run(self, logger: Union[Logger, _CliLogger] = logger): - """Deploys all deployments in this Application and logs status. - - This function keeps looping and printing status, so it must be manually - killed (e.g. using ctrl-C). When it recieves a kill signal, it tears - down all deployments that it deployed. If there are no deployments - left, it also shuts down the rest of Serve, including the controller. - This is meant to help interactive development. - - Args: - logger: Any Python object that implements the standard logger - interface. - """ - - try: - serve.start(detached=True) - self.deploy() - - logger.info("\nDeployed successfully!\n") - - while True: - statuses = serve_application_status_to_schema( - get_deployment_statuses() - ).json(indent=4) - logger.info(f"{statuses}") - time.sleep(10) - - except KeyboardInterrupt: - logger.info("Got SIGINT (KeyboardInterrupt). Removing deployments.") - for deployment in self._deployments.values(): - deployment.delete() - if len(serve.list_deployments()) == 0: - logger.info("No deployments left. Shutting down Serve.") - serve.shutdown() - sys.exit() - - def to_dict(self) -> Dict: - """Returns this Application's deployments as a dictionary. - - This dictionary adheres to the Serve REST API schema. It can be deployed - via the Serve REST API. - - Returns: - Dict: The Application's deployments formatted in a dictionary. - """ - - return serve_application_to_schema(self._deployments.values()).dict() - - @classmethod - def from_dict(cls, d: Dict) -> "Application": - """Converts a dictionary of deployment data to an Application. - - Takes in a dictionary matching the Serve REST API schema and converts - it to an Application containing those deployments. - - Args: - d (Dict): A dictionary containing the deployments' data that matches - the Serve REST API schema. - - Returns: - Application: a new Application object containing the deployments. - """ - - schema = ServeApplicationSchema.parse_obj(d) - return cls(schema_to_serve_application(schema)) - - def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: - """Returns this Application's deployments as a YAML string. - - Optionally writes the YAML string to a file as well. To write to a - file, use this pattern: - - with open("file_name.txt", "w") as f: - app.to_yaml(f=f) - - This file is formatted as a Serve YAML config file. It can be deployed - via the Serve CLI. - - Args: - f (Optional[TextIO]): A pointer to the file where the YAML should - be written. - - Returns: - Optional[String]: The deployments' YAML string. The output is from - yaml.safe_dump(). Returned only if no file pointer is passed in. - """ - - deployment_dict = serve_application_to_schema(self._deployments.values()).dict() - - if f: - yaml.safe_dump(deployment_dict, f, default_flow_style=False) - return yaml.safe_dump(deployment_dict, default_flow_style=False) - - @classmethod - def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": - """Converts YAML data to deployments for an Application. - - Takes in a string or a file pointer to a file containing deployment - definitions in YAML. These definitions are converted to a new - Application object containing the deployments. - - To read from a file, use the following pattern: - - with open("file_name.txt", "w") as f: - app = app.from_yaml(str_or_file) - - Args: - str_or_file (Union[String, TextIO]): Either a string containing - YAML deployment definitions or a pointer to a file containing - YAML deployment definitions. The YAML format must adhere to the - ServeApplicationSchema JSON Schema defined in - ray.serve.schema. This function works with - Serve YAML config files. - - Returns: - Application: a new Application object containing the deployments. - """ - - deployments_json = yaml.safe_load(str_or_file) - schema = ServeApplicationSchema.parse_obj(deployments_json) - return cls(schema_to_serve_application(schema)) - - def __getitem__(self, key: str): - """Fetch a deployment by name using dict syntax: app["name"] - - Raises: - KeyError: if the name is not in this Application. - """ - - if key in self._deployments: - return self._deployments[key] - else: - raise KeyError(f'Serve application does not contain a "{key}" deployment.') - - def __setitem__(self, key: str, value: Deployment): - """Set a deployment by name with dict syntax: app[name]=new_deployment - - Use this to overwrite existing deployments. - - Args: - key (String): name - value (Deployment): the deployment that the name maps to - - Raises: - TypeError: if the key is not a String or the value is not a deployment. - """ - - if not isinstance(key, str): - raise TypeError(f"key should be a string, but got object of type {key}.") - elif not isinstance(value, Deployment): - raise TypeError(f"Got {type(Deployment)} for value. Expected deployment.") - - self._deployments[key] = value - - def __iter__(self): - """Iterator over Application's deployments. - - Enables "for deployment in Application" pattern. - """ - - return iter(self._deployments.values()) - - def __len__(self): - """Number of deployments in this Application.""" - - return len(self._deployments) - - def __contains__(self, key: str): - """Checks if the key exists in self._deployments.""" - - return key in self._deployments From 706290a02694669b5c55e4e689b2792db1c31208 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Mon, 14 Mar 2022 15:47:51 -0700 Subject: [PATCH 49/94] Add test for immutable list --- python/ray/serve/tests/test_application.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/python/ray/serve/tests/test_application.py b/python/ray/serve/tests/test_application.py index 55207da550b1..d5037cc290d2 100644 --- a/python/ray/serve/tests/test_application.py +++ b/python/ray/serve/tests/test_application.py @@ -355,7 +355,7 @@ def test_deploy_from_yaml(self, serve_instance): @pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") -def test_get_item(serve_instance): +def test_immutable_deployment_list(serve_instance): config_file_name = os.path.join( os.path.dirname(__file__), "test_config_files", "two_deployments.yaml" ) @@ -363,6 +363,12 @@ def test_get_item(serve_instance): with open(config_file_name, "r") as f: app = Application.from_yaml(f) + assert len(app.deployments) == 2 + + for i in range(len(app.deployments)): + with pytest.raises(RuntimeError): + app.deployments[i] = app.deployments[i].options(name="sneaky") + for deployment in app.deployments: deployment.deploy() From 4453f0882af151ca6348686f37d1e7cbe840f9be Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Mon, 14 Mar 2022 15:53:30 -0700 Subject: [PATCH 50/94] Add type hint --- python/ray/serve/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index daf87919ec5f..cec20db3a616 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1557,7 +1557,7 @@ def __init__(self, deployments: List[Deployment], ingress: Deployment = None): deployments = deployments or [] - self._deployments = dict() + self._deployments: Dict[str, Deployment] = dict() for d in deployments: self._add_deployment(d) From 3edcee4b3dda8c54265db645c72461634aebe1b3 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Mon, 14 Mar 2022 17:20:22 -0700 Subject: [PATCH 51/94] Convert to dict --- dashboard/modules/serve/serve_head.py | 4 +- python/ray/serve/api.py | 55 ++-------------------- python/ray/serve/scripts.py | 4 +- python/ray/serve/tests/test_application.py | 18 +++---- 4 files changed, 18 insertions(+), 63 deletions(-) diff --git a/dashboard/modules/serve/serve_head.py b/dashboard/modules/serve/serve_head.py index 7f81ac0ac66c..1c4b512c5172 100644 --- a/dashboard/modules/serve/serve_head.py +++ b/dashboard/modules/serve/serve_head.py @@ -54,10 +54,10 @@ async def delete_serve_application(self, req: Request) -> Response: @optional_utils.init_ray_and_catch_exceptions(connect_to_serve=True) async def put_all_deployments(self, req: Request) -> Response: app = Application.from_dict(await req.json()) - deploy_group(app.deployments, blocking=False) + deploy_group(app.deployments.values(), blocking=False) new_names = set() - for deployment in app.deployments: + for deployment in app.deployments.values(): new_names.add(deployment.name) all_deployments = serve.list_deployments() diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 1904479f9d2c..3b59b2fcb6b0 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -19,7 +19,6 @@ Type, Union, List, - Iterable, overload, ) @@ -1519,16 +1518,10 @@ def get_deployment_statuses() -> Dict[str, DeploymentStatusInfo]: return internal_get_global_client().get_deployment_statuses() -<<<<<<< HEAD -class ImmutableDeploymentList(list): - def __init__(self, deployments: Iterable[Deployment]): - super().__init__() - [self.append(deployment) for deployment in deployments] -======= class ImmutableDeploymentDict(dict): - def __init__(self, deployments: List[Deployment]): - raise NotImplementedError() ->>>>>>> f646d3fc312f63a6cf3e59a00ae1b3d6ab40393a + def __init__(self, deployments: Dict[str, Deployment]): + super().__init__() + self.update(deployments) def __setitem__(self, *args): """Not allowed. Modify deployment options using set_options instead.""" @@ -1556,7 +1549,6 @@ class Application: to production using the Serve CLI or REST API. """ -<<<<<<< HEAD def __init__(self, deployments: List[Deployment], ingress: Deployment = None): # TODO (shrekris-anyscale): validate ingress @@ -1572,21 +1564,9 @@ def __init__(self, deployments: List[Deployment], ingress: Deployment = None): def ingress(self) -> Deployment: return self._ingress - @property - def deployments(self) -> ImmutableDeploymentList: - return ImmutableDeploymentList(self._deployments.values()) -======= - def __init__(self, ingress: Deployment, deployments: List[Deployment]): - raise NotImplementedError() - - @property - def ingress(self) -> Deployment: - raise NotImplementedError() - @property def deployments(self) -> ImmutableDeploymentDict: - raise NotImplementedError() ->>>>>>> f646d3fc312f63a6cf3e59a00ae1b3d6ab40393a + return ImmutableDeploymentDict(self._deployments) def to_dict(self) -> Dict: """Returns this Application's deployments as a dictionary. @@ -1597,12 +1577,8 @@ def to_dict(self) -> Dict: Returns: Dict: The Application's deployments formatted in a dictionary. """ -<<<<<<< HEAD return serve_application_to_schema(self._deployments.values()).dict() -======= - raise NotImplementedError() ->>>>>>> f646d3fc312f63a6cf3e59a00ae1b3d6ab40393a @classmethod def from_dict(cls, d: Dict) -> "Application": @@ -1618,13 +1594,9 @@ def from_dict(cls, d: Dict) -> "Application": Returns: Application: a new application object containing the deployments. """ -<<<<<<< HEAD schema = ServeApplicationSchema.parse_obj(d) return cls(schema_to_serve_application(schema)) -======= - raise NotImplementedError() ->>>>>>> f646d3fc312f63a6cf3e59a00ae1b3d6ab40393a def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: """Returns this application's deployments as a YAML string. @@ -1646,16 +1618,12 @@ def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: Optional[String]: The deployments' YAML string. The output is from yaml.safe_dump(). Returned only if no file pointer is passed in. """ -<<<<<<< HEAD deployment_dict = serve_application_to_schema(self._deployments.values()).dict() if f: yaml.safe_dump(deployment_dict, f, default_flow_style=False) return yaml.safe_dump(deployment_dict, default_flow_style=False) -======= - raise NotImplementedError() ->>>>>>> f646d3fc312f63a6cf3e59a00ae1b3d6ab40393a @classmethod def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": @@ -1679,7 +1647,6 @@ def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": Serve YAML config files. Returns: -<<<<<<< HEAD Application: a new Application object containing the deployments. """ @@ -1715,11 +1682,6 @@ def _add_deployment(self, deployment: Deployment): ) self._deployments[deployment.name] = deployment -======= - Application: a new application object containing the deployments. - """ - raise NotImplementedError() ->>>>>>> f646d3fc312f63a6cf3e59a00ae1b3d6ab40393a @PublicAPI(stability="alpha") @@ -1735,7 +1697,6 @@ def run( If a DeploymentNode is passed in, all of the deployments it depends on will be deployed. """ -<<<<<<< HEAD deployments = _get_deployments_from_target(target) @@ -1773,9 +1734,6 @@ def deploy_group(deployments: List[Deployment], *, blocking=True) -> RayServeHan parameter_group.append(deployment_parameters) internal_get_global_client().deploy_group(parameter_group, _blocking=blocking) -======= - raise NotImplementedError() ->>>>>>> f646d3fc312f63a6cf3e59a00ae1b3d6ab40393a @PublicAPI(stability="alpha") @@ -1797,7 +1755,6 @@ def build(target: DeploymentNode) -> Application: # TODO(edoakes): this should accept host and port, but we don't # currently support them in the REST API. raise NotImplementedError() -<<<<<<< HEAD def _get_deployments_from_target( @@ -1806,7 +1763,7 @@ def _get_deployments_from_target( """Validates target and gets its deployments.""" if isinstance(target, Application): - return target.deployments + return target.deployments.values() elif isinstance(target, DeploymentNode): return [] # return extract_deployments_from_serve_dag(target) @@ -1904,5 +1861,3 @@ def serve_application_status_to_schema( for deployment_name, status_info in status_infos.items() ] return ServeApplicationStatusSchema(statuses=schemas) -======= ->>>>>>> f646d3fc312f63a6cf3e59a00ae1b3d6ab40393a diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 36b6030d04e0..23d0c32b342a 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -359,7 +359,7 @@ def run( ray.init(address=address, namespace="serve", runtime_env=ray_runtime_env) - for deployment in app.deployments: + for deployment in app.deployments.values(): _configure_runtime_env(deployment, runtime_env_updates) try: @@ -375,7 +375,7 @@ def run( except KeyboardInterrupt: cli_logger.info("Got SIGINT (KeyboardInterrupt). Removing deployments.") - for deployment in app.deployments: + for deployment in app.deployments.values(): deployment.delete() if len(serve.list_deployments()) == 0: cli_logger.info("No deployments left. Shutting down Serve.") diff --git a/python/ray/serve/tests/test_application.py b/python/ray/serve/tests/test_application.py index d5037cc290d2..d9e521145f86 100644 --- a/python/ray/serve/tests/test_application.py +++ b/python/ray/serve/tests/test_application.py @@ -28,7 +28,7 @@ def test_add_deployment_valid(self): app._add_deployment(self.C) assert len(app.deployments) == 2 - app_deployment_names = {d.name for d in app.deployments} + app_deployment_names = {d.name for d in app.deployments.values()} assert "f" in app_deployment_names assert "C" in app_deployment_names @@ -71,7 +71,7 @@ def deploy_and_check_responses( the client to wait until the deployments finish deploying. """ - deploy_group(Application(deployments).deployments, blocking=blocking) + deploy_group(Application(deployments).deployments.values(), blocking=blocking) def check_all_deployed(): try: @@ -144,7 +144,7 @@ async def request_echo(self, echo: str): MutualHandles.options(name=deployment_name, init_args=(handle_name,)) ) - deploy_group(Application(deployments).deployments, blocking=True) + deploy_group(Application(deployments).deployments.values(), blocking=True) for deployment in deployments: assert (ray.get(deployment.get_handle().remote("hello"))) == "hello" @@ -309,7 +309,7 @@ def test_deploy_from_dict(self, serve_instance): compare_specified_options(config_dict, app_dict) - deploy_group(app.deployments) + deploy_group(app.deployments.values()) assert ( requests.get("http://localhost:8000/shallow").text == "Hello shallow world!" @@ -335,7 +335,7 @@ def test_deploy_from_yaml(self, serve_instance): compare_specified_options(app1.to_dict(), app2.to_dict()) # Check that deployment works - deploy_group(app1.deployments) + deploy_group(app1.deployments.values()) assert ( requests.get("http://localhost:8000/shallow").text == "Hello shallow world!" ) @@ -363,13 +363,13 @@ def test_immutable_deployment_list(serve_instance): with open(config_file_name, "r") as f: app = Application.from_yaml(f) - assert len(app.deployments) == 2 + assert len(app.deployments.values()) == 2 - for i in range(len(app.deployments)): + for name in app.deployments.keys(): with pytest.raises(RuntimeError): - app.deployments[i] = app.deployments[i].options(name="sneaky") + app.deployments[name] = app.deployments[name].options(name="sneaky") - for deployment in app.deployments: + for deployment in app.deployments.values(): deployment.deploy() assert requests.get("http://localhost:8000/shallow").text == "Hello shallow world!" From 6b4c7c50d2568c2799b56a7a752c6f68c94ef1bf Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Mon, 14 Mar 2022 19:49:45 -0700 Subject: [PATCH 52/94] Update error message in _add_deployment --- python/ray/serve/api.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 3b59b2fcb6b0..37c019edaf23 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1658,14 +1658,12 @@ def _add_deployment(self, deployment: Deployment): """Adds the deployment to this Serve application. Validates that a deployment with the same name doesn't already exist. - To overwrite an existing deployment, use index notation. For example: - - app[deployment_name] = deployment Args: deployment (Deployment): deployment to add to this Application. Raises: + TypeError: If a non-Deployment object is passed in. ValueError: If a deployment with deployment.name already exists in this Application. """ @@ -1673,13 +1671,7 @@ def _add_deployment(self, deployment: Deployment): if not isinstance(deployment, Deployment): raise TypeError(f"Got {type(deployment)}. Expected deployment.") elif deployment.name in self._deployments: - raise ValueError( - f'Deployment with name "{deployment.name}" already ' - "exists in this application. To overwrite this " - "deployment, use attribute or index notation. " - "For example:\n\napp.deployment_name = " - "new_deployment" - ) + raise ValueError(f'App already has deployment named "{deployment.name}".') self._deployments[deployment.name] = deployment From 4f15994055415232f17aa0e04fb95fe2a11f8f58 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Mon, 14 Mar 2022 19:56:40 -0700 Subject: [PATCH 53/94] Remove _add_deployment --- python/ray/serve/api.py | 29 ++++++---------------- python/ray/serve/tests/test_application.py | 18 +++++++------- 2 files changed, 16 insertions(+), 31 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 37c019edaf23..fea7ffceaa9c 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1558,7 +1558,13 @@ def __init__(self, deployments: List[Deployment], ingress: Deployment = None): self._deployments: Dict[str, Deployment] = dict() for d in deployments: - self._add_deployment(d) + + if not isinstance(d, Deployment): + raise TypeError(f"Got {type(d)}. Expected deployment.") + elif d.name in self._deployments: + raise ValueError(f'App got multiple deployments named "{d.name}".') + + self._deployments[d.name] = d @property def ingress(self) -> Deployment: @@ -1654,27 +1660,6 @@ def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": schema = ServeApplicationSchema.parse_obj(deployments_json) return cls(schema_to_serve_application(schema)) - def _add_deployment(self, deployment: Deployment): - """Adds the deployment to this Serve application. - - Validates that a deployment with the same name doesn't already exist. - - Args: - deployment (Deployment): deployment to add to this Application. - - Raises: - TypeError: If a non-Deployment object is passed in. - ValueError: If a deployment with deployment.name already exists in - this Application. - """ - - if not isinstance(deployment, Deployment): - raise TypeError(f"Got {type(deployment)}. Expected deployment.") - elif deployment.name in self._deployments: - raise ValueError(f'App already has deployment named "{deployment.name}".') - - self._deployments[deployment.name] = deployment - @PublicAPI(stability="alpha") def run( diff --git a/python/ray/serve/tests/test_application.py b/python/ray/serve/tests/test_application.py index d9e521145f86..d2a87b0e3471 100644 --- a/python/ray/serve/tests/test_application.py +++ b/python/ray/serve/tests/test_application.py @@ -12,7 +12,7 @@ from ray._private.test_utils import wait_for_condition -class TestAddDeployment: +class TestApplicationConstruction: @serve.deployment def f(*args): return "got f" @@ -22,25 +22,25 @@ class C: def __call__(self, *args): return "got C" - def test_add_deployment_valid(self): - app = Application([]) - app._add_deployment(self.f) - app._add_deployment(self.C) + def test_valid_deployments(self): + app = Application([self.f, self.C]) assert len(app.deployments) == 2 app_deployment_names = {d.name for d in app.deployments.values()} assert "f" in app_deployment_names assert "C" in app_deployment_names - def test_add_deployment_repeat_name(self): + def test_repeated_deployment_names(self): with pytest.raises(ValueError): - app = Application([]) - app._add_deployment(self.f) - app._add_deployment(self.C.options(name="f")) + Application([self.f, self.C.options(name="f")]) with pytest.raises(ValueError): Application([self.C, self.f.options(name="C")]) + def test_non_deployments(self): + with pytest.raises(TypeError): + Application([self.f, 5, "hello"]) + class TestDeployGroup: @serve.deployment From d1f661688b23797c586c6d761bc2d6860277595c Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Tue, 15 Mar 2022 12:11:33 -0500 Subject: [PATCH 54/94] fix --- python/ray/serve/scripts.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 0b5b50cbb67c..98b4aaff166c 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -10,7 +10,7 @@ from ray._private.utils import import_attr from ray.serve.config import DeploymentMode from ray import serve -from ray.serve import get_deployment_statuses +from ray.serve.api import get_deployment_statuses from ray.serve.constants import ( DEFAULT_CHECKPOINT_PATH, DEFAULT_HTTP_HOST, @@ -234,6 +234,10 @@ def deploy(config_file_name: str, address: str): @click.option( "--blocking/--non-blocking", default=True, + help=( + "Whether or not this command should be blocking. If blocking, it " + "will loop and log status until Ctrl-C'd, then clean up the app." + ), ) def run( config_or_import_path: str, From 5542e782c02b8ba0486386e43d252f6b84276a85 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Tue, 15 Mar 2022 10:30:42 -0700 Subject: [PATCH 55/94] Remove app-level import path conversion --- python/ray/serve/application.py | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/python/ray/serve/application.py b/python/ray/serve/application.py index 9f4f0f32eb98..ef30fc3d9e60 100644 --- a/python/ray/serve/application.py +++ b/python/ray/serve/application.py @@ -14,7 +14,7 @@ schema_to_serve_application, serve_application_status_to_schema, ) -from ray.serve.utils import logger, get_deployment_import_path +from ray.serve.utils import logger from ray.serve.api import get_deployment_statuses from ray.autoscaler._private.cli_logger import _CliLogger from logging import Logger @@ -155,8 +155,6 @@ def to_dict(self) -> Dict: Dict: The Application's deployments formatted in a dictionary. """ - self._convert_to_import_paths() - return serve_application_to_schema(self._deployments.values()).dict() @classmethod @@ -201,8 +199,6 @@ def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: yaml.safe_dump(). Returned only if no file pointer is passed in. """ - self._convert_to_import_paths() - deployment_dict = serve_application_to_schema(self._deployments.values()).dict() if f: @@ -238,18 +234,6 @@ def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": schema = ServeApplicationSchema.parse_obj(deployments_json) return cls(schema_to_serve_application(schema)) - def _convert_to_import_paths(self): - """Convert all func_or_classes to import paths. - - If any deployment's func_or_class is a function or class (and not an - import path), it overwrites it with that function or class's import path. - """ - - for name, deployment in self._deployments.items(): - self._deployments[name] = deployment.options( - func_or_class=get_deployment_import_path(deployment, replace_main=True) - ) - def __getitem__(self, key: str): """Fetch a deployment by name using dict syntax: app["name"] From af21886f170418f5011cd709b6d0ae0b1db5cfe5 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Tue, 15 Mar 2022 10:55:57 -0700 Subject: [PATCH 56/94] Update build to use new API --- python/ray/serve/scripts.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 737b95e8c337..0dd678e60e01 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -20,7 +20,7 @@ from ray.dashboard.modules.serve.sdk import ServeSubmissionClient from ray.autoscaler._private.cli_logger import cli_logger from ray.serve.application import Application -from ray.serve.api import Deployment +from ray.serve.api import Deployment, DeploymentNode, build as build_app from ray._private.utils import import_attr RAY_INIT_ADDRESS_HELP_STR = ( @@ -457,19 +457,31 @@ def delete(address: str, yes: bool): @cli.command( short_help="Build a Serve config using an Application", help=( - "Imports the Serve app or deployment stored at IMPORT_PATH, and " - "writes a config file to the FILE_PATH." + "Imports the Serve app or DeploymentNode stored at IMPORT_PATH, and " + "writes a config file to the FILE_PATH. The FILE_PATH must have " + 'the ".yaml" extension.' ), ) @click.argument("import_path") @click.argument("file_path") def build(import_path: str, file_path: str): - app_or_deployment: Union[Application, Deployment] = import_attr(import_path) + if not file_path.endswith(".yaml"): + raise ValueError( + f'Got "{file_path}" as FILE_PATH. FILE_PATH must end ' 'with ".yaml".' + ) + + app_or_node: Union[Application, DeploymentNode] = import_attr(import_path) - if isinstance(app_or_deployment, Deployment): - app = Application([app_or_deployment]) + if isinstance(app_or_node, DeploymentNode): + app = build_app(app_or_node) + elif isinstance(app, Application): + app = app_or_node else: - app = app_or_deployment + raise ValueError( + f'Got IMPORT_PATH "{import_path}", which contains ' + f'"{type(app_or_node)}" object. Expected either an Application or ' + "DeploymentNode." + ) with open(file_path, "w+") as f: app.to_yaml(f) From cc18a8b48ce8d4e595e6f1debe04b0acf56ad211 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Tue, 15 Mar 2022 10:57:14 -0700 Subject: [PATCH 57/94] Remove malformatted string --- python/ray/serve/scripts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 0dd678e60e01..003c8a0030e8 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -467,7 +467,7 @@ def delete(address: str, yes: bool): def build(import_path: str, file_path: str): if not file_path.endswith(".yaml"): raise ValueError( - f'Got "{file_path}" as FILE_PATH. FILE_PATH must end ' 'with ".yaml".' + f'Got "{file_path}" as FILE_PATH. FILE_PATH must end with ".yaml".' ) app_or_node: Union[Application, DeploymentNode] = import_attr(import_path) From e649c3e93844be95ce286339a5c2b7c9e82db0b2 Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Tue, 15 Mar 2022 19:44:08 -0500 Subject: [PATCH 58/94] fix --- python/ray/serve/api.py | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index b0cda6cc54b1..b7378e35f200 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1643,7 +1643,7 @@ def _get_deployments_from_node(node: DeploymentNode) -> List[Deployment]: Returns: deployment_list(List[Deployment]): the list of Deployment objects. The - last element correspond to the node passed in to this function. + last element corresponds to the node passed in to this function. """ from ray.serve.pipeline.api import build as pipeline_build @@ -1668,21 +1668,36 @@ def run( will be deployed. """ + client = start(detached=True, http_options={"host": host, "port": port}) + if isinstance(target, DeploymentNode): deployments = _get_deployments_from_node(target) else: raise NotImplementedError() - for d in deployments: - logger.debug(f"Deploying {d}") - d.deploy() + parameter_group = [ + { + "name": deployment._name, + "func_or_class": deployment._func_or_class, + "init_args": deployment.init_args, + "init_kwargs": deployment.init_kwargs, + "ray_actor_options": deployment._ray_actor_options, + "config": deployment._config, + "version": deployment._version, + "prev_version": deployment._prev_version, + "route_prefix": deployment.route_prefix, + "url": deployment.url, + } + for deployment in deployments + ] + client.deploy_group(parameter_group, _blocking=True) return deployments[-1].get_handle() @PublicAPI(stability="alpha") def build(target: DeploymentNode) -> Application: - """Builds a Serve application into a static configuration. + """Builds a Serve application into a static application. Takes in a DeploymentNode and converts it to a Serve application consisting of one or more deployments. This is intended to be used for From 301044741308cb4aded8006c5cb3048141248c9d Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Tue, 15 Mar 2022 20:11:33 -0700 Subject: [PATCH 59/94] Update test_convert_to_import_path --- python/ray/serve/application.py | 285 --------------------- python/ray/serve/tests/test_application.py | 2 +- 2 files changed, 1 insertion(+), 286 deletions(-) delete mode 100644 python/ray/serve/application.py diff --git a/python/ray/serve/application.py b/python/ray/serve/application.py deleted file mode 100644 index ef30fc3d9e60..000000000000 --- a/python/ray/serve/application.py +++ /dev/null @@ -1,285 +0,0 @@ -from typing import List, TextIO, Optional, Union, Dict -import yaml -import time -import sys - -from ray import serve -from ray.serve.api import ( - Deployment, - internal_get_global_client, -) -from ray.serve.schema import ( - ServeApplicationSchema, - serve_application_to_schema, - schema_to_serve_application, - serve_application_status_to_schema, -) -from ray.serve.utils import logger -from ray.serve.api import get_deployment_statuses -from ray.autoscaler._private.cli_logger import _CliLogger -from logging import Logger - - -class Application: - """Used to deploy, update, and monitor groups of Serve deployments. - - Args: - deployments (List[Deployment]): The Serve Application is - initialized to contain this group of deployments. - """ - - def __init__(self, deployments: List[Deployment] = None): - deployments = deployments or [] - - self._deployments: Dict[str, Deployment] = dict() - for d in deployments: - self.add_deployment(d) - - def add_deployment(self, deployment: Deployment): - """Adds the deployment to this Serve application. - - Validates that a deployment with the same name doesn't already exist. - To overwrite an existing deployment, use index notation. For example: - - app[deployment_name] = deployment - - Args: - deployment (Deployment): deployment to add to this Application. - - Raises: - ValueError: If a deployment with deployment.name already exists in - this Application. - """ - - if not isinstance(deployment, Deployment): - raise TypeError(f"Got {type(deployment)}. Expected deployment.") - elif deployment.name in self._deployments: - raise ValueError( - f'Deployment with name "{deployment.name}" already ' - "exists in this application. To overwrite this " - "deployment, use attribute or index notation. " - "For example:\n\napp.deployment_name = " - "new_deployment" - ) - - self._deployments[deployment.name] = deployment - - def deploy(self, blocking: bool = True): - """Atomically deploys the Application's deployments to the Ray cluster. - - The Application's deployments can carry handles to one another. - - Args: - blocking (bool): If True, this function only returns after the - deployment is finished. If False, this function returns - immediately after requesting the deployment. - """ - - if len(self._deployments) == 0: - return - - parameter_group = [] - - for deployment in self._deployments.values(): - if not isinstance(deployment, Deployment): - raise TypeError( - f"deploy_group only accepts Deployments, but got unexpected " - f"type {type(deployment)}." - ) - - deployment_parameters = { - "name": deployment._name, - "func_or_class": deployment._func_or_class, - "init_args": deployment.init_args, - "init_kwargs": deployment.init_kwargs, - "ray_actor_options": deployment._ray_actor_options, - "config": deployment._config, - "version": deployment._version, - "prev_version": deployment._prev_version, - "route_prefix": deployment.route_prefix, - "url": deployment.url, - } - - parameter_group.append(deployment_parameters) - - return internal_get_global_client().deploy_group( - parameter_group, _blocking=blocking - ) - - def run(self, logger: Union[Logger, _CliLogger] = logger): - """Deploys all deployments in this Application and logs status. - - This function keeps looping and printing status, so it must be manually - killed (e.g. using ctrl-C). When it recieves a kill signal, it tears - down all deployments that it deployed. If there are no deployments - left, it also shuts down the rest of Serve, including the controller. - This is meant to help interactive development. - - Args: - logger: Any Python object that implements the standard logger - interface. - """ - - try: - serve.start(detached=True) - self.deploy() - - logger.info("\nDeployed successfully!\n") - - while True: - statuses = serve_application_status_to_schema( - get_deployment_statuses() - ).json(indent=4) - logger.info(f"{statuses}") - time.sleep(10) - - except KeyboardInterrupt: - logger.info("Got SIGINT (KeyboardInterrupt). Removing deployments.") - for deployment in self._deployments.values(): - deployment.delete() - if len(serve.list_deployments()) == 0: - logger.info("No deployments left. Shutting down Serve.") - serve.shutdown() - sys.exit() - - def to_dict(self) -> Dict: - """Returns this Application's deployments as a dictionary. - - This dictionary adheres to the Serve REST API schema. It can be deployed - via the Serve REST API. - - If any deployment's func_or_class is a function or class (and not an - import path), it overwrites it with that function or class's import path. - - Returns: - Dict: The Application's deployments formatted in a dictionary. - """ - - return serve_application_to_schema(self._deployments.values()).dict() - - @classmethod - def from_dict(cls, d: Dict) -> "Application": - """Converts a dictionary of deployment data to an Application. - - Takes in a dictionary matching the Serve REST API schema and converts - it to an Application containing those deployments. - - Args: - d (Dict): A dictionary containing the deployments' data that matches - the Serve REST API schema. - - Returns: - Application: a new Application object containing the deployments. - """ - - schema = ServeApplicationSchema.parse_obj(d) - return cls(schema_to_serve_application(schema)) - - def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: - """Returns this Application's deployments as a YAML string. - - Optionally writes the YAML string to a file as well. To write to a - file, use this pattern: - - with open("file_name.txt", "w") as f: - app.to_yaml(f=f) - - This file is formatted as a Serve YAML config file. It can be deployed - via the Serve CLI. - - If any deployment's func_or_class is a function or class (and not an - import path), it overwrites it with that function or class's import path. - - Args: - f (Optional[TextIO]): A pointer to the file where the YAML should - be written. - - Returns: - Optional[String]: The deployments' YAML string. The output is from - yaml.safe_dump(). Returned only if no file pointer is passed in. - """ - - deployment_dict = serve_application_to_schema(self._deployments.values()).dict() - - if f: - yaml.safe_dump(deployment_dict, f, default_flow_style=False) - return yaml.safe_dump(deployment_dict, default_flow_style=False) - - @classmethod - def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": - """Converts YAML data to deployments for an Application. - - Takes in a string or a file pointer to a file containing deployment - definitions in YAML. These definitions are converted to a new - Application object containing the deployments. - - To read from a file, use the following pattern: - - with open("file_name.txt", "w") as f: - app = app.from_yaml(str_or_file) - - Args: - str_or_file (Union[String, TextIO]): Either a string containing - YAML deployment definitions or a pointer to a file containing - YAML deployment definitions. The YAML format must adhere to the - ServeApplicationSchema JSON Schema defined in - ray.serve.schema. This function works with - Serve YAML config files. - - Returns: - Application: a new Application object containing the deployments. - """ - - deployments_json = yaml.safe_load(str_or_file) - schema = ServeApplicationSchema.parse_obj(deployments_json) - return cls(schema_to_serve_application(schema)) - - def __getitem__(self, key: str): - """Fetch a deployment by name using dict syntax: app["name"] - - Raises: - KeyError: if the name is not in this Application. - """ - - if key in self._deployments: - return self._deployments[key] - else: - raise KeyError(f'Serve application does not contain a "{key}" deployment.') - - def __setitem__(self, key: str, value: Deployment): - """Set a deployment by name with dict syntax: app[name]=new_deployment - - Use this to overwrite existing deployments. - - Args: - key (String): name - value (Deployment): the deployment that the name maps to - - Raises: - TypeError: if the key is not a String or the value is not a deployment. - """ - - if not isinstance(key, str): - raise TypeError(f"key should be a string, but got object of type {key}.") - elif not isinstance(value, Deployment): - raise TypeError(f"Got {type(Deployment)} for value. Expected deployment.") - - self._deployments[key] = value - - def __iter__(self): - """Iterator over Application's deployments. - - Enables "for deployment in Application" pattern. - """ - - return iter(self._deployments.values()) - - def __len__(self): - """Number of deployments in this Application.""" - - return len(self._deployments) - - def __contains__(self, key: str): - """Checks if the key exists in self._deployments.""" - - return key in self._deployments diff --git a/python/ray/serve/tests/test_application.py b/python/ray/serve/tests/test_application.py index 69a985294a0e..d358f3248502 100644 --- a/python/ray/serve/tests/test_application.py +++ b/python/ray/serve/tests/test_application.py @@ -360,7 +360,7 @@ def test_convert_to_import_path(self, serve_instance): reconstructed_app = Application.from_yaml(app.to_yaml()) - reconstructed_app.deploy() + deploy_group(reconstructed_app.deployments.values()) assert requests.get("http://localhost:8000/f").text == "got decorated func" assert requests.get("http://localhost:8000/C").text == "got decorated class" From a2df33a63cc7bd37eb3699b47019f581e09c4e79 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Tue, 15 Mar 2022 23:34:48 -0700 Subject: [PATCH 60/94] Remove bad merge --- python/ray/serve/api.py | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index ebeb474dc1ab..e4eaa3cf034a 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1703,7 +1703,6 @@ def run( will be deployed. """ -<<<<<<< HEAD deployments = _get_deployments_from_target(target) # if isinstance(target, DeploymentNode): @@ -1751,33 +1750,6 @@ def deploy_group(deployments: List[Deployment], *, blocking=True) -> RayServeHan parameter_group.append(deployment_parameters) internal_get_global_client().deploy_group(parameter_group, _blocking=blocking) -======= - client = start(detached=True, http_options={"host": host, "port": port}) - - if isinstance(target, DeploymentNode): - deployments = _get_deployments_from_node(target) - else: - raise NotImplementedError() - - parameter_group = [ - { - "name": deployment._name, - "func_or_class": deployment._func_or_class, - "init_args": deployment.init_args, - "init_kwargs": deployment.init_kwargs, - "ray_actor_options": deployment._ray_actor_options, - "config": deployment._config, - "version": deployment._version, - "prev_version": deployment._prev_version, - "route_prefix": deployment.route_prefix, - "url": deployment.url, - } - for deployment in deployments - ] - - client.deploy_group(parameter_group, _blocking=True) - return deployments[-1].get_handle() ->>>>>>> fix-serve-run @PublicAPI(stability="alpha") From 101f039b877962884d3441dfd79e6bb2f9648107 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 00:20:14 -0700 Subject: [PATCH 61/94] Implement basic DeploymentNode handling --- python/ray/serve/api.py | 85 +++++++++++++++-------------------------- 1 file changed, 31 insertions(+), 54 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index e4eaa3cf034a..c1a2c6abce85 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1673,22 +1673,6 @@ def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": return cls(schema_to_serve_application(schema)) -def _get_deployments_from_node(node: DeploymentNode) -> List[Deployment]: - """Generate a list of deployment objects from a root node. - - Returns: - deployment_list(List[Deployment]): the list of Deployment objects. The - last element corresponds to the node passed in to this function. - """ - from ray.serve.pipeline.api import build as pipeline_build - - with PipelineInputNode() as input_node: - root = node.__call__.bind(input_node) - deployments = pipeline_build(root, inject_ingress=False) - - return deployments - - @PublicAPI(stability="alpha") def run( target: Union[DeploymentNode, Application], @@ -1703,32 +1687,23 @@ def run( will be deployed. """ - deployments = _get_deployments_from_target(target) - - # if isinstance(target, DeploymentNode): - # deployments = _get_deployments_from_node(target) - # else: - # raise NotImplementedError() - - # for d in deployments: - # logger.debug(f"Deploying {d}") - # d.deploy() + if isinstance(target, Application): + deployments = target.deployments.values() + elif isinstance(target, DeploymentNode): + deployments = _get_deployments_from_node(target) + else: + raise TypeError( + "Expected a DeploymentNode or " + "Application as target. Got unexpected type " + f'"{type(target)}" instead.' + ) - # return deployments[-1].get_handle() + if len(deployments) == 0: + return # TODO (shrekris-anyscale): validate ingress - start(detached=True, http_options={"host": host, "port": port}) - deploy_group(deployments, blocking=True) - - # TODO (shrekris-anyscale): return handle to ingress deployment - - -def deploy_group(deployments: List[Deployment], *, blocking=True) -> RayServeHandle: - """Atomically deploys a group of deployments.""" - - if len(deployments) == 0: - return + client = start(detached=True, http_options={"host": host, "port": port}) parameter_group = [] @@ -1749,7 +1724,11 @@ def deploy_group(deployments: List[Deployment], *, blocking=True) -> RayServeHan parameter_group.append(deployment_parameters) - internal_get_global_client().deploy_group(parameter_group, _blocking=blocking) + client.deploy_group(parameter_group, _blocking=True) + + return deployments[-1].get_handle() + + # TODO (shrekris-anyscale): return handle to ingress deployment @PublicAPI(stability="alpha") @@ -1773,22 +1752,20 @@ def build(target: DeploymentNode) -> Application: raise NotImplementedError() -def _get_deployments_from_target( - target: Union[DeploymentNode, Application] -) -> List[Deployment]: - """Validates target and gets its deployments.""" +def _get_deployments_from_node(node: DeploymentNode) -> List[Deployment]: + """Generate a list of deployment objects from a root node. - if isinstance(target, Application): - return target.deployments.values() - elif isinstance(target, DeploymentNode): - return [] - # return extract_deployments_from_serve_dag(target) - else: - raise TypeError( - "Expected a DeploymentNode or " - "Application as target. Got unexpected type " - f'"{type(target)}" instead.' - ) + Returns: + deployment_list(List[Deployment]): the list of Deployment objects. The + last element corresponds to the node passed in to this function. + """ + from ray.serve.pipeline.api import build as pipeline_build + + with PipelineInputNode() as input_node: + root = node.__call__.bind(input_node) + deployments = pipeline_build(root, inject_ingress=False) + + return deployments def deployment_to_schema(d: Deployment) -> DeploymentSchema: From 9f857cb07b927e9ac72bef99d454617da741a22d Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 00:51:27 -0700 Subject: [PATCH 62/94] Restore deploy_group --- python/ray/serve/api.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index c1a2c6abce85..387983620031 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1703,12 +1703,24 @@ def run( # TODO (shrekris-anyscale): validate ingress - client = start(detached=True, http_options={"host": host, "port": port}) + start(detached=True, http_options={"host": host, "port": port}) + + deploy_group(deployments, blocking=True) + + return deployments[-1].get_handle() + + # TODO (shrekris-anyscale): return handle to ingress deployment + + +def deploy_group(deployments: List[Deployment], *, blocking=True) -> RayServeHandle: + """Atomically deploys a group of deployments.""" + + if len(deployments) == 0: + return parameter_group = [] for deployment in deployments: - deployment_parameters = { "name": deployment._name, "func_or_class": deployment._func_or_class, @@ -1724,11 +1736,7 @@ def run( parameter_group.append(deployment_parameters) - client.deploy_group(parameter_group, _blocking=True) - - return deployments[-1].get_handle() - - # TODO (shrekris-anyscale): return handle to ingress deployment + internal_get_global_client().deploy_group(parameter_group, _blocking=blocking) @PublicAPI(stability="alpha") From 62804daa880fda18818228ce0f286b1176bc3b79 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 00:53:30 -0700 Subject: [PATCH 63/94] Remove repeated imports --- python/ray/serve/scripts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 60a5aa7a939d..4f831cef3ed1 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -21,7 +21,6 @@ from ray.dashboard.modules.dashboard_sdk import parse_runtime_env_args from ray.dashboard.modules.serve.sdk import ServeSubmissionClient from ray.autoscaler._private.cli_logger import cli_logger -from ray.serve.api import Deployment, DeploymentNode, build as build_app from ray._private.utils import import_attr from ray.serve.api import ( Application, @@ -31,6 +30,7 @@ get_deployment_statuses, serve_application_status_to_schema, ) + build_app = build RAY_INIT_ADDRESS_HELP_STR = ( From 6768271118a6657ab7ef85d1569f5d55178cdf79 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 00:56:13 -0700 Subject: [PATCH 64/94] Update application import --- python/ray/serve/tests/test_cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ray/serve/tests/test_cli.py b/python/ray/serve/tests/test_cli.py index b4fbe2e80bf8..7ca74cc09bab 100644 --- a/python/ray/serve/tests/test_cli.py +++ b/python/ray/serve/tests/test_cli.py @@ -14,7 +14,7 @@ from ray._private.test_utils import wait_for_condition from ray.dashboard.optional_utils import RAY_INTERNAL_DASHBOARD_NAMESPACE from ray.serve.scripts import _process_args_and_kwargs, _configure_runtime_env -from ray.serve.application import Application +from ray.serve.api import Application def ping_endpoint(endpoint: str, params: str = ""): From 8fd364acf0fcf6704b7efff372a93386953e1d3b Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Wed, 16 Mar 2022 10:59:04 -0500 Subject: [PATCH 65/94] working --- dashboard/modules/serve/serve_head.py | 7 +-- python/ray/serve/api.py | 82 ++++++++++--------------- python/ray/serve/pipeline/json_serde.py | 10 +++ python/ray/serve/utils.py | 2 +- 4 files changed, 48 insertions(+), 53 deletions(-) diff --git a/dashboard/modules/serve/serve_head.py b/dashboard/modules/serve/serve_head.py index 1c4b512c5172..547ed7a143a6 100644 --- a/dashboard/modules/serve/serve_head.py +++ b/dashboard/modules/serve/serve_head.py @@ -1,4 +1,5 @@ from aiohttp.web import Request, Response +import json import logging import ray.dashboard.utils as dashboard_utils @@ -9,7 +10,6 @@ Application, deploy_group, get_deployment_statuses, - serve_application_to_schema, serve_application_status_to_schema, ) @@ -26,10 +26,9 @@ def __init__(self, dashboard_head): @routes.get("/api/serve/deployments/") @optional_utils.init_ray_and_catch_exceptions(connect_to_serve=True) async def get_all_deployments(self, req: Request) -> Response: - deployments = list(serve.list_deployments().values()) - serve_application_schema = serve_application_to_schema(deployments=deployments) + app = Application(list(serve.list_deployments().values())) return Response( - text=serve_application_schema.json(), + text=json.dumps(app.to_dict()), content_type="application/json", ) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 387983620031..f2907b7bf185 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1561,30 +1561,21 @@ class Application: to production using the Serve CLI or REST API. """ - def __init__(self, deployments: List[Deployment], ingress: Deployment = None): - - # TODO (shrekris-anyscale): validate ingress - self._ingress = ingress - - deployments = deployments or [] - - self._deployments: Dict[str, Deployment] = dict() + def __init__(self, deployments: List[Deployment]): + deployment_dict = {} for d in deployments: - if not isinstance(d, Deployment): raise TypeError(f"Got {type(d)}. Expected deployment.") - elif d.name in self._deployments: - raise ValueError(f'App got multiple deployments named "{d.name}".') + elif d.name in deployment_dict: + raise ValueError(f"App got multiple deployments named '{d.name}'.") - self._deployments[d.name] = d + deployment_dict[d.name] = d - @property - def ingress(self) -> Deployment: - return self._ingress + self._deployments = ImmutableDeploymentDict(deployment_dict) @property def deployments(self) -> ImmutableDeploymentDict: - return ImmutableDeploymentDict(self._deployments) + return self._deployments def to_dict(self) -> Dict: """Returns this Application's deployments as a dictionary. @@ -1595,8 +1586,9 @@ def to_dict(self) -> Dict: Returns: Dict: The Application's deployments formatted in a dictionary. """ - - return serve_application_to_schema(self._deployments.values()).dict() + return ServeApplicationSchema( + deployments=[deployment_to_schema(d) for d in self._deployments.values()] + ).dict() @classmethod def from_dict(cls, d: Dict) -> "Application": @@ -1614,7 +1606,7 @@ def from_dict(cls, d: Dict) -> "Application": """ schema = ServeApplicationSchema.parse_obj(d) - return cls(schema_to_serve_application(schema)) + return cls([schema_to_deployment(s) for s in schema.deployments]) def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: """Returns this application's deployments as a YAML string. @@ -1636,12 +1628,7 @@ def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: Optional[String]: The deployments' YAML string. The output is from yaml.safe_dump(). Returned only if no file pointer is passed in. """ - - deployment_dict = serve_application_to_schema(self._deployments.values()).dict() - - if f: - yaml.safe_dump(deployment_dict, f, default_flow_style=False) - return yaml.safe_dump(deployment_dict, default_flow_style=False) + return yaml.safe_dump(self.to_dict(), stream=f, default_flow_style=False) @classmethod def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": @@ -1667,10 +1654,7 @@ def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": Returns: Application: a new Application object containing the deployments. """ - - deployments_json = yaml.safe_load(str_or_file) - schema = ServeApplicationSchema.parse_obj(deployments_json) - return cls(schema_to_serve_application(schema)) + return cls.from_dict(yaml.safe_load(str_or_file)) @PublicAPI(stability="alpha") @@ -1688,7 +1672,7 @@ def run( """ if isinstance(target, Application): - deployments = target.deployments.values() + deployments = list(target.deployments.values()) elif isinstance(target, DeploymentNode): deployments = _get_deployments_from_node(target) else: @@ -1757,7 +1741,7 @@ def build(target: DeploymentNode) -> Application: """ # TODO(edoakes): this should accept host and port, but we don't # currently support them in the REST API. - raise NotImplementedError() + return Application(_get_deployments_from_node(target)) def _get_deployments_from_node(node: DeploymentNode) -> List[Deployment]: @@ -1777,6 +1761,16 @@ def _get_deployments_from_node(node: DeploymentNode) -> List[Deployment]: def deployment_to_schema(d: Deployment) -> DeploymentSchema: + """Converts a live deployment object to a corresponding structured schema. + + If the deployment has a class or function, it will be attemptetd to be + converted to a valid corresponding import path. + + init_args and init_kwargs must also be JSON-serializable or this call will + fail. + """ + from ray.serve.pipeline.json_serde import convert_to_json_safe_obj + if d.ray_actor_options is not None: ray_actor_options_schema = RayActorOptionsSchema.parse_obj(d.ray_actor_options) else: @@ -1785,12 +1779,12 @@ def deployment_to_schema(d: Deployment) -> DeploymentSchema: return DeploymentSchema( name=d.name, import_path=get_deployment_import_path(d), - init_args=d.init_args, - init_kwargs=d.init_kwargs, + init_args=convert_to_json_safe_obj(d.init_args, err_key="init_args"), + init_kwargs=convert_to_json_safe_obj(d.init_kwargs, err_key="init_kwargs"), num_replicas=d.num_replicas, route_prefix=d.route_prefix, max_concurrent_queries=d.max_concurrent_queries, - user_config=d.user_config, + user_config=convert_to_json_safe_obj(d.user_config, err_key="user_config"), autoscaling_config=d._config.autoscaling_config, graceful_shutdown_wait_loop_s=d._config.graceful_shutdown_wait_loop_s, graceful_shutdown_timeout_s=d._config.graceful_shutdown_timeout_s, @@ -1801,6 +1795,8 @@ def deployment_to_schema(d: Deployment) -> DeploymentSchema: def schema_to_deployment(s: DeploymentSchema) -> Deployment: + from ray.serve.pipeline.json_serde import convert_from_json_safe_obj + if s.ray_actor_options is None: ray_actor_options = None else: @@ -1808,31 +1804,21 @@ def schema_to_deployment(s: DeploymentSchema) -> Deployment: return deployment( name=s.name, + init_args=convert_from_json_safe_obj(s.init_args), + init_kwargs=convert_from_json_safe_obj(s.init_kwargs), num_replicas=s.num_replicas, - init_args=s.init_args, - init_kwargs=s.init_kwargs, route_prefix=s.route_prefix, - ray_actor_options=ray_actor_options, max_concurrent_queries=s.max_concurrent_queries, + user_config=convert_from_json_safe_obj(s.user_config), _autoscaling_config=s.autoscaling_config, _graceful_shutdown_wait_loop_s=s.graceful_shutdown_wait_loop_s, _graceful_shutdown_timeout_s=s.graceful_shutdown_timeout_s, _health_check_period_s=s.health_check_period_s, _health_check_timeout_s=s.health_check_timeout_s, + ray_actor_options=ray_actor_options, )(s.import_path) -def serve_application_to_schema( - deployments: List[Deployment], -) -> ServeApplicationSchema: - schemas = [deployment_to_schema(d) for d in deployments] - return ServeApplicationSchema(deployments=schemas) - - -def schema_to_serve_application(schema: ServeApplicationSchema) -> List[Deployment]: - return [schema_to_deployment(s) for s in schema.deployments] - - def status_info_to_schema( deployment_name: str, status_info: Union[DeploymentStatusInfo, Dict] ) -> DeploymentStatusSchema: diff --git a/python/ray/serve/pipeline/json_serde.py b/python/ray/serve/pipeline/json_serde.py index b82935d14b9a..e5096bc9afff 100644 --- a/python/ray/serve/pipeline/json_serde.py +++ b/python/ray/serve/pipeline/json_serde.py @@ -21,6 +21,16 @@ from ray.serve.constants import SERVE_HANDLE_JSON_KEY +def convert_to_json_safe_obj(obj: Any, *, err_key: str) -> Any: + # XXX: comment, err msg + return json.loads(json.dumps(obj, cls=DAGNodeEncoder)) + + +def convert_from_json_safe_obj(obj: Any) -> Any: + # XXX: comment, err msg + return json.loads(json.dumps(obj), object_hook=dagnode_from_json) + + class DAGNodeEncoder(json.JSONEncoder): """ Custom JSON serializer for DAGNode type that takes care of RayServeHandle diff --git a/python/ray/serve/utils.py b/python/ray/serve/utils.py index b2b3050663ca..5f18bbe0d7b5 100644 --- a/python/ray/serve/utils.py +++ b/python/ray/serve/utils.py @@ -7,7 +7,7 @@ import random import string import time -from typing import Iterable, List, Tuple, Dict, Any +from typing import Any, Dict, Iterable, List, Tuple import os import traceback from enum import Enum From f9b76657d2d73ee58d05ba2fa0dbaaf5dc85d2ac Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Wed, 16 Mar 2022 13:27:45 -0500 Subject: [PATCH 66/94] fix --- python/ray/serve/scripts.py | 52 +++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 4f831cef3ed1..a0268ab45aca 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -6,7 +6,7 @@ import click import time import sys -from typing import Tuple, List, Dict, Union +from typing import Dict, List, Optional, Tuple import argparse import ray @@ -473,33 +473,39 @@ def delete(address: str, yes: bool): @cli.command( - short_help="Build a Serve config using an Application", + short_help="Build a Serve app into a structured config.", help=( - "Imports the Serve app or DeploymentNode stored at IMPORT_PATH, and " - "writes a config file to the FILE_PATH. The FILE_PATH must have " - 'the ".yaml" extension.' + "Imports the Serve app at IMPORT_PATH and generates a structured " + "config for it that can be used by `serve deploy` or the REST API. " + ), +) +@click.option( + "--output-path", + "-o", + default=None, + type=str, + help=( + "Local path where the output config will be written in YAML format. " + "If not provided, the config will be printed to STDOUT." ), ) @click.argument("import_path") -@click.argument("file_path") -def build(import_path: str, file_path: str): - if not file_path.endswith(".yaml"): - raise ValueError( - f'Got "{file_path}" as FILE_PATH. FILE_PATH must end with ".yaml".' +def build(output_path: Optional[str], import_path: str): + sys.path.insert(0, ".") + + node: DeploymentNode = import_attr(import_path) + if not isinstance(node, DeploymentNode): + raise TypeError( + f"Expected '{import_path}' to be DeploymentNode, but got {type(node)}." ) - app_or_node: Union[Application, DeploymentNode] = import_attr(import_path) + app = serve.build(node) - if isinstance(app_or_node, DeploymentNode): - app = build_app(app_or_node) - elif isinstance(app, Application): - app = app_or_node - else: - raise ValueError( - f'Got IMPORT_PATH "{import_path}", which contains ' - f'"{type(app_or_node)}" object. Expected either an Application or ' - "DeploymentNode." - ) + if output_path is not None: + if not output_path.endswith(".yaml"): + raise ValueError("FILE_PATH must end with '.yaml'.") - with open(file_path, "w+") as f: - app.to_yaml(f) + with open(output_path, "w") as f: + app.to_yaml(f) + else: + print(app.to_yaml(), end="") From bac3ef555b09b4e21e5096ab773eb21b4dbef1a9 Mon Sep 17 00:00:00 2001 From: Ed Oakes Date: Wed, 16 Mar 2022 13:50:42 -0500 Subject: [PATCH 67/94] add --app-dir --- python/ray/serve/scripts.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index a0268ab45aca..c2e463b120a8 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -33,6 +33,12 @@ build_app = build +APP_DIR_HELP_STR = ( + "Local directory to look for the IMPORT_PATH (will be inserted into " + "PYTHONPATH). Defaults to '.', meaning that an object in ./main.py " + "can be imported as 'main.object'. Not relevant if you're importing " + "from an installed module." +) RAY_INIT_ADDRESS_HELP_STR = ( "Address to use for ray.init(). Can also be specified " "using the RAY_ADDRESS environment variable." @@ -479,6 +485,13 @@ def delete(address: str, yes: bool): "config for it that can be used by `serve deploy` or the REST API. " ), ) +@click.option( + "--app-dir", + "-d", + default=".", + type=str, + help=APP_DIR_HELP_STR, +) @click.option( "--output-path", "-o", @@ -490,8 +503,8 @@ def delete(address: str, yes: bool): ), ) @click.argument("import_path") -def build(output_path: Optional[str], import_path: str): - sys.path.insert(0, ".") +def build(app_dir: str, output_path: Optional[str], import_path: str): + sys.path.insert(0, app_dir) node: DeploymentNode = import_attr(import_path) if not isinstance(node, DeploymentNode): From 1dfc066d84b797af6a90d167ed68d8a536b40a57 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 12:16:50 -0700 Subject: [PATCH 68/94] Make test_delete use yaml --- python/ray/serve/tests/test_cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/ray/serve/tests/test_cli.py b/python/ray/serve/tests/test_cli.py index 88420fd62bc5..e308430b159c 100644 --- a/python/ray/serve/tests/test_cli.py +++ b/python/ray/serve/tests/test_cli.py @@ -242,8 +242,8 @@ def test_delete(ray_start_stop): # Deploys a config file and deletes it def get_num_deployments(): - info_response = subprocess.check_output(["serve", "info", "-j"]) - info = json.loads(info_response) + info_response = subprocess.check_output(["serve", "config"]) + info = yaml.safe_load(info_response) return len(info["deployments"]) config_file_name = os.path.join( From e62d87d42ca561144e331b77d30cad5f54235f54 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 12:17:13 -0700 Subject: [PATCH 69/94] Remove test_run_simultaneous --- python/ray/serve/tests/test_cli.py | 43 ------------------------------ 1 file changed, 43 deletions(-) diff --git a/python/ray/serve/tests/test_cli.py b/python/ray/serve/tests/test_cli.py index e308430b159c..d330001c552e 100644 --- a/python/ray/serve/tests/test_cli.py +++ b/python/ray/serve/tests/test_cli.py @@ -372,49 +372,6 @@ def test_run_init_args_kwargs(ray_start_stop): ) -@pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") -def test_run_simultaneous(ray_start_stop): - # Test that two serve run processes can run simultaneously - - p1 = subprocess.Popen( - ["serve", "run", "--address=auto", "ray.serve.tests.test_cli.parrot"] - ) - wait_for_condition( - lambda: ping_endpoint("parrot", params="?sound=squawk") == "squawk", timeout=10 - ) - - p2 = subprocess.Popen( - [ - "serve", - "run", - "--address=auto", - "ray.serve.tests.test_cli.Macaw", - "--", - "green", - "--name=Molly", - "--surname=Malarkey", - ] - ) - wait_for_condition( - lambda: ping_endpoint("parrot", params="?sound=squawk") == "squawk", timeout=10 - ) - wait_for_condition( - lambda: ping_endpoint("Macaw") == "Molly Malarkey is green!", timeout=10 - ) - - # Macaw should still be available after parrot is torn down - p1.send_signal(signal.SIGINT) - p1.wait() - assert "Path '/parrot' not found" in ping_endpoint("parrot") - assert ping_endpoint("Macaw") == "Molly Malarkey is green!" - - # Serve should shut down after all deployments are torn down - p2.send_signal(signal.SIGINT) - p2.wait() - assert ping_endpoint("parrot") == "connection error" - assert ping_endpoint("Macaw") == "connection error" - - @pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") def test_run_runtime_env(ray_start_stop): # Tests serve run with runtime_envs specified From d9617e71b97917c080b8f93fa64ec62042d4cc90 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 12:20:42 -0700 Subject: [PATCH 70/94] Reimplement blocking --- python/ray/serve/scripts.py | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 237b5e3e050f..e29dd8e7c358 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -287,21 +287,22 @@ def run( # Setting the runtime_env here will set defaults for the deployments. ray.init(address=address, namespace="serve", runtime_env=final_runtime_env) - try: - serve.run(app_or_node, host=host, port=port) - cli_logger.success("Deployed successfully!\n") - - while True: - statuses = serve_application_status_to_schema( - get_deployment_statuses() - ).json(indent=4) - cli_logger.info(f"{statuses}") - time.sleep(10) - - except KeyboardInterrupt: - cli_logger.info("Got KeyboardInterrupt, shutting down...") - serve.shutdown() - sys.exit() + serve.run(app_or_node, host=host, port=port) + cli_logger.success("Deployed successfully!\n") + + if blocking: + try: + while True: + statuses = serve_application_status_to_schema( + get_deployment_statuses() + ).json(indent=4) + cli_logger.info(f"{statuses}") + time.sleep(10) + + except KeyboardInterrupt: + cli_logger.info("Got KeyboardInterrupt, shutting down...") + serve.shutdown() + sys.exit() @cli.command( From 8c45b4e5ef15341da222f99a14bc6f21258033cf Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 12:46:00 -0700 Subject: [PATCH 71/94] Fold deploy_group into serve.run --- dashboard/modules/serve/serve_head.py | 3 +- python/ray/serve/api.py | 53 ++++++++-------------- python/ray/serve/tests/test_application.py | 18 ++++---- 3 files changed, 28 insertions(+), 46 deletions(-) diff --git a/dashboard/modules/serve/serve_head.py b/dashboard/modules/serve/serve_head.py index 547ed7a143a6..50e1be5d63a0 100644 --- a/dashboard/modules/serve/serve_head.py +++ b/dashboard/modules/serve/serve_head.py @@ -8,7 +8,6 @@ from ray import serve from ray.serve.api import ( Application, - deploy_group, get_deployment_statuses, serve_application_status_to_schema, ) @@ -53,7 +52,7 @@ async def delete_serve_application(self, req: Request) -> Response: @optional_utils.init_ray_and_catch_exceptions(connect_to_serve=True) async def put_all_deployments(self, req: Request) -> Response: app = Application.from_dict(await req.json()) - deploy_group(app.deployments.values(), blocking=False) + serve.run(app, blocking=False) new_names = set() for deployment in app.deployments.values(): diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index d284b304a4a3..35b144cd409b 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1720,15 +1720,17 @@ def _get_deployments_from_node(node: DeploymentNode) -> List[Deployment]: @PublicAPI(stability="alpha") def run( target: Union[DeploymentNode, Application], + blocking: bool = True, *, host: str = DEFAULT_HTTP_HOST, port: int = DEFAULT_HTTP_PORT, -) -> RayServeHandle: +) -> Optional[RayServeHandle]: """Run a Serve application and return a ServeHandle to the ingress. Either a DeploymentNode or a pre-built application can be passed in. If a DeploymentNode is passed in, all of the deployments it depends on - will be deployed. + will be deployed. If there is an ingress (i.e. only one deployment with a + route prefix), its handle will be returned. """ if isinstance(target, Application): @@ -1742,39 +1744,7 @@ def run( f'"{type(target)}" instead.' ) - if len(deployments) == 0: - return - - # TODO (shrekris-anyscale): validate ingress - client = start(detached=True, http_options={"host": host, "port": port}) - parameter_group = [ - { - "name": deployment._name, - "func_or_class": deployment._func_or_class, - "init_args": deployment.init_args, - "init_kwargs": deployment.init_kwargs, - "ray_actor_options": deployment._ray_actor_options, - "config": deployment._config, - "version": deployment._version, - "prev_version": deployment._prev_version, - "route_prefix": deployment.route_prefix, - "url": deployment.url, - } - for deployment in deployments - ] - - client.deploy_group(parameter_group, _blocking=True) - return deployments[-1].get_handle() - - # TODO (shrekris-anyscale): return handle to ingress deployment - - -def deploy_group(deployments: List[Deployment], *, blocking=True) -> RayServeHandle: - """Atomically deploys a group of deployments.""" - - if len(deployments) == 0: - return parameter_group = [] @@ -1794,7 +1764,20 @@ def deploy_group(deployments: List[Deployment], *, blocking=True) -> RayServeHan parameter_group.append(deployment_parameters) - internal_get_global_client().deploy_group(parameter_group, _blocking=blocking) + client.deploy_group(parameter_group, _blocking=blocking) + + # If only one deployment has a route_prefix, consider that the ingress. + # Otherwise, there is no ingress. + ingress_handle = None + for deployment in deployments: + if deployment.route_prefix is not None: + if ingress_handle is None: + ingress_handle = deployment.get_handle() + else: + ingress_handle = None + break + + return ingress_handle @PublicAPI(stability="alpha") diff --git a/python/ray/serve/tests/test_application.py b/python/ray/serve/tests/test_application.py index d358f3248502..44d8c238865d 100644 --- a/python/ray/serve/tests/test_application.py +++ b/python/ray/serve/tests/test_application.py @@ -8,7 +8,7 @@ import ray from ray import serve -from ray.serve.api import Application, deploy_group +from ray.serve.api import Application from ray._private.test_utils import wait_for_condition @@ -42,7 +42,7 @@ def test_non_deployments(self): Application([self.f, 5, "hello"]) -class TestDeployGroup: +class TestRun: @serve.deployment def f(): return "f reached" @@ -71,7 +71,7 @@ def deploy_and_check_responses( the client to wait until the deployments finish deploying. """ - deploy_group(Application(deployments).deployments.values(), blocking=blocking) + serve.run(Application(deployments), blocking=blocking) def check_all_deployed(): try: @@ -90,7 +90,7 @@ def check_all_deployed(): # If non-blocking, this should pass eventually. wait_for_condition(check_all_deployed) - def test_basic_deploy_group(self, serve_instance): + def test_basic_run(self, serve_instance): """ Atomically deploys a group of deployments, including both functions and classes. Checks whether they deploy correctly. @@ -101,7 +101,7 @@ def test_basic_deploy_group(self, serve_instance): self.deploy_and_check_responses(deployments, responses) - def test_non_blocking_deploy_group(self, serve_instance): + def test_non_blocking_run(self, serve_instance): """Checks Application's deploy() behavior when blocking=False.""" deployments = [self.f, self.g, self.C, self.D] @@ -144,7 +144,7 @@ async def request_echo(self, echo: str): MutualHandles.options(name=deployment_name, init_args=(handle_name,)) ) - deploy_group(Application(deployments).deployments.values(), blocking=True) + serve.run(Application(deployments), blocking=True) for deployment in deployments: assert (ray.get(deployment.get_handle().remote("hello"))) == "hello" @@ -309,7 +309,7 @@ def test_deploy_from_dict(self, serve_instance): compare_specified_options(config_dict, app_dict) - deploy_group(app.deployments.values()) + serve.run(app.from_dict(app_dict)) assert ( requests.get("http://localhost:8000/shallow").text == "Hello shallow world!" @@ -335,7 +335,7 @@ def test_deploy_from_yaml(self, serve_instance): compare_specified_options(app1.to_dict(), app2.to_dict()) # Check that deployment works - deploy_group(app1.deployments.values()) + serve.run(app1) assert ( requests.get("http://localhost:8000/shallow").text == "Hello shallow world!" ) @@ -360,7 +360,7 @@ def test_convert_to_import_path(self, serve_instance): reconstructed_app = Application.from_yaml(app.to_yaml()) - deploy_group(reconstructed_app.deployments.values()) + serve.run(reconstructed_app) assert requests.get("http://localhost:8000/f").text == "got decorated func" assert requests.get("http://localhost:8000/C").text == "got decorated class" From 9b3d0463e64daeffce18e3cf341c5cff5d67ae15 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 14:19:17 -0700 Subject: [PATCH 72/94] Make run pass tests --- python/ray/serve/api.py | 4 +- python/ray/serve/scripts.py | 25 ++-- python/ray/serve/tests/test_cli.py | 180 ++--------------------------- 3 files changed, 30 insertions(+), 179 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 35b144cd409b..fcc1acd61174 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1733,6 +1733,8 @@ def run( route prefix), its handle will be returned. """ + client = start(detached=True, http_options={"host": host, "port": port}) + if isinstance(target, Application): deployments = list(target.deployments.values()) elif isinstance(target, DeploymentNode): @@ -1744,8 +1746,6 @@ def run( f'"{type(target)}" instead.' ) - client = start(detached=True, http_options={"host": host, "port": port}) - parameter_group = [] for deployment in deployments: diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index e29dd8e7c358..dd08ccd0f3f0 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -1,5 +1,4 @@ #!/usr/bin/env python -import click import json import os import pathlib @@ -13,7 +12,6 @@ from ray._private.utils import import_attr from ray.serve.config import DeploymentMode from ray import serve -from ray.serve.api import get_deployment_statuses from ray.serve.constants import ( DEFAULT_CHECKPOINT_PATH, DEFAULT_HTTP_HOST, @@ -23,17 +21,13 @@ from ray.dashboard.modules.dashboard_sdk import parse_runtime_env_args from ray.dashboard.modules.serve.sdk import ServeSubmissionClient from ray.autoscaler._private.cli_logger import cli_logger -from ray._private.utils import import_attr from ray.serve.api import ( Application, DeploymentNode, - build, get_deployment_statuses, serve_application_status_to_schema, ) -build_app = build - APP_DIR_HELP_STR = ( "Local directory to look for the IMPORT_PATH (will be inserted into " "PYTHONPATH). Defaults to '.', meaning that an object in ./main.py " @@ -225,6 +219,13 @@ def deploy(config_file_name: str, address: str): "deployments." ), ) +@click.option( + "--app-dir", + "-d", + default=".", + type=str, + help=APP_DIR_HELP_STR, +) @click.option( "--address", "-a", @@ -262,11 +263,14 @@ def run( runtime_env: str, runtime_env_json: str, working_dir: str, + app_dir: str, address: str, host: str, port: int, blocking: bool, ): + sys.path.insert(0, app_dir) + final_runtime_env = parse_runtime_env_args( runtime_env=runtime_env, runtime_env_json=runtime_env_json, @@ -287,11 +291,11 @@ def run( # Setting the runtime_env here will set defaults for the deployments. ray.init(address=address, namespace="serve", runtime_env=final_runtime_env) - serve.run(app_or_node, host=host, port=port) - cli_logger.success("Deployed successfully!\n") - if blocking: try: + serve.run(app_or_node, host=host, port=port) + cli_logger.success("Deployed successfully!\n") + while True: statuses = serve_application_status_to_schema( get_deployment_statuses() @@ -303,6 +307,9 @@ def run( cli_logger.info("Got KeyboardInterrupt, shutting down...") serve.shutdown() sys.exit() + else: + serve.run(app_or_node, host=host, port=port) + cli_logger.success("Deployed successfully!\n") @cli.command( diff --git a/python/ray/serve/tests/test_cli.py b/python/ray/serve/tests/test_cli.py index d330001c552e..1d3eb7adfb62 100644 --- a/python/ray/serve/tests/test_cli.py +++ b/python/ray/serve/tests/test_cli.py @@ -7,7 +7,6 @@ import signal import pytest import requests -from tempfile import NamedTemporaryFile import ray from ray import serve @@ -259,12 +258,16 @@ def get_num_deployments(): wait_for_condition(lambda: get_num_deployments() == 0, timeout=35) +@serve.deployment def parrot(request): return request.query_params["sound"] +parrot_app = Application([parrot]) + + @pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") -def test_run_basic(ray_start_stop): +def test_run_application(ray_start_stop): # Deploys valid config file and import path via serve run # Deploy via config file @@ -285,7 +288,7 @@ def test_run_basic(ray_start_stop): # Deploy via import path p = subprocess.Popen( - ["serve", "run", "--address=auto", "ray.serve.tests.test_cli.parrot"] + ["serve", "run", "--address=auto", "ray.serve.tests.test_cli.parrot_app"] ) wait_for_condition( lambda: ping_endpoint("parrot", params="?sound=squawk") == "squawk", timeout=10 @@ -296,6 +299,7 @@ def test_run_basic(ray_start_stop): assert ping_endpoint("parrot", params="?sound=squawk") == "connection error" +@serve.deployment class Macaw: def __init__(self, color, name="Mulligan", surname=None): self.color = color @@ -309,8 +313,11 @@ def __call__(self): return f"{self.name} is {self.color}!" +molly_macaw = Macaw.bind("green", name="Molly") + + @pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") -def test_run_init_args_kwargs(ray_start_stop): +def test_run_deployment_node(ray_start_stop): # Tests serve run with specified args and kwargs # Deploy via import path @@ -319,11 +326,7 @@ def test_run_init_args_kwargs(ray_start_stop): "serve", "run", "--address=auto", - "ray.serve.tests.test_cli.Macaw", - "--", - "green", - "--name", - "Molly", + "ray.serve.tests.test_cli.molly_macaw", ] ) wait_for_condition(lambda: ping_endpoint("Macaw") == "Molly is green!", timeout=10) @@ -331,165 +334,6 @@ def test_run_init_args_kwargs(ray_start_stop): p.wait() assert ping_endpoint("Macaw") == "connection error" - # Mix and match keyword notation - p = subprocess.Popen( - [ - "serve", - "run", - "--address=auto", - "ray.serve.tests.test_cli.Macaw", - "--", - "green", - "--name", - "Molly", - "--surname==./u=6y", - ] - ) - wait_for_condition( - lambda: ping_endpoint("Macaw") == "Molly =./u=6y is green!", timeout=10 - ) - p.send_signal(signal.SIGINT) - p.wait() - assert ping_endpoint("Macaw") == "connection error" - - # Args/kwargs with config file - config_file_name = os.path.join( - os.path.dirname(__file__), "test_config_files", "macaw.yaml" - ) - - with pytest.raises(subprocess.CalledProcessError): - subprocess.check_output( - [ - "serve", - "run", - "--address=auto", - config_file_name, - "--", - "green", - "--name", - "Molly", - ] - ) - - -@pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") -def test_run_runtime_env(ray_start_stop): - # Tests serve run with runtime_envs specified - - # Use local working_dir with import path - p = subprocess.Popen( - [ - "serve", - "run", - "--address=auto", - "test_cli.Macaw", - "--working-dir", - os.path.dirname(__file__), - "--", - "green", - "--name=Molly", - ] - ) - wait_for_condition(lambda: ping_endpoint("Macaw") == "Molly is green!", timeout=10) - p.send_signal(signal.SIGINT) - p.wait() - - # Use local working_dir with config file - p = subprocess.Popen( - [ - "serve", - "run", - "--address=auto", - os.path.join( - os.path.dirname(__file__), "test_config_files", "scarlet.yaml" - ), - "--working-dir", - os.path.dirname(__file__), - ] - ) - wait_for_condition( - lambda: ping_endpoint("Scarlet") == "Scarlet is red!", timeout=10 - ) - p.send_signal(signal.SIGINT) - p.wait() - - # Use remote working_dir - p = subprocess.Popen( - [ - "serve", - "run", - "--address=auto", - "test_module.test.one", - "--working-dir", - "https://github.com/shrekris-anyscale/test_module/archive/HEAD.zip", - ] - ) - wait_for_condition(lambda: ping_endpoint("one") == "2", timeout=10) - p.send_signal(signal.SIGINT) - p.wait() - - # Use runtime env - p = subprocess.Popen( - [ - "serve", - "run", - "--address=auto", - os.path.join( - os.path.dirname(__file__), - "test_config_files", - "missing_runtime_env.yaml", - ), - "--runtime-env-json", - ( - '{"py_modules": ["https://github.com/shrekris-anyscale/' - 'test_deploy_group/archive/HEAD.zip"],' - '"working_dir": "http://nonexistentlink-q490123950ni34t"}' - ), - "--working-dir", - "https://github.com/shrekris-anyscale/test_module/archive/HEAD.zip", - ] - ) - wait_for_condition(lambda: ping_endpoint("one") == "2", timeout=10) - p.send_signal(signal.SIGINT) - p.wait() - - -@serve.deployment -def global_f(*args): - return "wonderful world" - - -test_build_app = Application( - [ - global_f.options(name="f1"), - global_f.options(name="f2"), - ] -) - - -@pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") -def test_build(ray_start_stop): - f = NamedTemporaryFile(mode="w", delete=False) - - # Build an app - subprocess.check_output( - ["serve", "build", "ray.serve.tests.test_cli.test_build_app", f.name] - ) - subprocess.check_output(["serve", "deploy", f.name]) - - assert requests.get("http://localhost:8000/f1").text == "wonderful world" - assert requests.get("http://localhost:8000/f2").text == "wonderful world" - - # Build a deployment - subprocess.check_output( - ["serve", "build", "ray.serve.tests.test_cli.global_f", f.name] - ) - subprocess.check_output(["serve", "deploy", f.name]) - - assert requests.get("http://localhost:8000/global_f").text == "wonderful world" - - os.unlink(f.name) - if __name__ == "__main__": sys.exit(pytest.main(["-v", "-s", __file__])) From dfbd4073351f471d10829d72fab7ab2518a5add0 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 14:53:33 -0700 Subject: [PATCH 73/94] Remove application.py --- python/ray/serve/application.py | 287 -------------------------------- 1 file changed, 287 deletions(-) delete mode 100644 python/ray/serve/application.py diff --git a/python/ray/serve/application.py b/python/ray/serve/application.py deleted file mode 100644 index ebcbb6e22efb..000000000000 --- a/python/ray/serve/application.py +++ /dev/null @@ -1,287 +0,0 @@ -from typing import List, TextIO, Optional, Union, Dict -import yaml -import time -import sys - -from ray import serve -from ray.serve.api import ( - Deployment, - internal_get_global_client, -) -from ray.serve.schema import ( - ServeApplicationSchema, - serve_application_to_schema, - schema_to_serve_application, - serve_application_status_to_schema, -) -from ray.serve.utils import logger -from ray.serve.api import get_deployment_statuses -from ray.autoscaler._private.cli_logger import _CliLogger -from logging import Logger - - -class Application: - """Used to deploy, update, and monitor groups of Serve deployments. - - Args: - deployments (List[Deployment]): The Serve Application is - initialized to contain this group of deployments. - """ - - def __init__(self, deployments: List[Deployment] = None): - deployments = deployments or [] - - self._deployments: Dict[str, Deployment] = dict() - for d in deployments: - self.add_deployment(d) - - def add_deployment(self, deployment: Deployment): - """Adds the deployment to this Serve application. - - Validates that a deployment with the same name doesn't already exist. - To overwrite an existing deployment, use index notation. For example: - - app[deployment_name] = deployment - - Args: - deployment (Deployment): deployment to add to this Application. - - Raises: - ValueError: If a deployment with deployment.name already exists in - this Application. - """ - - if not isinstance(deployment, Deployment): - raise TypeError(f"Got {type(deployment)}. Expected deployment.") - elif deployment.name in self._deployments: - raise ValueError( - f'Deployment with name "{deployment.name}" already ' - "exists in this application. To overwrite this " - "deployment, use attribute or index notation. " - "For example:\n\napp.deployment_name = " - "new_deployment" - ) - - self._deployments[deployment.name] = deployment - - def deploy(self, blocking: bool = True): - """Atomically deploys the Application's deployments to the Ray cluster. - - The Application's deployments can carry handles to one another. - - Args: - blocking (bool): If True, this function only returns after the - deployment is finished. If False, this function returns - immediately after requesting the deployment. - """ - - if len(self._deployments) == 0: - return - - parameter_group = [] - - for deployment in self._deployments.values(): - if not isinstance(deployment, Deployment): - raise TypeError( - f"deploy_group only accepts Deployments, but got unexpected " - f"type {type(deployment)}." - ) - - deployment_parameters = { - "name": deployment._name, - "func_or_class": deployment._func_or_class, - "init_args": deployment.init_args, - "init_kwargs": deployment.init_kwargs, - "ray_actor_options": deployment._ray_actor_options, - "config": deployment._config, - "version": deployment._version, - "prev_version": deployment._prev_version, - "route_prefix": deployment.route_prefix, - "url": deployment.url, - } - - parameter_group.append(deployment_parameters) - - return internal_get_global_client().deploy_group( - parameter_group, _blocking=blocking - ) - - def run(self, logger: Union[Logger, _CliLogger] = logger): - """Deploys all deployments in this Application and logs status. - - This function keeps looping and printing status, so it must be manually - killed (e.g. using ctrl-C). When it recieves a kill signal, it tears - down all deployments that it deployed. If there are no deployments - left, it also shuts down the rest of Serve, including the controller. - This is meant to help interactive development. - - Args: - logger: Any Python object that implements the standard logger - interface. - """ - - try: - serve.start(detached=True) - self.deploy() - - logger.info("\nDeployed successfully!\n") - - while True: - statuses = serve_application_status_to_schema( - get_deployment_statuses() - ).json(indent=4) - logger.info(f"{statuses}") - time.sleep(10) - - except KeyboardInterrupt: - logger.info("Got SIGINT (KeyboardInterrupt). Removing deployments.") - deployment_names = [d.name for d in self._deployments.values()] - internal_get_global_client().delete_deployments( - deployment_names, blocking=True - ) - if len(serve.list_deployments()) == 0: - logger.info("No deployments left. Shutting down Serve.") - serve.shutdown() - sys.exit() - - def to_dict(self) -> Dict: - """Returns this Application's deployments as a dictionary. - - This dictionary adheres to the Serve REST API schema. It can be deployed - via the Serve REST API. - - If any deployment's func_or_class is a function or class (and not an - import path), it overwrites it with that function or class's import path. - - Returns: - Dict: The Application's deployments formatted in a dictionary. - """ - - return serve_application_to_schema(self._deployments.values()).dict() - - @classmethod - def from_dict(cls, d: Dict) -> "Application": - """Converts a dictionary of deployment data to an Application. - - Takes in a dictionary matching the Serve REST API schema and converts - it to an Application containing those deployments. - - Args: - d (Dict): A dictionary containing the deployments' data that matches - the Serve REST API schema. - - Returns: - Application: a new Application object containing the deployments. - """ - - schema = ServeApplicationSchema.parse_obj(d) - return cls(schema_to_serve_application(schema)) - - def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: - """Returns this Application's deployments as a YAML string. - - Optionally writes the YAML string to a file as well. To write to a - file, use this pattern: - - with open("file_name.txt", "w") as f: - app.to_yaml(f=f) - - This file is formatted as a Serve YAML config file. It can be deployed - via the Serve CLI. - - If any deployment's func_or_class is a function or class (and not an - import path), it overwrites it with that function or class's import path. - - Args: - f (Optional[TextIO]): A pointer to the file where the YAML should - be written. - - Returns: - Optional[String]: The deployments' YAML string. The output is from - yaml.safe_dump(). Returned only if no file pointer is passed in. - """ - - deployment_dict = serve_application_to_schema(self._deployments.values()).dict() - - if f: - yaml.safe_dump(deployment_dict, f, default_flow_style=False) - return yaml.safe_dump(deployment_dict, default_flow_style=False) - - @classmethod - def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": - """Converts YAML data to deployments for an Application. - - Takes in a string or a file pointer to a file containing deployment - definitions in YAML. These definitions are converted to a new - Application object containing the deployments. - - To read from a file, use the following pattern: - - with open("file_name.txt", "w") as f: - app = app.from_yaml(str_or_file) - - Args: - str_or_file (Union[String, TextIO]): Either a string containing - YAML deployment definitions or a pointer to a file containing - YAML deployment definitions. The YAML format must adhere to the - ServeApplicationSchema JSON Schema defined in - ray.serve.schema. This function works with - Serve YAML config files. - - Returns: - Application: a new Application object containing the deployments. - """ - - deployments_json = yaml.safe_load(str_or_file) - schema = ServeApplicationSchema.parse_obj(deployments_json) - return cls(schema_to_serve_application(schema)) - - def __getitem__(self, key: str): - """Fetch a deployment by name using dict syntax: app["name"] - - Raises: - KeyError: if the name is not in this Application. - """ - - if key in self._deployments: - return self._deployments[key] - else: - raise KeyError(f'Serve application does not contain a "{key}" deployment.') - - def __setitem__(self, key: str, value: Deployment): - """Set a deployment by name with dict syntax: app[name]=new_deployment - - Use this to overwrite existing deployments. - - Args: - key (String): name - value (Deployment): the deployment that the name maps to - - Raises: - TypeError: if the key is not a String or the value is not a deployment. - """ - - if not isinstance(key, str): - raise TypeError(f"key should be a string, but got object of type {key}.") - elif not isinstance(value, Deployment): - raise TypeError(f"Got {type(Deployment)} for value. Expected deployment.") - - self._deployments[key] = value - - def __iter__(self): - """Iterator over Application's deployments. - - Enables "for deployment in Application" pattern. - """ - - return iter(self._deployments.values()) - - def __len__(self): - """Number of deployments in this Application.""" - - return len(self._deployments) - - def __contains__(self, key: str): - """Checks if the key exists in self._deployments.""" - - return key in self._deployments From 1842e0626989a755a38d9c0d0411e0d8924a0f56 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 14:56:13 -0700 Subject: [PATCH 74/94] Use delete_deployments --- dashboard/modules/serve/serve_head.py | 4 ++-- python/ray/serve/api.py | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/dashboard/modules/serve/serve_head.py b/dashboard/modules/serve/serve_head.py index 50e1be5d63a0..e536ba0806b6 100644 --- a/dashboard/modules/serve/serve_head.py +++ b/dashboard/modules/serve/serve_head.py @@ -9,6 +9,7 @@ from ray.serve.api import ( Application, get_deployment_statuses, + internal_get_global_client, serve_application_status_to_schema, ) @@ -61,8 +62,7 @@ async def put_all_deployments(self, req: Request) -> Response: all_deployments = serve.list_deployments() all_names = set(all_deployments.keys()) names_to_delete = all_names.difference(new_names) - for name in names_to_delete: - all_deployments[name].delete() + internal_get_global_client().delete_deployments(names_to_delete) return Response() diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 8679a574cf4d..3a1a5f270f72 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -7,11 +7,8 @@ import random import re import time -<<<<<<< HEAD import yaml -======= import json ->>>>>>> 8707eb628843a6ab02e6452f487bb7f7df8a5e8f from dataclasses import dataclass from functools import wraps from typing import ( From 3e5052dc9cb9e07e8dbdfd697ea80a9a13c7e67b Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 17:27:53 -0700 Subject: [PATCH 75/94] Add test for runtime environments --- python/ray/serve/scripts.py | 3 +- python/ray/serve/tests/test_cli.py | 76 ++++++++++++++++++++++-------- 2 files changed, 58 insertions(+), 21 deletions(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index dd08ccd0f3f0..540704a6f712 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -185,6 +185,7 @@ def deploy(config_file_name: str, address: str): short_help="Run a Serve app.", help=( "Runs the Serve app from the specified import path or YAML config.\n" + "Any import path must lead to an Application or DeploymentNode object. " "By default, this will block and periodically log status. If you " "Ctrl-C the command, it will tear down the app." ), @@ -285,7 +286,7 @@ def run( app_or_node = Application.from_yaml(config_file) else: import_path = config_or_import_path - cli_logger.print(f"Loading app from import path: '{import_path}'.") + cli_logger.print(f"Loading app or node from import path: '{import_path}'.") app_or_node = import_attr(import_path) # Setting the runtime_env here will set defaults for the deployments. diff --git a/python/ray/serve/tests/test_cli.py b/python/ray/serve/tests/test_cli.py index 1d3eb7adfb62..b7564aca1830 100644 --- a/python/ray/serve/tests/test_cli.py +++ b/python/ray/serve/tests/test_cli.py @@ -1,7 +1,6 @@ import yaml import json import os -from pathlib import Path import subprocess import sys import signal @@ -45,25 +44,6 @@ def test_start_shutdown_in_namespace(ray_start_stop): subprocess.check_output(["serve", "shutdown", "-n", "test"]) -class A: - def __init__(self, value, increment=1): - self.value = value - self.increment = increment - self.decrement = 0 - self.multiplier = int(os.environ["SERVE_TEST_MULTIPLIER"]) - - p = Path("hello") - assert p.exists() - with open(p) as f: - assert f.read() == "world" - - def reconfigure(self, config): - self.decrement = config["decrement"] - - def __call__(self, inp): - return (self.value + self.increment - self.decrement) * self.multiplier - - @pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") def test_deploy(ray_start_stop): # Deploys some valid config files and checks that the deployments work @@ -335,5 +315,61 @@ def test_run_deployment_node(ray_start_stop): assert ping_endpoint("Macaw") == "connection error" +@serve.deployment +class MetalDetector: + def __call__(self, *args): + return os.environ.get("buried_item", "no dice") + + +metal_detector_node = MetalDetector.bind() + + +@pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.") +def test_run_runtime_env(ray_start_stop): + # Test serve run with runtime_env passed in + + # With import path + p = subprocess.Popen( + [ + "serve", + "run", + "--address=auto", + "ray.serve.tests.test_cli.metal_detector_node", + "--runtime-env-json", + ('{"env_vars": {"buried_item": "lucky coin"} }'), + ] + ) + wait_for_condition( + lambda: ping_endpoint("MetalDetector") == "lucky coin", timeout=10 + ) + p.send_signal(signal.SIGINT) + p.wait() + + # With config + p = subprocess.Popen( + [ + "serve", + "run", + "--address=auto", + os.path.join( + os.path.dirname(__file__), + "test_config_files", + "missing_runtime_env.yaml", + ), + "--runtime-env-json", + ( + '{"py_modules": ["https://github.com/shrekris-anyscale/' + 'test_deploy_group/archive/HEAD.zip"],' + '"working_dir": "http://nonexistentlink-q490123950ni34t"}' + ), + "--working-dir", + "https://github.com/shrekris-anyscale/test_module/archive/HEAD.zip", + ] + ) + wait_for_condition(lambda: ping_endpoint("one") == "2", timeout=10) + p.send_signal(signal.SIGINT) + p.wait() + + if __name__ == "__main__": sys.exit(pytest.main(["-v", "-s", __file__])) From 17fb3c4a27d905914fe6f51cab39275b027b8329 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 18:30:43 -0700 Subject: [PATCH 76/94] Test ingress handling in serve.run() --- python/ray/serve/api.py | 37 +++++++++++++++------- python/ray/serve/tests/test_api.py | 50 +++++++++++++++++++++++++++--- 2 files changed, 71 insertions(+), 16 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 3a1a5f270f72..132f7e8811b3 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1735,6 +1735,25 @@ def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": """ return cls.from_dict(yaml.safe_load(str_or_file)) + def get_ingress(self) -> Optional[Deployment]: + """Gets the app's ingress, if one exists. + + The ingress is the single deployment with a non-None route prefix. If more + or less than one deployment has a route prefix, no single ingress exists, + so returns None. + """ + + ingress = None + + for deployment in self._deployments.values(): + if deployment.route_prefix is not None: + if ingress is None: + ingress = deployment + else: + return None + + return ingress + @PublicAPI(stability="alpha") def run( @@ -1768,8 +1787,10 @@ def run( if isinstance(target, Application): deployments = list(target.deployments.values()) + ingress = target.get_ingress() elif isinstance(target, DeploymentNode): deployments = pipeline_build(target) + ingress = deployments[-1] else: raise TypeError( "Expected a DeploymentNode or " @@ -1777,6 +1798,8 @@ def run( f'"{type(target)}" instead.' ) + print(deployments) + parameter_group = [] for deployment in deployments: @@ -1797,18 +1820,8 @@ def run( client.deploy_group(parameter_group, _blocking=blocking) - # If only one deployment has a route_prefix, consider that the ingress. - # Otherwise, there is no ingress. - ingress_handle = None - for deployment in deployments: - if deployment.route_prefix is not None: - if ingress_handle is None: - ingress_handle = deployment.get_handle() - else: - ingress_handle = None - break - - return ingress_handle + if ingress is not None: + return ingress.get_handle() @PublicAPI(stability="alpha") diff --git a/python/ray/serve/tests/test_api.py b/python/ray/serve/tests/test_api.py index e8d5736dc8e6..2f48ff523df6 100644 --- a/python/ray/serve/tests/test_api.py +++ b/python/ray/serve/tests/test_api.py @@ -8,7 +8,7 @@ import ray from ray import serve from ray._private.test_utils import SignalActor, wait_for_condition -from ray.serve.api import internal_get_global_client +from ray.serve.api import Application def test_e2e(serve_instance): @@ -269,9 +269,7 @@ def g(*args): # Check idempotence for _ in range(2): - internal_get_global_client().delete_deployments( - ["f", "g"], blocking=blocking - ) + serve_instance.delete_deployments(["f", "g"], blocking=blocking) wait_for_condition( lambda: requests.get("http://127.0.0.1:8000/f").status_code == 404, @@ -336,6 +334,50 @@ def __del__(self): B.delete() +def test_run_get_ingress_app(serve_instance): + """Check that serve.run() with an app returns the ingress.""" + + @serve.deployment(route_prefix=None) + def f(): + return "got f" + + @serve.deployment(route_prefix="/g") + def g(): + return "got g" + + app = Application([f, g]) + ingress_handle = serve.run(app) + + assert ray.get(ingress_handle.remote()) == "got g" + serve_instance.delete_deployments(["f", "g"]) + + no_ingress_app = Application([f.options(route_prefix="/f"), g]) + ingress_handle = serve.run(no_ingress_app) + assert ingress_handle is None + + +def test_run_get_ingress_node(serve_instance): + """Check that serve.run() with a node returns the ingress.""" + + @serve.deployment + class Driver: + def __init__(self, dag): + self.dag = dag + + async def __call__(self, *args): + return await self.dag.remote() + + @serve.deployment + class f: + def __call__(self, *args): + return "got f" + + dag = Driver.bind(f.bind()) + ingress_handle = serve.run(dag) + + assert ray.get(ingress_handle.remote()) == "got f" + + if __name__ == "__main__": import sys From 287c2d77c991335ad723029fa185a4562afe54f5 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 20:54:05 -0700 Subject: [PATCH 77/94] Remove json serialization --- python/ray/serve/api.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 132f7e8811b3..22abc3e22cc1 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1854,7 +1854,6 @@ def deployment_to_schema(d: Deployment) -> DeploymentSchema: init_args and init_kwargs must also be JSON-serializable or this call will fail. """ - from ray.serve.pipeline.json_serde import convert_to_json_safe_obj if d.ray_actor_options is not None: ray_actor_options_schema = RayActorOptionsSchema.parse_obj(d.ray_actor_options) @@ -1864,12 +1863,12 @@ def deployment_to_schema(d: Deployment) -> DeploymentSchema: return DeploymentSchema( name=d.name, import_path=get_deployment_import_path(d), - init_args=convert_to_json_safe_obj(d.init_args, err_key="init_args"), - init_kwargs=convert_to_json_safe_obj(d.init_kwargs, err_key="init_kwargs"), + init_args=d.init_args, + init_kwargs=d.init_kwargs, num_replicas=d.num_replicas, route_prefix=d.route_prefix, max_concurrent_queries=d.max_concurrent_queries, - user_config=convert_to_json_safe_obj(d.user_config, err_key="user_config"), + user_config=d.user_config, autoscaling_config=d._config.autoscaling_config, graceful_shutdown_wait_loop_s=d._config.graceful_shutdown_wait_loop_s, graceful_shutdown_timeout_s=d._config.graceful_shutdown_timeout_s, From 82a5610553eb40e111bb262aa8725949c6211bfa Mon Sep 17 00:00:00 2001 From: shrekris-anyscale <92341594+shrekris-anyscale@users.noreply.github.com> Date: Wed, 16 Mar 2022 20:57:53 -0700 Subject: [PATCH 78/94] Update logging message Co-authored-by: Edward Oakes --- python/ray/serve/scripts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 540704a6f712..217504cca7eb 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -286,7 +286,7 @@ def run( app_or_node = Application.from_yaml(config_file) else: import_path = config_or_import_path - cli_logger.print(f"Loading app or node from import path: '{import_path}'.") + cli_logger.print(f"Loading app from import path: '{import_path}'.") app_or_node = import_attr(import_path) # Setting the runtime_env here will set defaults for the deployments. From 02cf933a9b4735675bcc0d2f2702046397be8314 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 20:59:08 -0700 Subject: [PATCH 79/94] Remove drivers.py --- python/ray/serve/drivers.py | 21 --------------------- 1 file changed, 21 deletions(-) delete mode 100644 python/ray/serve/drivers.py diff --git a/python/ray/serve/drivers.py b/python/ray/serve/drivers.py deleted file mode 100644 index c8c8ff1abf2e..000000000000 --- a/python/ray/serve/drivers.py +++ /dev/null @@ -1,21 +0,0 @@ -from typing import Callable, Optional, Union - -import starlette - -from ray import serve -from ray.serve.api import DAGHandle - - -@serve.deployment -class PipelineDriverDeployment: - def __init__( - self, - dag_handle: DAGHandle, - *, - input_schema: Optional[Union[str, Callable]] = None, - ): - raise NotImplementedError() - - async def __call__(self, request: starlette.requests.Request): - """Parse input schema and pass the result to the DAG handle.""" - raise NotImplementedError() From 4dd55f08f4fb3081661829514b0308e7c3e0f56f Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 21:02:27 -0700 Subject: [PATCH 80/94] Remove implementation detail in docstring for serve.run() --- python/ray/serve/api.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 22abc3e22cc1..beb4a19e1504 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1768,8 +1768,7 @@ def run( Either a DeploymentNode or a pre-built application can be passed in. If a DeploymentNode is passed in, all of the deployments it depends on - will be deployed. If there is an ingress (i.e. only one deployment with a - route prefix), its handle will be returned. + will be deployed. If there is an ingress, its handle will be returned. Args: target: User built serve Application or DeploymentNode that acts as From 70893655e3d1a08d60fe7a5b820602661b4a683c Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Wed, 16 Mar 2022 21:03:26 -0700 Subject: [PATCH 81/94] Fix typo --- python/ray/serve/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index beb4a19e1504..7cd90bd2e439 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1777,7 +1777,7 @@ def run( Returns: RayServeHandle: A regular ray serve handle that can be called by user - to exeucte the serve DAG. + to execute the serve DAG. """ # TODO (jiaodong): Resolve circular reference in pipeline codebase and serve from ray.serve.pipeline.api import build as pipeline_build From 867db5fa9defb62a733814ad4456f89c476ebf80 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 17 Mar 2022 10:15:08 -0700 Subject: [PATCH 82/94] Restore schema helpers --- python/ray/serve/api.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 7cd90bd2e439..ee1ee45677e8 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1902,6 +1902,17 @@ def schema_to_deployment(s: DeploymentSchema) -> Deployment: )(s.import_path) +def serve_application_to_schema( + deployments: List[Deployment], +) -> ServeApplicationSchema: + schemas = [deployment_to_schema(d) for d in deployments] + return ServeApplicationSchema(deployments=schemas) + + +def schema_to_serve_application(schema: ServeApplicationSchema) -> List[Deployment]: + return [schema_to_deployment(s) for s in schema.deployments] + + def status_info_to_schema( deployment_name: str, status_info: Union[DeploymentStatusInfo, Dict] ) -> DeploymentStatusSchema: From 9017cffac492d89f44fed616d134eac07a36ebe6 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 17 Mar 2022 10:44:21 -0700 Subject: [PATCH 83/94] Remove print statement --- python/ray/serve/api.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index ee1ee45677e8..7b4918a105c1 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1797,8 +1797,6 @@ def run( f'"{type(target)}" instead.' ) - print(deployments) - parameter_group = [] for deployment in deployments: From d606997523b0f4119e5001779787e5d65bc9d224 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 17 Mar 2022 12:50:14 -0700 Subject: [PATCH 84/94] Remove incomplete build implementation --- python/ray/serve/scripts.py | 48 ------------------------------------- 1 file changed, 48 deletions(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 217504cca7eb..379afaea1bea 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -6,7 +6,6 @@ import time import sys import yaml -from typing import Optional import ray from ray._private.utils import import_attr @@ -23,7 +22,6 @@ from ray.autoscaler._private.cli_logger import cli_logger from ray.serve.api import ( Application, - DeploymentNode, get_deployment_statuses, serve_application_status_to_schema, ) @@ -383,49 +381,3 @@ def delete(address: str, yes: bool): cli_logger.newline() cli_logger.success("\nSent delete request successfully!\n") cli_logger.newline() - - -@cli.command( - short_help="Build a Serve app into a structured config.", - help=( - "Imports the Serve app at IMPORT_PATH and generates a structured " - "config for it that can be used by `serve deploy` or the REST API. " - ), -) -@click.option( - "--app-dir", - "-d", - default=".", - type=str, - help=APP_DIR_HELP_STR, -) -@click.option( - "--output-path", - "-o", - default=None, - type=str, - help=( - "Local path where the output config will be written in YAML format. " - "If not provided, the config will be printed to STDOUT." - ), -) -@click.argument("import_path") -def build(app_dir: str, output_path: Optional[str], import_path: str): - sys.path.insert(0, app_dir) - - node: DeploymentNode = import_attr(import_path) - if not isinstance(node, DeploymentNode): - raise TypeError( - f"Expected '{import_path}' to be DeploymentNode, but got {type(node)}." - ) - - app = serve.build(node) - - if output_path is not None: - if not output_path.endswith(".yaml"): - raise ValueError("FILE_PATH must end with '.yaml'.") - - with open(output_path, "w") as f: - app.to_yaml(f) - else: - print(app.to_yaml(), end="") From 56af91479a286285b0938a07f7e4ce2fae44b9d1 Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 17 Mar 2022 12:53:15 -0700 Subject: [PATCH 85/94] Make ingress a property of Application --- python/ray/serve/api.py | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 7b4918a105c1..e17039937072 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1656,6 +1656,26 @@ def __init__(self, deployments: List[Deployment]): def deployments(self) -> ImmutableDeploymentDict: return self._deployments + @property + def ingress(self) -> Optional[Deployment]: + """Gets the app's ingress, if one exists. + + The ingress is the single deployment with a non-None route prefix. If more + or less than one deployment has a route prefix, no single ingress exists, + so returns None. + """ + + ingress = None + + for deployment in self._deployments.values(): + if deployment.route_prefix is not None: + if ingress is None: + ingress = deployment + else: + return None + + return ingress + def to_dict(self) -> Dict: """Returns this Application's deployments as a dictionary. @@ -1735,25 +1755,6 @@ def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": """ return cls.from_dict(yaml.safe_load(str_or_file)) - def get_ingress(self) -> Optional[Deployment]: - """Gets the app's ingress, if one exists. - - The ingress is the single deployment with a non-None route prefix. If more - or less than one deployment has a route prefix, no single ingress exists, - so returns None. - """ - - ingress = None - - for deployment in self._deployments.values(): - if deployment.route_prefix is not None: - if ingress is None: - ingress = deployment - else: - return None - - return ingress - @PublicAPI(stability="alpha") def run( @@ -1786,7 +1787,7 @@ def run( if isinstance(target, Application): deployments = list(target.deployments.values()) - ingress = target.get_ingress() + ingress = target.ingress elif isinstance(target, DeploymentNode): deployments = pipeline_build(target) ingress = deployments[-1] From 6dae11623eadbfa18330b0450e724796398f1a4c Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 17 Mar 2022 15:13:36 -0700 Subject: [PATCH 86/94] Change if to elif --- python/ray/serve/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index c5a52c9b5ea1..bca4e3bd02f1 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1803,7 +1803,7 @@ def run( deployments = list(target.deployments.values()) ingress = target.ingress # Each DAG should always provide a valid Driver DeploymentNode - if isinstance(target, DeploymentNode): + elif isinstance(target, DeploymentNode): deployments = pipeline_build(target) ingress = deployments[-1] # Special case where user is doing single function serve.run(func.bind()) From de5cde93765e538ed08cbbbff7580e88887797dd Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 17 Mar 2022 16:44:21 -0700 Subject: [PATCH 87/94] Get ingress in DeploymentFunctionNode case --- python/ray/serve/api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index bca4e3bd02f1..d46ffa368824 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1809,6 +1809,7 @@ def run( # Special case where user is doing single function serve.run(func.bind()) elif isinstance(target, DeploymentFunctionNode): deployments = pipeline_build(target) + ingress = deployments[-1] if len(deployments) != 1: raise ValueError( "We only support single function node in serve.run, ex: " From 186ebf783f69059fa6946ed715f3ec492904047d Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 17 Mar 2022 16:48:33 -0700 Subject: [PATCH 88/94] Remove repetitive serve.start() call --- python/ray/serve/scripts.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 379afaea1bea..1e9a0ab50c60 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -290,11 +290,11 @@ def run( # Setting the runtime_env here will set defaults for the deployments. ray.init(address=address, namespace="serve", runtime_env=final_runtime_env) - if blocking: - try: - serve.run(app_or_node, host=host, port=port) - cli_logger.success("Deployed successfully!\n") + try: + serve.run(app_or_node, host=host, port=port) + cli_logger.success("Deployed successfully!\n") + if blocking: while True: statuses = serve_application_status_to_schema( get_deployment_statuses() @@ -302,13 +302,10 @@ def run( cli_logger.info(f"{statuses}") time.sleep(10) - except KeyboardInterrupt: - cli_logger.info("Got KeyboardInterrupt, shutting down...") - serve.shutdown() - sys.exit() - else: - serve.run(app_or_node, host=host, port=port) - cli_logger.success("Deployed successfully!\n") + except KeyboardInterrupt: + cli_logger.info("Got KeyboardInterrupt, shutting down...") + serve.shutdown() + sys.exit() @cli.command( From cb0dab5983905d9166bfda1b98ff387d1f3a970b Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 17 Mar 2022 16:53:47 -0700 Subject: [PATCH 89/94] Make blocking a private argument for serve.run() --- dashboard/modules/serve/serve_head.py | 2 +- python/ray/serve/api.py | 4 ++-- python/ray/serve/tests/test_application.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dashboard/modules/serve/serve_head.py b/dashboard/modules/serve/serve_head.py index e536ba0806b6..157c4d1eb95e 100644 --- a/dashboard/modules/serve/serve_head.py +++ b/dashboard/modules/serve/serve_head.py @@ -53,7 +53,7 @@ async def delete_serve_application(self, req: Request) -> Response: @optional_utils.init_ray_and_catch_exceptions(connect_to_serve=True) async def put_all_deployments(self, req: Request) -> Response: app = Application.from_dict(await req.json()) - serve.run(app, blocking=False) + serve.run(app, _blocking=False) new_names = set() for deployment in app.deployments.values(): diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index d46ffa368824..23965a95f0b4 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1773,7 +1773,7 @@ def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": @PublicAPI(stability="alpha") def run( target: Union[DeploymentNode, Application], - blocking: bool = True, + _blocking: bool = True, *, host: str = DEFAULT_HTTP_HOST, port: int = DEFAULT_HTTP_PORT, @@ -1849,7 +1849,7 @@ def run( parameter_group.append(deployment_parameters) - client.deploy_group(parameter_group, _blocking=blocking) + client.deploy_group(parameter_group, _blocking=_blocking) if ingress is not None: return ingress.get_handle() diff --git a/python/ray/serve/tests/test_application.py b/python/ray/serve/tests/test_application.py index 44d8c238865d..c9ee97c9ce0b 100644 --- a/python/ray/serve/tests/test_application.py +++ b/python/ray/serve/tests/test_application.py @@ -71,7 +71,7 @@ def deploy_and_check_responses( the client to wait until the deployments finish deploying. """ - serve.run(Application(deployments), blocking=blocking) + serve.run(Application(deployments), _blocking=blocking) def check_all_deployed(): try: @@ -144,7 +144,7 @@ async def request_echo(self, echo: str): MutualHandles.options(name=deployment_name, init_args=(handle_name,)) ) - serve.run(Application(deployments), blocking=True) + serve.run(Application(deployments), _blocking=True) for deployment in deployments: assert (ray.get(deployment.get_handle().remote("hello"))) == "hello" From 17eb0fad2729a823b211c5f39296414dc859c0db Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 17 Mar 2022 16:56:58 -0700 Subject: [PATCH 90/94] Add descriptions for host and port --- python/ray/serve/api.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 23965a95f0b4..20bfd1d63ff1 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1789,6 +1789,8 @@ def run( target: User built serve Application or DeploymentNode that acts as the root node of DAG. By default DeploymentNode is the Driver deployment unless user provided customized one. + host (str): The host passed into serve.start(). + port (int): The port passed into serve.start(). Returns: RayServeHandle: A regular ray serve handle that can be called by user From 8016b0adb4e279fc614c4e2c8845f36427d6a6fb Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 17 Mar 2022 16:58:58 -0700 Subject: [PATCH 91/94] Update description for target in run --- python/ray/serve/api.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 20bfd1d63ff1..c98e05d7c8b4 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1786,9 +1786,10 @@ def run( will be deployed. If there is an ingress, its handle will be returned. Args: - target: User built serve Application or DeploymentNode that acts as - the root node of DAG. By default DeploymentNode is the Driver - deployment unless user provided customized one. + target (Union[DeploymentNode, Application]): A user-built Serve + Application or a DeploymentNode that acts as the root node of DAG. + By default DeploymentNode is the Driver deployment unless user + provides a customized one. host (str): The host passed into serve.start(). port (int): The port passed into serve.start(). From 7b45ec3feeacd03844e644a908fb4d8cae2dfe2d Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 17 Mar 2022 17:05:03 -0700 Subject: [PATCH 92/94] Update type hints for serve.run() --- python/ray/serve/api.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index c98e05d7c8b4..a067fe9470a7 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1772,7 +1772,7 @@ def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": @PublicAPI(stability="alpha") def run( - target: Union[DeploymentNode, Application], + target: Union[DeploymentNode, DeploymentFunctionNode, Application], _blocking: bool = True, *, host: str = DEFAULT_HTTP_HOST, @@ -1781,15 +1781,15 @@ def run( ) -> RayServeHandle: """Run a Serve application and return a ServeHandle to the ingress. - Either a DeploymentNode or a pre-built application can be passed in. - If a DeploymentNode is passed in, all of the deployments it depends on - will be deployed. If there is an ingress, its handle will be returned. + Either a DeploymentNode, DeploymentFunctionNode, or a pre-built application + can be passed in. If a node is passed in, all of the deployments it depends + on will be deployed. If there is an ingress, its handle will be returned. Args: - target (Union[DeploymentNode, Application]): A user-built Serve - Application or a DeploymentNode that acts as the root node of DAG. - By default DeploymentNode is the Driver deployment unless user - provides a customized one. + target (Union[DeploymentNode, DeploymentFunctionNode, Application]): + A user-built Serve Application or a DeploymentNode that acts as the + root node of DAG. By default DeploymentNode is the Driver + deployment unless user provides a customized one. host (str): The host passed into serve.start(). port (int): The port passed into serve.start(). @@ -1829,7 +1829,7 @@ def run( ) else: raise TypeError( - "Expected a DeploymentNode or " + "Expected a DeploymentNode, DeploymentFunctionNode, or " "Application as target. Got unexpected type " f'"{type(target)}" instead.' ) From 08c69ba237f5e9fb8709807edc353307dd3a1daf Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 17 Mar 2022 19:02:08 -0700 Subject: [PATCH 93/94] Don't sort keys when converting to YAML in to_yaml() --- python/ray/serve/api.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/ray/serve/api.py b/python/ray/serve/api.py index 2c857e814f2b..0f900ee6d55a 100644 --- a/python/ray/serve/api.py +++ b/python/ray/serve/api.py @@ -1799,7 +1799,9 @@ def to_yaml(self, f: Optional[TextIO] = None) -> Optional[str]: Optional[String]: The deployments' YAML string. The output is from yaml.safe_dump(). Returned only if no file pointer is passed in. """ - return yaml.safe_dump(self.to_dict(), stream=f, default_flow_style=False) + return yaml.safe_dump( + self.to_dict(), stream=f, default_flow_style=False, sort_keys=False + ) @classmethod def from_yaml(cls, str_or_file: Union[str, TextIO]) -> "Application": From 73b29e68173e96d6606bfaa79a50fdbfee59425a Mon Sep 17 00:00:00 2001 From: Shreyas Krishnaswamy Date: Thu, 17 Mar 2022 19:11:12 -0700 Subject: [PATCH 94/94] Don't sort keys in serve config --- python/ray/serve/scripts.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/ray/serve/scripts.py b/python/ray/serve/scripts.py index 1e9a0ab50c60..ccc46ba22b23 100644 --- a/python/ray/serve/scripts.py +++ b/python/ray/serve/scripts.py @@ -319,11 +319,11 @@ def run( type=str, help=RAY_DASHBOARD_ADDRESS_HELP_STR, ) -def config(address: str, json_format=bool): +def config(address: str): app_info = ServeSubmissionClient(address).get_info() if app_info is not None: - print(yaml.dump(app_info)) + print(yaml.safe_dump(app_info, sort_keys=False)) @cli.command(