Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[serve] Add support for multi app to serve build #33216

Merged
merged 4 commits into from
Mar 14, 2023

Conversation

zcin
Copy link
Contributor

@zcin zcin commented Mar 10, 2023

Why are these changes needed?

Users should be able to build multiple app targets into one ServeDeploySchema file using one serve build command, which can then be submitted through serve deploy or PUT /api/serve/applications.

  • By default, serve build will only accept one import path.
  • If the feature flag --multi-app or -m is specified, serve build will accept 1 or more import paths. The app configs are identical to the single-app config except:
    • route_prefix is extracted from the ingress deployment and placed at the application level of the configs
    • Auto-generated names app1, app2, ... etc are added to the application level of the configs
    • Host and port are removed from the application level of the configs and placed at the top level of the deploy config

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>
@zcin zcin marked this pull request as ready for review March 10, 2023 23:22
Copy link
Contributor

@shrekris-anyscale shrekris-anyscale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work so far! I left some comments.

python/ray/serve/deployment.py Outdated Show resolved Hide resolved
python/ray/serve/scripts.py Outdated Show resolved Hide resolved
python/ray/serve/scripts.py Outdated Show resolved Hide resolved
python/ray/serve/scripts.py Outdated Show resolved Hide resolved
python/ray/serve/scripts.py Outdated Show resolved Hide resolved
python/ray/serve/scripts.py Show resolved Hide resolved
Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>
@zcin zcin force-pushed the serve-build-multi branch from 225ea0d to 9786d0c Compare March 13, 2023 17:33
Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>
@zcin zcin force-pushed the serve-build-multi branch from 6aa3bed to d7249a1 Compare March 13, 2023 18:16
Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>
Copy link
Contributor

@shrekris-anyscale shrekris-anyscale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work on this change!

@zcin
Copy link
Contributor Author

zcin commented Mar 14, 2023

@edoakes ready for merge!

@edoakes edoakes merged commit 4abc15f into ray-project:master Mar 14, 2023
@zcin zcin deleted the serve-build-multi branch March 14, 2023 21:54
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request Mar 21, 2023
Users should be able to build multiple app targets into one `ServeDeploySchema` file using one `serve build` command, which can then be submitted through `serve deploy` or `PUT /api/serve/applications`.

- By default, `serve build` will only accept one import path.
- If the feature flag `--multi-app` or `-m` is specified, `serve build` will accept 1 or more import paths. The app configs are identical to the single-app config *except*:
  - `route_prefix` is extracted from the ingress deployment and placed at the application level of the configs
  - Auto-generated names `app1`, `app2`, ... etc are added to the application level of the configs
  - Host and port are removed from the application level of the configs and placed at the top level of the deploy config

Signed-off-by: Jack He <jackhe2345@gmail.com>
edoakes pushed a commit to edoakes/ray that referenced this pull request Mar 22, 2023
Users should be able to build multiple app targets into one `ServeDeploySchema` file using one `serve build` command, which can then be submitted through `serve deploy` or `PUT /api/serve/applications`.

- By default, `serve build` will only accept one import path.
- If the feature flag `--multi-app` or `-m` is specified, `serve build` will accept 1 or more import paths. The app configs are identical to the single-app config *except*:
  - `route_prefix` is extracted from the ingress deployment and placed at the application level of the configs
  - Auto-generated names `app1`, `app2`, ... etc are added to the application level of the configs
  - Host and port are removed from the application level of the configs and placed at the top level of the deploy config

Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
peytondmurray pushed a commit to peytondmurray/ray that referenced this pull request Mar 22, 2023
Users should be able to build multiple app targets into one `ServeDeploySchema` file using one `serve build` command, which can then be submitted through `serve deploy` or `PUT /api/serve/applications`.

- By default, `serve build` will only accept one import path.
- If the feature flag `--multi-app` or `-m` is specified, `serve build` will accept 1 or more import paths. The app configs are identical to the single-app config *except*:
  - `route_prefix` is extracted from the ingress deployment and placed at the application level of the configs
  - Auto-generated names `app1`, `app2`, ... etc are added to the application level of the configs
  - Host and port are removed from the application level of the configs and placed at the top level of the deploy config
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
Users should be able to build multiple app targets into one `ServeDeploySchema` file using one `serve build` command, which can then be submitted through `serve deploy` or `PUT /api/serve/applications`.

- By default, `serve build` will only accept one import path.
- If the feature flag `--multi-app` or `-m` is specified, `serve build` will accept 1 or more import paths. The app configs are identical to the single-app config *except*:
  - `route_prefix` is extracted from the ingress deployment and placed at the application level of the configs
  - Auto-generated names `app1`, `app2`, ... etc are added to the application level of the configs
  - Host and port are removed from the application level of the configs and placed at the top level of the deploy config

Signed-off-by: elliottower <elliot@elliottower.com>
ProjectsByJackHe pushed a commit to ProjectsByJackHe/ray that referenced this pull request May 4, 2023
Users should be able to build multiple app targets into one `ServeDeploySchema` file using one `serve build` command, which can then be submitted through `serve deploy` or `PUT /api/serve/applications`.

- By default, `serve build` will only accept one import path.
- If the feature flag `--multi-app` or `-m` is specified, `serve build` will accept 1 or more import paths. The app configs are identical to the single-app config *except*:
  - `route_prefix` is extracted from the ingress deployment and placed at the application level of the configs
  - Auto-generated names `app1`, `app2`, ... etc are added to the application level of the configs
  - Host and port are removed from the application level of the configs and placed at the top level of the deploy config

Signed-off-by: Jack He <jackhe2345@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants