Skip to content

Commit

Permalink
[serve] Remove warnings for upcoming default change (#44051)
Browse files Browse the repository at this point in the history
This is unnecessarily spammy for multiple releases. We will do the following:

- Add a warning in 2.10 release notes about upcoming change.
- Next release (3.11) we'll change the default and log a warning if default is used.
- In 3.12 we'll remove the warning.

---------

Signed-off-by: Edward Oakes <[email protected]>
  • Loading branch information
edoakes authored Mar 15, 2024
1 parent 228c7ee commit 24aa146
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 67 deletions.
37 changes: 0 additions & 37 deletions dashboard/modules/serve/serve_rest_api_impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,6 @@ async def put_all_applications(self, req: Request) -> Response:
text=repr(e),
)

self.log_config_change_default_warning(config)

config_http_options = config.http_options.dict()
location = ProxyLocation._to_deployment_mode(config.proxy_location)
full_http_options = dict({"location": location}, **config_http_options)
Expand Down Expand Up @@ -215,41 +213,6 @@ def validate_http_options(self, client, http_options):
f"updated: {divergent_http_options}."
)

def log_config_change_default_warning(self, config):
from ray.serve.config import AutoscalingConfig

for deployment in [
d for app in config.applications for d in app.deployments
]:
if "max_ongoing_requests" not in deployment.dict(exclude_unset=True):
logger.warning(
"The default value for `max_ongoing_requests` will "
"change from 100 to 5 in an upcoming release."
)
break

for deployment in [
d for app in config.applications for d in app.deployments
]:
if isinstance(deployment.autoscaling_config, dict):
autoscaling_config = deployment.autoscaling_config
elif isinstance(deployment.autoscaling_config, AutoscalingConfig):
autoscaling_config = deployment.autoscaling_config.dict(
exclude_unset=True
)
else:
continue

if (
"target_num_ongoing_requests_per_replica" not in autoscaling_config
and "target_ongoing_requests" not in autoscaling_config
):
logger.warning(
"The default value for `target_ongoing_requests` will "
"change from 1.0 to 2.0 in an upcoming release."
)
break

async def get_serve_controller(self):
"""Gets the ServeController to the this cluster's Serve app.
Expand Down
32 changes: 2 additions & 30 deletions python/ray/serve/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,7 @@
ReplicaConfig,
handle_num_replicas_auto,
)
from ray.serve._private.constants import (
DEFAULT_MAX_ONGOING_REQUESTS,
NEW_DEFAULT_MAX_ONGOING_REQUESTS,
SERVE_DEFAULT_APP_NAME,
SERVE_LOGGER_NAME,
)
from ray.serve._private.constants import SERVE_DEFAULT_APP_NAME, SERVE_LOGGER_NAME
from ray.serve._private.deployment_graph_build import build as pipeline_build
from ray.serve._private.deployment_graph_build import (
get_and_validate_ingress_deployment,
Expand Down Expand Up @@ -347,27 +342,11 @@ class MyDeployment:
logger.warning(
"DeprecationWarning: `target_num_ongoing_requests_per_replica` in "
"`autoscaling_config` has been deprecated and replaced by "
"`target_ongoing_requests`. Note that "
"`target_ongoing_requests`. "
"`target_num_ongoing_requests_per_replica` will be removed in a future "
"version."
)

if (
isinstance(autoscaling_config, dict)
and "target_num_ongoing_requests_per_replica" not in autoscaling_config
and "target_ongoing_requests" not in autoscaling_config
) or (
isinstance(autoscaling_config, AutoscalingConfig)
and "target_num_ongoing_requests_per_replica"
not in autoscaling_config.dict(exclude_unset=True)
and "target_ongoing_requests"
not in autoscaling_config.dict(exclude_unset=True)
):
logger.warning(
"The default value for `target_ongoing_requests` is currently 1.0, "
"but will change to 2.0 in an upcoming release."
)

max_ongoing_requests = (
max_ongoing_requests
if max_ongoing_requests is not DEFAULT.VALUE
Expand Down Expand Up @@ -422,13 +401,6 @@ class MyDeployment:
"been deprecated and replaced by `max_ongoing_requests`."
)

if max_concurrent_queries is DEFAULT.VALUE:
logger.warning(
"The default value for `max_ongoing_requests` is currently "
f"{DEFAULT_MAX_ONGOING_REQUESTS}, but will change to "
f"{NEW_DEFAULT_MAX_ONGOING_REQUESTS} in the next upcoming release."
)

if isinstance(logging_config, LoggingConfig):
logging_config = logging_config.dict()

Expand Down

0 comments on commit 24aa146

Please sign in to comment.