[Serve] Change API to use ServeDeploySchema to replace ServeApplicationSchema #32285

zcin · 2023-02-07T22:08:35Z

Signed-off-by: Cindy Zhang cindyzyx9@gmail.com

Why are these changes needed?

This changes the controller to deploy config files of the format ServeDeploySchema instead of ServeApplicationSchema. This prepares the backend python API to support the new, multi-app compatible REST API.

deploy_app --> deploy_apps
[single-app compatibility] controller.deploy_apps still takes ServeApplicationSchema but converts it to a ServeDeploySchema with a single application (and takes name "" if no name is provided).
~~If an application is redeployed (i.e. an app with same name already exists) and the dictionary of deployment versions is unchanged, its deployment timestamp does not update.~~ We don't have the versioning support for this yet, this should be added in the future.
[multi-app config] Given a deployed config file, each application's deployments will have the app name prepended to the deployment name (so there is no gap between the names of a deployment when it is actually deployed vs the names in the config file), and if user fetches the config using get_app_config, the app name prefix is removed.
[multi-app mode] Applying a ServeDeploySchema config file will set that as the goal state; it will deploy the apps listed in the config file and remove any that are not listed.

Related issue number

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

sihanwang41

lgtm!

python/ray/serve/schema.py

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

zcin · 2023-02-10T23:43:38Z

@sihanwang41 Added some changes, could you do another pass?
@shrekris-anyscale @edoakes Ping for review!

sihanwang41 · 2023-02-13T19:03:35Z

Hi @zcin , the pr has unit tests failures and lint issue, can you take a look?

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

zcin · 2023-02-14T18:42:33Z

@sihanwang41 @shrekris-anyscale Tests fixed by moving to a new file. I also addressed comments, please take another look when you get the chance, ty!

sihanwang41 · 2023-02-14T19:28:53Z

@sihanwang41 @shrekris-anyscale Tests fixed by moving to a new file. I also addressed comments, please take another look when you get the chance, ty!

thanks! Do we know why we have to move to a new file?

zcin · 2023-02-14T20:03:16Z

Do we know why we have to move to a new file?

I'm not completely sure, but after adding the new tests for this PR the original test class TestDeployApp that was in test_standalone2.py became pretty big so it also seemed reasonable to move to a new file.

It's possible that test_standalone2.py became too big? I also ran into problems with the new test file test_deploy_app.py when I labeled it as "medium", and the problems were fixed when I changed it to "large".

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

zcin · 2023-02-14T23:53:13Z

Synced with @sihanwang41, looking into why keeping the tests in the same file causes failures.

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

zcin · 2023-02-15T02:58:04Z

Tests re-added to test_standalone2.py and are passing! I modified the test fixture used for TestDeployApp, I think it helps with state-sharing problems.

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

sihanwang41 · 2023-02-15T18:28:57Z

LGTM

shrekris-anyscale

Nice work, the changes look good to me! Once we add the warning, this change should be ready to merge.

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

zcin · 2023-02-15T22:31:33Z

@edoakes Should be ready to merge!

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

…onSchema (ray-project#32285) This changes the controller to deploy config files of the format `ServeDeploySchema` instead of `ServeApplicationSchema`. This prepares the backend python API to support the new, multi-app compatible REST API. - `deploy_app` --> `deploy_apps` - [single-app compatibility] `controller.deploy_apps` still takes `ServeApplicationSchema` but converts it to a `ServeDeploySchema` with a single application (and takes name "" if no name is provided). - ~~If an application is redeployed (i.e. an app with same name already exists) and the dictionary of deployment versions is unchanged, its deployment timestamp does not update.~~ We don't have the versioning support for this yet, this should be added in the future. - [multi-app config] Given a deployed config file, each application's deployments will have the app name prepended to the deployment name (so there is no gap between the names of a deployment when it is actually deployed vs the names in the config file), and if user fetches the config using `get_app_config`, the app name prefix is removed. - [multi-app mode] Applying a `ServeDeploySchema` config file will set that as the goal state; it will deploy the apps listed in the config file and remove any that are not listed.

…onSchema (ray-project#32285) This changes the controller to deploy config files of the format `ServeDeploySchema` instead of `ServeApplicationSchema`. This prepares the backend python API to support the new, multi-app compatible REST API. - `deploy_app` --> `deploy_apps` - [single-app compatibility] `controller.deploy_apps` still takes `ServeApplicationSchema` but converts it to a `ServeDeploySchema` with a single application (and takes name "" if no name is provided). - ~~If an application is redeployed (i.e. an app with same name already exists) and the dictionary of deployment versions is unchanged, its deployment timestamp does not update.~~ We don't have the versioning support for this yet, this should be added in the future. - [multi-app config] Given a deployed config file, each application's deployments will have the app name prepended to the deployment name (so there is no gap between the names of a deployment when it is actually deployed vs the names in the config file), and if user fetches the config using `get_app_config`, the app name prefix is removed. - [multi-app mode] Applying a `ServeDeploySchema` config file will set that as the goal state; it will deploy the apps listed in the config file and remove any that are not listed. Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>

…onSchema (ray-project#32285) This changes the controller to deploy config files of the format `ServeDeploySchema` instead of `ServeApplicationSchema`. This prepares the backend python API to support the new, multi-app compatible REST API. - `deploy_app` --> `deploy_apps` - [single-app compatibility] `controller.deploy_apps` still takes `ServeApplicationSchema` but converts it to a `ServeDeploySchema` with a single application (and takes name "" if no name is provided). - ~~If an application is redeployed (i.e. an app with same name already exists) and the dictionary of deployment versions is unchanged, its deployment timestamp does not update.~~ We don't have the versioning support for this yet, this should be added in the future. - [multi-app config] Given a deployed config file, each application's deployments will have the app name prepended to the deployment name (so there is no gap between the names of a deployment when it is actually deployed vs the names in the config file), and if user fetches the config using `get_app_config`, the app name prefix is removed. - [multi-app mode] Applying a `ServeDeploySchema` config file will set that as the goal state; it will deploy the apps listed in the config file and remove any that are not listed.

…onSchema (ray-project#32285) This changes the controller to deploy config files of the format `ServeDeploySchema` instead of `ServeApplicationSchema`. This prepares the backend python API to support the new, multi-app compatible REST API. - `deploy_app` --> `deploy_apps` - [single-app compatibility] `controller.deploy_apps` still takes `ServeApplicationSchema` but converts it to a `ServeDeploySchema` with a single application (and takes name "" if no name is provided). - ~~If an application is redeployed (i.e. an app with same name already exists) and the dictionary of deployment versions is unchanged, its deployment timestamp does not update.~~ We don't have the versioning support for this yet, this should be added in the future. - [multi-app config] Given a deployed config file, each application's deployments will have the app name prepended to the deployment name (so there is no gap between the names of a deployment when it is actually deployed vs the names in the config file), and if user fetches the config using `get_app_config`, the app name prefix is removed. - [multi-app mode] Applying a `ServeDeploySchema` config file will set that as the goal state; it will deploy the apps listed in the config file and remove any that are not listed. Signed-off-by: elliottower <elliot@elliottower.com>

zcin force-pushed the serve-deploy-schema branch from 927ef30 to 97551e6 Compare February 8, 2023 00:43

zcin added 3 commits February 8, 2023 12:03

change backend to use deploy schema

9f21dc2

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

wip

f6fdebb

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

fix doc example

0e3ba8f

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

zcin force-pushed the serve-deploy-schema branch from d16fa75 to 0e3ba8f Compare February 8, 2023 20:56

zcin marked this pull request as ready for review February 8, 2023 20:57

zcin requested review from edoakes, simon-mo, jiaodong, shrekris-anyscale, sihanwang41, architkulkarni and a team as code owners February 8, 2023 20:57

zcin assigned sihanwang41 and shrekris-anyscale Feb 8, 2023

sihanwang41 approved these changes Feb 9, 2023

View reviewed changes

python/ray/serve/schema.py Outdated Show resolved Hide resolved

zcin added 4 commits February 9, 2023 11:40

Merge branch 'master' into serve-deploy-schema

9852660

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

revert deployment timestamp changes

72988a6

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

add schema unit test

5bbfaab

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

fix

a32c384

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

zcin changed the title ~~[Serve] Change API to use ServeDeploySchema to replace ServeApplicationSchema~~ [WIP][Serve] Change API to use ServeDeploySchema to replace ServeApplicationSchema Feb 10, 2023

zcin added 2 commits February 10, 2023 15:20

fix multi-app deploy path + add unit tests

783dd93

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

Merge branch 'master' into serve-deploy-schema

6657dba

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

zcin changed the title ~~[WIP][Serve] Change API to use ServeDeploySchema to replace ServeApplicationSchema~~ [Serve] Change API to use ServeDeploySchema to replace ServeApplicationSchema Feb 10, 2023

add schema unit tests

c29c21c

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

zcin added 3 commits February 13, 2023 11:12

Merge branch 'master' into serve-deploy-schema

06ce4d4

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

fix tests

59e584f

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

update docstring

bf30cb6

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

zcin added 3 commits February 13, 2023 22:16

try deflake tests

6465240

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

try deflake tests

1684544

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

lint

a7fb792

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

zcin added 2 commits February 14, 2023 14:57

Merge branch 'master' into serve-deploy-schema

d7d9ef3

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

re-add tests to test_standalone

2b30c57

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

format changes

9affa3e

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

zcin force-pushed the serve-deploy-schema branch 2 times, most recently from ac87ca7 to a978b38 Compare February 15, 2023 01:02

format changes

055560d

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

zcin force-pushed the serve-deploy-schema branch from a978b38 to 055560d Compare February 15, 2023 01:12

Merge branch 'master' into serve-deploy-schema

fb0c1be

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

shrekris-anyscale approved these changes Feb 15, 2023

View reviewed changes

add debug statements for host/port override

b6242a1

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

edoakes merged commit b742538 into ray-project:master Feb 15, 2023

zcin deleted the serve-deploy-schema branch February 16, 2023 00:43

zcin added a commit to zcin/ray that referenced this pull request Mar 2, 2023

include change from ray-project#31342 and ray-project#32285

700db50

Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>

zcin mentioned this pull request Mar 3, 2023

Revert "[serve] Replace Ray Client with Ray Job Submission for `serve… #32976

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Serve] Change API to use ServeDeploySchema to replace ServeApplicationSchema #32285

[Serve] Change API to use ServeDeploySchema to replace ServeApplicationSchema #32285

zcin commented Feb 7, 2023 •

edited

Loading

sihanwang41 left a comment

zcin commented Feb 10, 2023

sihanwang41 commented Feb 13, 2023

zcin commented Feb 14, 2023

sihanwang41 commented Feb 14, 2023

zcin commented Feb 14, 2023 •

edited

Loading

zcin commented Feb 14, 2023

zcin commented Feb 15, 2023

sihanwang41 commented Feb 15, 2023

shrekris-anyscale left a comment

zcin commented Feb 15, 2023

[Serve] Change API to use ServeDeploySchema to replace ServeApplicationSchema #32285

[Serve] Change API to use ServeDeploySchema to replace ServeApplicationSchema #32285

Conversation

zcin commented Feb 7, 2023 • edited Loading

Why are these changes needed?

Related issue number

Checks

sihanwang41 left a comment

Choose a reason for hiding this comment

zcin commented Feb 10, 2023

sihanwang41 commented Feb 13, 2023

zcin commented Feb 14, 2023

sihanwang41 commented Feb 14, 2023

zcin commented Feb 14, 2023 • edited Loading

zcin commented Feb 14, 2023

zcin commented Feb 15, 2023

sihanwang41 commented Feb 15, 2023

shrekris-anyscale left a comment

Choose a reason for hiding this comment

zcin commented Feb 15, 2023

zcin commented Feb 7, 2023 •

edited

Loading

zcin commented Feb 14, 2023 •

edited

Loading