mirror of
https://github.com/vale981/ray
synced 2025-03-05 10:01:43 -05:00
[Serve] Deprecate deployment's prev_version field (#26217)
This commit is contained in:
parent
3ffff53428
commit
20c6c0725a
8 changed files with 0 additions and 133 deletions
|
@ -328,7 +328,6 @@ def deployment(func_or_class: Callable) -> Deployment:
|
|||
def deployment(
|
||||
name: Optional[str] = None,
|
||||
version: Optional[str] = None,
|
||||
prev_version: Optional[str] = None,
|
||||
num_replicas: Optional[int] = None,
|
||||
init_args: Optional[Tuple[Any]] = None,
|
||||
init_kwargs: Optional[Dict[Any, Any]] = None,
|
||||
|
@ -350,7 +349,6 @@ def deployment(
|
|||
_func_or_class: Optional[Callable] = None,
|
||||
name: Optional[str] = None,
|
||||
version: Optional[str] = None,
|
||||
prev_version: Optional[str] = None,
|
||||
num_replicas: Optional[int] = None,
|
||||
init_args: Optional[Tuple[Any]] = None,
|
||||
init_kwargs: Optional[Dict[Any, Any]] = None,
|
||||
|
@ -374,11 +372,6 @@ def deployment(
|
|||
with a version change, a rolling update of the replicas will be
|
||||
performed. If not provided, every deployment will be treated as a
|
||||
new version.
|
||||
prev_version (Optional[str]): Version of the existing deployment which
|
||||
is used as a precondition for the next deployment. If prev_version
|
||||
does not match with the existing deployment's version, the
|
||||
deployment will fail. If not provided, deployment procedure will
|
||||
not check the existing deployment's version.
|
||||
num_replicas (Optional[int]): The number of processes to start up that
|
||||
will handle requests to this deployment. Defaults to 1.
|
||||
init_args (Optional[Tuple]): Positional args to be passed to the class
|
||||
|
@ -451,7 +444,6 @@ def deployment(
|
|||
name if name is not None else _func_or_class.__name__,
|
||||
config,
|
||||
version=version,
|
||||
prev_version=prev_version,
|
||||
init_args=init_args,
|
||||
init_kwargs=init_kwargs,
|
||||
route_prefix=route_prefix,
|
||||
|
@ -601,7 +593,6 @@ def run(
|
|||
"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,
|
||||
}
|
||||
|
|
|
@ -218,7 +218,6 @@ class ServeControllerClient:
|
|||
ray_actor_options: Optional[Dict] = None,
|
||||
config: Optional[Union[DeploymentConfig, Dict[str, Any]]] = None,
|
||||
version: Optional[str] = None,
|
||||
prev_version: Optional[str] = None,
|
||||
route_prefix: Optional[str] = None,
|
||||
url: Optional[str] = None,
|
||||
_blocking: Optional[bool] = True,
|
||||
|
@ -232,7 +231,6 @@ class ServeControllerClient:
|
|||
ray_actor_options=ray_actor_options,
|
||||
config=config,
|
||||
version=version,
|
||||
prev_version=prev_version,
|
||||
route_prefix=route_prefix,
|
||||
)
|
||||
|
||||
|
@ -262,7 +260,6 @@ class ServeControllerClient:
|
|||
ray_actor_options=deployment["ray_actor_options"],
|
||||
config=deployment["config"],
|
||||
version=deployment["version"],
|
||||
prev_version=deployment["prev_version"],
|
||||
route_prefix=deployment["route_prefix"],
|
||||
)
|
||||
)
|
||||
|
@ -444,7 +441,6 @@ class ServeControllerClient:
|
|||
ray_actor_options: Optional[Dict] = None,
|
||||
config: Optional[Union[DeploymentConfig, Dict[str, Any]]] = None,
|
||||
version: Optional[str] = None,
|
||||
prev_version: Optional[str] = None,
|
||||
route_prefix: Optional[str] = None,
|
||||
) -> Dict:
|
||||
"""
|
||||
|
@ -482,7 +478,6 @@ class ServeControllerClient:
|
|||
raise TypeError("config must be a DeploymentConfig or a dictionary.")
|
||||
|
||||
deployment_config.version = version
|
||||
deployment_config.prev_version = prev_version
|
||||
|
||||
if (
|
||||
deployment_config.autoscaling_config is not None
|
||||
|
|
|
@ -133,7 +133,6 @@ class DeploymentConfig(BaseModel):
|
|||
deployment_language: Any = DeploymentLanguage.PYTHON
|
||||
|
||||
version: Optional[str] = None
|
||||
prev_version: Optional[str] = None
|
||||
|
||||
class Config:
|
||||
validate_assignment = True
|
||||
|
@ -178,9 +177,6 @@ class DeploymentConfig(BaseModel):
|
|||
data["user_config"] = None
|
||||
if "autoscaling_config" in data:
|
||||
data["autoscaling_config"] = AutoscalingConfig(**data["autoscaling_config"])
|
||||
if "prev_version" in data:
|
||||
if data["prev_version"] == "":
|
||||
data["prev_version"] = None
|
||||
if "version" in data:
|
||||
if data["version"] == "":
|
||||
data["version"] = None
|
||||
|
|
|
@ -311,27 +311,10 @@ class ServeController:
|
|||
deployment_config_proto_bytes
|
||||
)
|
||||
version = deployment_config.version
|
||||
prev_version = deployment_config.prev_version
|
||||
replica_config = ReplicaConfig.from_proto_bytes(
|
||||
replica_config_proto_bytes, deployment_config.deployment_language
|
||||
)
|
||||
|
||||
if prev_version is not None:
|
||||
existing_deployment_info = self.deployment_state_manager.get_deployment(
|
||||
name
|
||||
)
|
||||
if existing_deployment_info is None or not existing_deployment_info.version:
|
||||
raise ValueError(
|
||||
f"prev_version '{prev_version}' is specified but "
|
||||
"there is no existing deployment."
|
||||
)
|
||||
if existing_deployment_info.version != prev_version:
|
||||
raise ValueError(
|
||||
f"prev_version '{prev_version}' "
|
||||
"does not match with the existing "
|
||||
f"version '{existing_deployment_info.version}'."
|
||||
)
|
||||
|
||||
autoscaling_config = deployment_config.autoscaling_config
|
||||
if autoscaling_config is not None:
|
||||
# TODO: is this the desired behaviour? Should this be a setting?
|
||||
|
|
|
@ -38,7 +38,6 @@ class Deployment:
|
|||
name: str,
|
||||
config: DeploymentConfig,
|
||||
version: Optional[str] = None,
|
||||
prev_version: Optional[str] = None,
|
||||
init_args: Optional[Tuple[Any]] = None,
|
||||
init_kwargs: Optional[Tuple[Any]] = None,
|
||||
route_prefix: Union[str, None, DEFAULT] = DEFAULT.VALUE,
|
||||
|
@ -63,8 +62,6 @@ class Deployment:
|
|||
raise TypeError("name must be a string.")
|
||||
if not (version is None or isinstance(version, str)):
|
||||
raise TypeError("version must be a string.")
|
||||
if not (prev_version is None or isinstance(prev_version, str)):
|
||||
raise TypeError("prev_version must be a string.")
|
||||
if not (init_args is None or isinstance(init_args, (tuple, list))):
|
||||
raise TypeError("init_args must be a tuple.")
|
||||
if not (init_kwargs is None or isinstance(init_kwargs, dict)):
|
||||
|
@ -91,7 +88,6 @@ class Deployment:
|
|||
self._func_or_class = func_or_class
|
||||
self._name = name
|
||||
self._version = version
|
||||
self._prev_version = prev_version
|
||||
self._config = config
|
||||
self._init_args = init_args
|
||||
self._init_kwargs = init_kwargs
|
||||
|
@ -111,15 +107,6 @@ class Deployment:
|
|||
"""
|
||||
return self._version
|
||||
|
||||
@property
|
||||
def prev_version(self) -> Optional[str]:
|
||||
"""Existing version of deployment to target.
|
||||
|
||||
If prev_version does not match with existing deployment
|
||||
version, the deployment will fail to be deployed.
|
||||
"""
|
||||
return self._prev_version
|
||||
|
||||
@property
|
||||
def func_or_class(self) -> Union[Callable, str]:
|
||||
"""Underlying class or function that this deployment wraps."""
|
||||
|
@ -235,7 +222,6 @@ class Deployment:
|
|||
ray_actor_options=self._ray_actor_options,
|
||||
config=self._config,
|
||||
version=self._version,
|
||||
prev_version=self._prev_version,
|
||||
route_prefix=self.route_prefix,
|
||||
url=self.url,
|
||||
_blocking=_blocking,
|
||||
|
@ -271,7 +257,6 @@ class Deployment:
|
|||
func_or_class: Optional[Callable] = None,
|
||||
name: Optional[str] = None,
|
||||
version: Optional[str] = None,
|
||||
prev_version: Optional[str] = None,
|
||||
init_args: Optional[Tuple[Any]] = None,
|
||||
init_kwargs: Optional[Dict[Any, Any]] = None,
|
||||
route_prefix: Union[str, None, DEFAULT] = DEFAULT.VALUE,
|
||||
|
@ -317,9 +302,6 @@ class Deployment:
|
|||
if version is None:
|
||||
version = self._version
|
||||
|
||||
if prev_version is None:
|
||||
prev_version = self._prev_version
|
||||
|
||||
if init_args is None:
|
||||
init_args = self._init_args
|
||||
|
||||
|
@ -353,7 +335,6 @@ class Deployment:
|
|||
name,
|
||||
new_config,
|
||||
version=version,
|
||||
prev_version=prev_version,
|
||||
init_args=init_args,
|
||||
init_kwargs=init_kwargs,
|
||||
route_prefix=route_prefix,
|
||||
|
@ -367,7 +348,6 @@ class Deployment:
|
|||
func_or_class: Optional[Callable] = None,
|
||||
name: Optional[str] = None,
|
||||
version: Optional[str] = None,
|
||||
prev_version: Optional[str] = None,
|
||||
init_args: Optional[Tuple[Any]] = None,
|
||||
init_kwargs: Optional[Dict[Any, Any]] = None,
|
||||
route_prefix: Union[str, None, DEFAULT] = DEFAULT.VALUE,
|
||||
|
@ -391,7 +371,6 @@ class Deployment:
|
|||
func_or_class=func_or_class,
|
||||
name=name,
|
||||
version=version,
|
||||
prev_version=prev_version,
|
||||
init_args=init_args,
|
||||
init_kwargs=init_kwargs,
|
||||
route_prefix=route_prefix,
|
||||
|
@ -409,7 +388,6 @@ class Deployment:
|
|||
self._func_or_class = validated._func_or_class
|
||||
self._name = validated._name
|
||||
self._version = validated._version
|
||||
self._prev_version = validated._prev_version
|
||||
self._init_args = validated._init_args
|
||||
self._init_kwargs = validated._init_kwargs
|
||||
self._route_prefix = validated._route_prefix
|
||||
|
|
|
@ -430,7 +430,6 @@ class TestSetOptions:
|
|||
@serve.deployment(
|
||||
num_replicas=4,
|
||||
max_concurrent_queries=3,
|
||||
prev_version="abcd",
|
||||
ray_actor_options={"num_cpus": 2},
|
||||
_health_check_timeout_s=17,
|
||||
)
|
||||
|
@ -439,14 +438,12 @@ class TestSetOptions:
|
|||
|
||||
f.set_options(
|
||||
num_replicas=9,
|
||||
prev_version="abcd",
|
||||
version="efgh",
|
||||
ray_actor_options={"num_gpus": 3},
|
||||
)
|
||||
|
||||
assert f.num_replicas == 9
|
||||
assert f.max_concurrent_queries == 3
|
||||
assert f.prev_version == "abcd"
|
||||
assert f.version == "efgh"
|
||||
assert f.ray_actor_options == {"num_gpus": 3}
|
||||
assert f._config.health_check_timeout_s == 17
|
||||
|
|
|
@ -128,77 +128,6 @@ def test_deploy_no_version(serve_instance, use_handle):
|
|||
assert pid5 == pid4
|
||||
|
||||
|
||||
@pytest.mark.parametrize("use_handle", [True, False])
|
||||
def test_deploy_prev_version(serve_instance, use_handle):
|
||||
name = "test"
|
||||
|
||||
@serve.deployment(name=name)
|
||||
def v1(*args):
|
||||
return f"1|{os.getpid()}"
|
||||
|
||||
def call():
|
||||
if use_handle:
|
||||
ret = ray.get(v1.get_handle().remote())
|
||||
else:
|
||||
ret = requests.get(f"http://localhost:8000/{name}").text
|
||||
|
||||
return ret.split("|")[0], ret.split("|")[1]
|
||||
|
||||
# Deploy with prev_version specified, where there is no existing deployment
|
||||
with pytest.raises(ValueError):
|
||||
v1.options(version="1", prev_version="0").deploy()
|
||||
|
||||
v1.deploy()
|
||||
val1, pid1 = call()
|
||||
assert val1 == "1"
|
||||
|
||||
@serve.deployment(name=name)
|
||||
def v2(*args):
|
||||
return f"2|{os.getpid()}"
|
||||
|
||||
# Deploying without specifying prev_version should still be possible.
|
||||
v2.deploy()
|
||||
val2, pid2 = call()
|
||||
assert val2 == "2"
|
||||
assert pid2 != pid1
|
||||
|
||||
v2.options(version="1").deploy()
|
||||
val3, pid3 = call()
|
||||
assert val3 == "2"
|
||||
assert pid3 != pid2
|
||||
|
||||
@serve.deployment(name=name)
|
||||
def v3(*args):
|
||||
return f"3|{os.getpid()}"
|
||||
|
||||
# If prev_version does not match with the existing version, it should fail.
|
||||
with pytest.raises(ValueError):
|
||||
v3.options(version="2", prev_version="0").deploy()
|
||||
|
||||
# If prev_version matches with the existing version, it should succeed.
|
||||
v3.options(version="2", prev_version="1").deploy()
|
||||
val4, pid4 = call()
|
||||
assert val4 == "3"
|
||||
assert pid4 != pid3
|
||||
|
||||
# Specifying the version should stop updates from happening.
|
||||
v3.options(version="2").deploy()
|
||||
val5, pid5 = call()
|
||||
assert val5 == "3"
|
||||
assert pid5 == pid4
|
||||
|
||||
v2.options(version="3", prev_version="2").deploy()
|
||||
val6, pid6 = call()
|
||||
assert val6 == "2"
|
||||
assert pid6 != pid5
|
||||
|
||||
# Deploying without specifying prev_version should still be possible.
|
||||
v1.deploy()
|
||||
val7, pid7 = call()
|
||||
assert val7 == "1"
|
||||
assert pid7 != pid6
|
||||
|
||||
|
||||
@pytest.mark.parametrize("use_handle", [True, False])
|
||||
def test_config_change(serve_instance, use_handle):
|
||||
@serve.deployment(version="1")
|
||||
|
|
|
@ -88,8 +88,6 @@ message DeploymentConfig {
|
|||
AutoscalingConfig autoscaling_config = 10;
|
||||
|
||||
string version = 11;
|
||||
|
||||
string prev_version = 12;
|
||||
}
|
||||
|
||||
// Deployment language.
|
||||
|
|
Loading…
Add table
Reference in a new issue