[serve] Refactor prepare_http_options method

Signed-off-by: axreldable <aleksei.starikov.ax@gmail.com>

Author: axreldable

python/ray/serve/api.py

@@ -39,11 +39,11 @@
 )
 from ray.serve.config import (
     AutoscalingConfig,
-    DeploymentMode,
     HTTPOptions,
     ProxyLocation,
     RequestRouterConfig,
     gRPCOptions,
+    prepare_http_options,
 )
 from ray.serve.context import (
     ReplicaContext,
@@ -64,32 +64,6 @@
 logger = logging.getLogger(SERVE_LOGGER_NAME)
 
 
-def _prepare_http_options(
-    proxy_location: Union[None, str, ProxyLocation],
-    http_options: Union[None, dict, HTTPOptions],
-) -> HTTPOptions:
-    if proxy_location is None:
-        # default value of ProxyLocation (EveryNode) will be used
-        # to set http_options.location if it wasn't set explicitly
-        if http_options is None:
-            http_options = HTTPOptions(location=DeploymentMode.EveryNode)
-            return http_options
-        elif isinstance(http_options, dict):
-            result_http_options = HTTPOptions(**http_options)
-            if "location" not in http_options:
-                result_http_options.location = DeploymentMode.EveryNode
-            return result_http_options
-        else:
-            return http_options
-    else:
-        if http_options is None:
-            http_options = HTTPOptions()
-        elif isinstance(http_options, dict):
-            http_options = HTTPOptions(**http_options)
-        http_options.location = ProxyLocation._to_deployment_mode(proxy_location)
-        return http_options
-
-
 @PublicAPI(stability="stable")
 def start(
     proxy_location: Union[None, str, ProxyLocation] = None,
@@ -122,7 +96,7 @@ class See `gRPCOptions` for supported options.
         logging_config: logging config options for the serve component (
             controller & proxy).
     """
-    http_options = _prepare_http_options(proxy_location, http_options)
+    http_options = prepare_http_options(proxy_location, http_options)
     _private_api.serve_start(
         http_options=http_options,
         grpc_options=grpc_options,

python/ray/serve/config.py

@@ -500,6 +500,55 @@ class Config:
         arbitrary_types_allowed = True
 
 
+def prepare_http_options(
+    proxy_location: Union[None, str, ProxyLocation],
+    http_options: Union[None, dict, HTTPOptions],
+) -> HTTPOptions:
+    """Prepare `HTTPOptions` with desired location based on the proxy location given.
+
+    If `proxy_location` is provided `http_options.location` is determined by it.
+    Alternatively, if `http_options.location` is specified explicitly by user,
+    e.g. {"location": "NoServer"} or via HTTPOptions with or without location field
+    e.g. HTTPOptions(location=DeploymentMode.EveryNode) or HTTPOptions(), then
+    location provided by the user is used.
+
+    Args:
+        proxy_location: provided ProxyLocation
+        http_options: provided HTTPOptions
+
+    Returns:
+        HTTPOptions: A new `HTTPOptions` instance with resolved location.
+
+    Note:
+        Default ProxyLocation is `EveryNode`; default HTTPOptions() location is `HeadOnly`.
+
+    Raises:
+        ValueError: If `http_options` is of an unexpected type.
+    """
+    if http_options is None:
+        location_set_explicitly = False
+        http_options = HTTPOptions()
+    elif isinstance(http_options, dict):
+        location_set_explicitly = "location" in http_options
+        http_options = HTTPOptions(**http_options)
+    elif isinstance(http_options, HTTPOptions):
+        # empty `HTTPOptions()` is considered as user specified the default location value `HeadOnly` explicitly
+        location_set_explicitly = True
+        http_options = HTTPOptions(**http_options.dict(exclude_unset=True))
+    else:
+        raise ValueError(
+            f"Unexpected type for http_options: `{type(http_options).__name__}`"
+        )
+
+    if proxy_location is None:
+        if not location_set_explicitly:
+            http_options.location = DeploymentMode.EveryNode
+    else:
+        http_options.location = ProxyLocation._to_deployment_mode(proxy_location)
+
+    return http_options
+
+
 @PublicAPI(stability="alpha")
 class gRPCOptions(BaseModel):
     """gRPC options for the proxies. Supported fields:

python/ray/serve/scripts.py

@@ -27,8 +27,13 @@
     SERVE_DEFAULT_APP_NAME,
     SERVE_NAMESPACE,
 )
-from ray.serve.api import _prepare_http_options
-from ray.serve.config import DeploymentMode, HTTPOptions, ProxyLocation, gRPCOptions
+from ray.serve.config import (
+    DeploymentMode,
+    HTTPOptions,
+    ProxyLocation,
+    gRPCOptions,
+    prepare_http_options,
+)
 from ray.serve.deployment import Application, deployment_to_schema
 from ray.serve.schema import (
     LoggingConfig,
@@ -539,7 +544,7 @@ def run(
         http_options = HTTPOptions(**config.http_options.dict())
         grpc_options = gRPCOptions(**config.grpc_options.dict())
 
-    http_options = _prepare_http_options(proxy_location, http_options)
+    http_options = prepare_http_options(proxy_location, http_options)
     client = _private_api.serve_start(
         http_options=http_options,
         grpc_options=grpc_options,

python/ray/serve/tests/unit/test_config.py

@@ -6,7 +6,11 @@
 from ray import cloudpickle, serve
 from ray._common.pydantic_compat import ValidationError
 from ray._common.utils import import_attr
-from ray.serve._private.config import DeploymentConfig, ReplicaConfig, _proto_to_dict
+from ray.serve._private.config import (
+    DeploymentConfig,
+    ReplicaConfig,
+    _proto_to_dict,
+)
 from ray.serve._private.constants import (
     DEFAULT_AUTOSCALING_POLICY_NAME,
     DEFAULT_GRPC_PORT,
@@ -21,6 +25,7 @@
     ProxyLocation,
     RequestRouterConfig,
     gRPCOptions,
+    prepare_http_options,
 )
 from ray.serve.generated.serve_pb2 import (
     AutoscalingConfig as AutoscalingConfigProto,
@@ -649,6 +654,65 @@ def test_http_options():
     assert HTTPOptions(location=DeploymentMode.EveryNode).location == "EveryNode"
 
 
+def test_prepare_http_options():
+    assert prepare_http_options(
+        proxy_location=None,
+        http_options=None,
+    ) == HTTPOptions(location=DeploymentMode.EveryNode)
+
+    assert prepare_http_options(
+        proxy_location=None,
+        http_options={},
+    ) == HTTPOptions(location=DeploymentMode.EveryNode)
+
+    assert (
+        prepare_http_options(
+            proxy_location=None,
+            http_options=HTTPOptions(),
+        )
+        == HTTPOptions()
+    )
+
+    assert prepare_http_options(
+        proxy_location=None,
+        http_options={"test": "test"},
+    ) == HTTPOptions(location=DeploymentMode.EveryNode)
+
+    assert prepare_http_options(
+        proxy_location=None,
+        http_options={"host": "0.0.0.0"},
+    ) == HTTPOptions(location=DeploymentMode.EveryNode, host="0.0.0.0")
+
+    assert prepare_http_options(
+        proxy_location=None,
+        http_options={"location": "NoServer"},
+    ) == HTTPOptions(location=DeploymentMode.NoServer)
+
+    assert prepare_http_options(
+        proxy_location=ProxyLocation.Disabled,
+        http_options=None,
+    ) == HTTPOptions(location=DeploymentMode.NoServer)
+
+    assert prepare_http_options(
+        proxy_location=ProxyLocation.HeadOnly,
+        http_options={"host": "0.0.0.0"},
+    ) == HTTPOptions(location=DeploymentMode.HeadOnly, host="0.0.0.0")
+
+    assert prepare_http_options(
+        proxy_location=ProxyLocation.HeadOnly,
+        http_options={"location": "NoServer"},
+    ) == HTTPOptions(location=DeploymentMode.HeadOnly)
+
+    with pytest.raises(ValueError, match="not a valid ProxyLocation"):
+        prepare_http_options(proxy_location="wrong", http_options=None)
+
+    with pytest.raises(ValueError, match="not a valid enumeration"):
+        prepare_http_options(proxy_location=None, http_options={"location": "123"})
+
+    with pytest.raises(ValueError, match="Unexpected type"):
+        prepare_http_options(proxy_location=None, http_options="wrong")
+
+
 def test_with_proto():
     # Test roundtrip
     config = DeploymentConfig(num_replicas=100, max_ongoing_requests=16)