Skip to content

Commit

Permalink
[Serve] Relaxed input Serve config's schemas to allow extra params (r…
Browse files Browse the repository at this point in the history
…ay-project#38748)

* Relaxed input Serve config's schemas to allow extra params

Signed-off-by: Alexey Kudinkin <ak@anyscale.com>

* Adjusted for `DeploymentSchema`, `RayActorOptionsSchema`

Signed-off-by: Alexey Kudinkin <ak@anyscale.com>

* Enabled forward-compatibility for internal configs (`DeploymentConfig`, `HTTPOptions`)

Signed-off-by: Alexey Kudinkin <ak@anyscale.com>
  • Loading branch information
alexeykudinkin authored Aug 24, 2023
1 parent f6c7dd9 commit dd41535
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 32 deletions.
2 changes: 0 additions & 2 deletions python/ray/serve/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ class DeploymentConfig(BaseModel):

class Config:
validate_assignment = True
extra = "forbid"
arbitrary_types_allowed = True

# Dynamic default for max_concurrent_queries
Expand Down Expand Up @@ -712,7 +711,6 @@ def fixed_number_replicas_should_exist(cls, v, values):

class Config:
validate_assignment = True
extra = "forbid"
arbitrary_types_allowed = True


Expand Down
21 changes: 14 additions & 7 deletions python/ray/serve/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def _route_prefix_format(cls, v):


@PublicAPI(stability="beta")
class RayActorOptionsSchema(BaseModel, extra=Extra.forbid):
class RayActorOptionsSchema(BaseModel):
"""Options with which to start a replica actor."""

runtime_env: dict = Field(
Expand Down Expand Up @@ -138,9 +138,7 @@ def runtime_env_contains_remote_uris(cls, v):


@PublicAPI(stability="beta")
class DeploymentSchema(
BaseModel, extra=Extra.forbid, allow_population_by_field_name=True
):
class DeploymentSchema(BaseModel, allow_population_by_field_name=True):
"""
Specifies options for one deployment within a Serve application. For each deployment
this can optionally be included in `ServeApplicationSchema` to override deployment
Expand Down Expand Up @@ -315,7 +313,7 @@ def _deployment_info_to_schema(name: str, info: DeploymentInfo) -> DeploymentSch


@PublicAPI(stability="beta")
class ServeApplicationSchema(BaseModel, extra=Extra.forbid):
class ServeApplicationSchema(BaseModel):
"""
Describes one Serve application, and currently can also be used as a standalone
config to deploy a single application to a Ray cluster.
Expand Down Expand Up @@ -532,7 +530,12 @@ class gRPCOptionsSchema(BaseModel):

@PublicAPI(stability="alpha")
class HTTPOptionsSchema(BaseModel):
"""Options to start the HTTP Proxy with."""
"""Options to start the HTTP Proxy with.
NOTE: This config allows extra parameters to make it forward-compatible (ie
older versions of Serve are able to accept configs from a newer versions,
simply ignoring new parameters)
"""

host: str = Field(
default="0.0.0.0",
Expand Down Expand Up @@ -571,13 +574,17 @@ class HTTPOptionsSchema(BaseModel):


@PublicAPI(stability="alpha")
class ServeDeploySchema(BaseModel, extra=Extra.forbid):
class ServeDeploySchema(BaseModel):
"""
Multi-application config for deploying a list of Serve applications to the Ray
cluster.
This is the request JSON schema for the v2 REST API
`PUT "/api/serve/applications/"`.
NOTE: This config allows extra parameters to make it forward-compatible (ie
older versions of Serve are able to accept configs from a newer versions,
simply ignoring new parameters)
"""

proxy_location: DeploymentMode = Field(
Expand Down
20 changes: 14 additions & 6 deletions python/ray/serve/tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ def test_deploy_duplicate_routes(ray_start_stop):


@pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.")
def test_deploy_bad_config1(ray_start_stop):
def test_deploy_bad_v2_config(ray_start_stop):
"""Deploy a bad config with field applications, should try to parse as v2 config."""

config_file = os.path.join(
Expand All @@ -321,12 +321,16 @@ def test_deploy_bad_config1(ray_start_stop):
subprocess.check_output(
["serve", "deploy", config_file], stderr=subprocess.STDOUT
)
assert "ValidationError" in e.value.output.decode("utf-8")
assert "ServeDeploySchema" in e.value.output.decode("utf-8")

output = e.value.output.decode("utf-8")

assert "ValidationError" in output, output
assert "ServeDeploySchema" in output, output
assert "Please ensure each application's route_prefix is unique" in output


@pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.")
def test_deploy_bad_config2(ray_start_stop):
def test_deploy_bad_v1_config(ray_start_stop):
"""
Deploy a bad config without field applications, should try to parse as v1 config.
"""
Expand All @@ -339,8 +343,12 @@ def test_deploy_bad_config2(ray_start_stop):
subprocess.check_output(
["serve", "deploy", config_file], stderr=subprocess.STDOUT
)
assert "ValidationError" in e.value.output.decode("utf-8")
assert "ServeApplicationSchema" in e.value.output.decode("utf-8")

output = e.value.output.decode("utf-8")

assert "none is not an allowed value" in output
assert "ValidationError" in output, output
assert "ServeApplicationSchema" in output, output


@pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.")
Expand Down
38 changes: 35 additions & 3 deletions python/ray/serve/tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@
from ray.serve._private.utils import DEFAULT
from ray.serve.generated.serve_pb2_grpc import add_UserDefinedServiceServicer_to_server
from ray.serve._private.constants import DEFAULT_GRPC_PORT
from ray.serve.schema import (
ServeDeploySchema,
HTTPOptionsSchema,
ServeApplicationSchema,
DeploymentSchema,
)


def test_autoscaling_config_validation():
Expand Down Expand Up @@ -59,9 +65,8 @@ def test_autoscaling_config_validation():

class TestDeploymentConfig:
def test_deployment_config_validation(self):
# Test unknown key.
with pytest.raises(ValidationError):
DeploymentConfig(unknown_key=-1)
# Test config ignoring unknown keys (required for forward-compatibility)
DeploymentConfig(new_version_key=-1)

# Test num_replicas validation.
DeploymentConfig(num_replicas=1)
Expand Down Expand Up @@ -343,9 +348,36 @@ def f():
assert config.init_kwargs == dict()


def test_config_schemas_forward_compatible():
# Test configs ignoring unknown keys (required for forward-compatibility)
ServeDeploySchema(
http_options=HTTPOptionsSchema(
new_version_config_key="this config is from newer version of Ray"
),
applications=[
ServeApplicationSchema(
import_path="module.app",
deployments=[
DeploymentSchema(
name="deployment",
new_version_config_key="this config is from newer version"
" of Ray",
)
],
new_version_config_key="this config is from newer version of Ray",
),
],
new_version_config_key="this config is from newer version of Ray",
)


def test_http_options():
HTTPOptions()
HTTPOptions(host="8.8.8.8", middlewares=[object()])

# Test configs ignoring unknown keys (required for forward-compatibility)
HTTPOptions(new_version_config_key="this config is from newer version of Ray")

assert HTTPOptions(host=None).location == "NoServer"
assert HTTPOptions(location=None).location == "NoServer"
assert HTTPOptions(location=DeploymentMode.EveryNode).location == "EveryNode"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ applications:
increment: 1

- name: "app2"
# Route prefixes should be unique across all apps!
route_prefix: "/app1"
import_path: ray.serve.tests.test_config_files.pizza.serve_dag
deployments:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import_path: ray.serve.tests.test_config_files.test_dag.conditional_dag.serve_dag
# Import path is a required field
import_path:

deployments:

- name: Multiplier
user_config:
factor: 1
factor: 1
21 changes: 9 additions & 12 deletions python/ray/serve/tests/test_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,9 @@ def test_extra_fields_invalid_ray_actor_options(self):
# 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)
# Schema should NOT raise error when extra field is included
ray_actor_options_schema["extra_field"] = None
RayActorOptionsSchema.parse_obj(ray_actor_options_schema)

def test_dict_defaults_ray_actor_options(self):
# Dictionary fields should have empty dictionaries as defaults, not None
Expand Down Expand Up @@ -339,10 +338,9 @@ def test_extra_fields_invalid_deployment_schema(self):
# 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)
# Schema should NOT raise error when extra field is included
deployment_schema["extra_field"] = None
DeploymentSchema.parse_obj(deployment_schema)

@pytest.mark.parametrize(
"option",
Expand Down Expand Up @@ -430,10 +428,9 @@ def test_extra_fields_invalid_serve_application_schema(self):
# 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)
# Schema should NOT raise error when extra field is included
serve_application_schema["extra_field"] = None
ServeApplicationSchema.parse_obj(serve_application_schema)

@pytest.mark.parametrize("env", get_valid_runtime_envs())
def test_serve_application_valid_runtime_env(self, env):
Expand Down

0 comments on commit dd41535

Please sign in to comment.