Skip to content

feat(rayapp): convert new ComputeConfig schema to legacy at build#492

Merged
elliot-barn merged 5 commits into
mainfrom
rayapp-convert-compute-config
Jun 3, 2026
Merged

feat(rayapp): convert new ComputeConfig schema to legacy at build#492
elliot-barn merged 5 commits into
mainfrom
rayapp-convert-compute-config

Conversation

@Aydin-ab

@Aydin-ab Aydin-ab commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

rayapp build converts compute configs from the new user-facing ComputeConfig schema to legacy CreateComputeTemplateConfig before embedding in ray-app.json (legacy passes through unchanged), keeping the console's template-clone path on the legacy schema as anyscale/templates migrates. Mapping mirrors the anyscale SDK (compute_config_sdk.py).

Test

Built rayapp from this branch and published two templates end-to-end on staging via the tmpl-publish pipeline (the tmpl-cc-staging-test templates branch uses the new schema and overrides download_rayapp.sh to build from here), then confirmed the staging console clone path returns the expected legacy configs:

Templates are migrating compute configs to the user-facing ComputeConfig
schema (head_node/worker_nodes/...). Convert new-schema configs to the legacy
CreateComputeTemplateConfig schema during `rayapp build` so published bundles
stay legacy and the console clone path (which parses legacy) is unaffected.
Mirrors the anyscale SDK user->backend converter; legacy configs pass through.

Signed-off-by: Aydin Abiar <aydin@anyscale.com>
Copilot AI review requested due to automatic review settings June 2, 2026 23:28

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces functionality to convert the new user-facing ComputeConfig schema to the legacy CreateComputeTemplateConfig format, ensuring compatibility with the backend console clone path. Key changes include the addition of a converter utility and corresponding unit tests, as well as integration into the builder pipeline. The review feedback highlights several valid improvement opportunities: addressing redundant disk I/O by passing pre-read bytes to the legacy format detector, standardizing on a single YAML library version (v3) across the package to avoid subtle bugs, returning errors for invalid types in intOrDefault to prevent silent misconfigurations, and extending the isTruthy helper to support map[any]any types.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread rayapp/builder.go Outdated
Comment thread rayapp/compute_config_convert.go
Comment thread rayapp/compute_config_convert.go Outdated
Comment thread rayapp/compute_config_convert.go

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds build-time conversion so rayapp build always publishes legacy-schema compute configs inside ray-app.json, while still allowing template authors to use the new user-facing ComputeConfig schema. This keeps the Anyscale console “template clone” path (which expects legacy) working without backend changes.

Changes:

  • Introduces a YAML converter from new ComputeConfig → legacy CreateComputeTemplateConfig (rayapp/compute_config_convert.go).
  • Adds unit tests covering common conversion scenarios and a few error cases (rayapp/compute_config_convert_test.go).
  • Hooks conversion into the build pipeline: legacy configs pass through unchanged; new-schema configs are converted before base64 embedding (rayapp/builder.go).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
rayapp/compute_config_convert.go Implements new→legacy compute config conversion with key validation and field mapping.
rayapp/compute_config_convert_test.go Adds unit tests for conversion correctness and basic validation errors.
rayapp/builder.go Converts non-legacy compute configs during rayapp build before embedding into ray-app.json.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread rayapp/compute_config_convert.go Outdated
Comment thread rayapp/compute_config_convert.go Outdated
Comment thread rayapp/compute_config_convert.go Outdated
Comment thread rayapp/compute_config_convert.go Outdated
Comment thread rayapp/compute_config_convert.go Outdated
Comment thread rayapp/builder.go Outdated
Aydin-ab added 2 commits June 2, 2026 16:36
- builder: detect schema from the already-read bytes (new isLegacyComputeConfigData) instead of re-reading the file.
- intOrDefault: error on a present-but-non-integer min_nodes/max_nodes instead of silently defaulting.

Signed-off-by: Aydin Abiar <aydin@anyscale.com>
Per Copilot review: validate the types of present fields (auto_select_worker_config,
enable_cross_zone_scaling, worker_nodes, zones) and error on a mismatch instead of
silently defaulting; reject max_nodes < min_nodes; drop "template" from the
unknown-key error message. Adds tests.

Signed-off-by: Aydin Abiar <aydin@anyscale.com>
@Aydin-ab Aydin-ab requested a review from Copilot June 2, 2026 23:44
@Aydin-ab

Aydin-ab commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to convert the new user-facing ComputeConfig schema into the legacy schema format, allowing the backend to process new configurations without breaking the console clone path. This is implemented via a new conversion utility (compute_config_convert.go) and integrated into the app builder. The review feedback focuses on strengthening the robustness of the YAML parsing and conversion logic. Specifically, the reviewer recommends adding explicit type validation for fields like head_node, flags, name, instance_type, and market_type to avoid silent failures or misleading errors. Additionally, suggestions were made to enforce non-negative constraints on worker node counts and to prevent silent truncation of fractional float values during integer conversion.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread rayapp/compute_config_convert.go Outdated
Comment thread rayapp/compute_config_convert.go
Comment thread rayapp/compute_config_convert.go Outdated
Comment thread rayapp/compute_config_convert.go
Comment thread rayapp/compute_config_convert.go Outdated
Comment thread rayapp/compute_config_convert.go

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

Comment thread rayapp/compute_config_convert.go
Comment thread rayapp/compute_config_convert.go Outdated
Comment thread rayapp/compute_config_convert.go Outdated
Comment thread rayapp/compute_config_convert.go
Comment thread rayapp/compute_config_convert_test.go
Aydin-ab added 2 commits June 2, 2026 16:50
Harden the new->legacy converter per review: error when head_node or flags are present but not mappings, when name/instance_type/market_type are non-strings, when min_nodes/max_nodes are negative, and when a node count is a non-integer float (was silently truncated). Adds an error-path test for each.

Signed-off-by: Aydin Abiar <aydin@anyscale.com>
convertNodeCommonFields copied head_node.flags / worker_nodes[*].flags through unchecked; a mis-typed flags value would emit an invalid legacy schema instead of failing fast. Add a shared mapField helper (also used to de-dup the top-level flags check) and reject non-mapping per-node flags, with error-path tests. Per Copilot review.

Signed-off-by: Aydin Abiar <aydin@anyscale.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread rayapp/compute_config_convert.go
@elliot-barn

elliot-barn commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

can you test this with a new compute config in the templates repo? we have logic in rayapp test to create the config with the new schema here, but would be good to verify it works. Everything in the pr looks good.
The new config needs to be in a new config folder so we can test that it gets created. We create the config using the folder name

Aydin-ab added a commit to anyscale/templates that referenced this pull request Jun 3, 2026
…config (DO NOT MERGE)

Throwaway template to drive a full tmpl-publish run validating rayapp build + test handle a ComputeConfig in a brand-new folder. Per reviewer request on ray-project/rayci#492.

Signed-off-by: Aydin Abiar <aydin@anyscale.com>
@Aydin-ab

Aydin-ab commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Verified the new-schema create path with a config in a new folder
https://buildkite.com/anyscale/tmpl-publish/builds/369

  • build-template (rayapp build) ✅ — converts the new ComputeConfig schema → legacy in ray-app.json

compute config ceated here
https://console.anyscale.com/cld_kvedZWag2qA8i5BjxUevf5i7/compute-configs/cpt_flktwtugnn597qrt8i1bp3xkv3/versions

@elliot-barn elliot-barn self-requested a review June 3, 2026 20:36
Aydin-ab added a commit to anyscale/templates that referenced this pull request Jun 3, 2026
Migrate every compute config under configs/ from the legacy CreateComputeTemplateConfig schema to the new user-facing ComputeConfig schema (head_node/worker_nodes/min_nodes/max_nodes/market_type/enable_cross_zone_scaling), pruning defaults (min 0, ON_DEMAND, auto_select false, unschedulable-by-default head resources). Flip ci/validate_build_yaml.py to validate the new schema: strict top-level keys reject legacy configs, and auto_select_worker_config cannot be combined with worker_nodes (mirrors the SDK constraint).

Published bundles are unaffected: rayapp build converts new->legacy at publish time (ray-project/rayci#492), so the console clone path keeps receiving legacy configs.

Signed-off-by: Aydin Abiar <aydin@anyscale.com>
@elliot-barn elliot-barn merged commit 5af6bb2 into main Jun 3, 2026
5 of 8 checks passed
@elliot-barn elliot-barn deleted the rayapp-convert-compute-config branch June 3, 2026 23:03
Aydin-ab added a commit to anyscale/templates that referenced this pull request Jun 5, 2026
Migrate every compute config under configs/ from the legacy CreateComputeTemplateConfig schema to the new user-facing ComputeConfig schema (head_node/worker_nodes/min_nodes/max_nodes/market_type/enable_cross_zone_scaling), pruning defaults (min 0, ON_DEMAND, auto_select false, unschedulable-by-default head resources). Flip ci/validate_build_yaml.py to validate the new schema: strict top-level keys reject legacy configs, and auto_select_worker_config cannot be combined with worker_nodes (mirrors the SDK constraint).

Published bundles are unaffected: rayapp build converts new->legacy at publish time (ray-project/rayci#492), so the console clone path keeps receiving legacy configs.

Signed-off-by: Aydin Abiar <aydin@anyscale.com>
Aydin-ab added a commit to anyscale/templates that referenced this pull request Jun 8, 2026
Migrate every compute config under configs/ from the legacy CreateComputeTemplateConfig schema to the new user-facing ComputeConfig schema (head_node/worker_nodes/min_nodes/max_nodes/market_type/enable_cross_zone_scaling), pruning defaults (min 0, ON_DEMAND, auto_select false, unschedulable-by-default head resources). Flip ci/validate_build_yaml.py to validate the new schema: strict top-level keys reject legacy configs, and auto_select_worker_config cannot be combined with worker_nodes (mirrors the SDK constraint).

Published bundles are unaffected: rayapp build converts new->legacy at publish time (ray-project/rayci#492), so the console clone path keeps receiving legacy configs.

Signed-off-by: Aydin Abiar <aydin@anyscale.com>
Aydin-ab added a commit to anyscale/templates that referenced this pull request Jun 8, 2026
)

Migrates all 111 compute configs under `configs/` from the legacy schema
to the user-facing
[`ComputeConfig`](https://docs.anyscale.com/reference/compute-config-api#computeconfig)
schema; flips `ci/validate_build_yaml.py` to validate it.

Published bundles are unchanged — `rayapp build` converts to the legacy
bundle format at publish time (ray-project/rayci#492, shipped in rayci
v0.45.0, now pinned on main). Validated: converter audit 111/111;
staging publishes tmpl-publish
[#367](https://buildkite.com/anyscale/tmpl-publish/builds/367)–[#369](https://buildkite.com/anyscale/tmpl-publish/builds/369);
release-binary smoke
[#371](https://buildkite.com/anyscale/tmpl-publish/builds/371) (v0.45.0
→ staging, clone path verified).

---------

Signed-off-by: Aydin Abiar <aydin@anyscale.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants