feat(rayapp): Add Anyscale CLI wrapper and workspace test runner for templates#406
feat(rayapp): Add Anyscale CLI wrapper and workspace test runner for templates#406sai-miduthuri wants to merge 24 commits intoray-project:mainfrom
Conversation
Adds AnyscaleCLI struct with methods for: - Compute config management (Create, Get, List) - Workspace operations (Create, Start, Terminate, Push, RunCommand, GetStatus, WaitForState) - Build ID to image URI conversion - Compute config name parsing All methods are standalone and don't depend on external types. Full unit test coverage included. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Adds comprehensive Anyscale CLI wrapper and workspace test runner: **Anyscale CLI (anyscale_cli.go)** - Compute config management (Create, Get, List) - Workspace operations (Create, Start, Terminate, Push, RunCommand) - Build ID to image URI conversion - Full unit test coverage **Workspace Test Runner (test.go)** - WorkspaceTestConfig for running tests in Anyscale workspaces - Automated flow: create compute config, create workspace, push template, run tests, cleanup - Zips template to temp directory before pushing - Full unit test coverage for all scenarios **Utilities (util.go)** - zipDirectory function for packaging templates Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Restore WorkspaceTestConfig struct that was commented out but still referenced by workspace functions - Update TestCreateComputeConfig to use temp config files instead of non-existent paths, since CreateComputeConfig now reads the file - Remove ListComputeConfigs function and its tests Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
The token field was never used - authentication relies on environment variables (ANYSCALE_CLI_TOKEN, ANYSCALE_HOST) instead. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Authentication is handled via environment variables (ANYSCALE_CLI_TOKEN & ANYSCALE_HOST) rather than interactive login. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Summary of ChangesHello @sai-miduthuri, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive Anyscale CLI wrapper and a workspace test runner, which is a significant feature addition. The new code includes a Go wrapper for the anyscale CLI, a test runner for templates, and extensive unit tests. The command-line interface for rayapp has also been improved to accommodate the new test command. My review has identified a couple of high-severity issues with a restrictive version parsing logic that could cause problems with future Ray versions, and a medium-severity issue with a confusing CLI usage message. Despite these points, the overall contribution is solid and includes excellent test coverage.
| buildIDVersionRe := regexp.MustCompile(`^(\d)(\d{2})(\d+)$`) | ||
| matches := buildIDVersionRe.FindStringSubmatch(versionStr) | ||
| if matches == nil { | ||
| return "", "", fmt.Errorf("version string must match major(1 digit).minor(2 digits).patch(1+ digits): %s", versionStr) | ||
| } |
There was a problem hiding this comment.
The regular expression for parsing the version from a build ID is too restrictive. It assumes the major version is always a single digit (^(\d)). This will fail for Ray versions 10 and above (e.g., a build ID for Ray 10.0.0). To make this more robust for future versions, the regex should allow for one or more digits for the major version.
| buildIDVersionRe := regexp.MustCompile(`^(\d)(\d{2})(\d+)$`) | |
| matches := buildIDVersionRe.FindStringSubmatch(versionStr) | |
| if matches == nil { | |
| return "", "", fmt.Errorf("version string must match major(1 digit).minor(2 digits).patch(1+ digits): %s", versionStr) | |
| } | |
| buildIDVersionRe := regexp.MustCompile(`^(\d+)(\d{2})(\d+)$`) | |
| matches := buildIDVersionRe.FindStringSubmatch(versionStr) | |
| if matches == nil { | |
| return "", "", fmt.Errorf("version string must match major(1+ digits).minor(2 digits).patch(1+ digits): %s", versionStr) | |
| } |
| imageURIVersionRe := regexp.MustCompile(`^(\d)\.(\d{2})\.(\d+)$`) | ||
| matches := imageURIVersionRe.FindStringSubmatch(versionStr) | ||
| if matches == nil { | ||
| return "", "", fmt.Errorf("image URI version must match major(1 digit).minor(2 digits).patch(1+ digits): %s", versionStr) | ||
| } |
There was a problem hiding this comment.
Similar to the build ID conversion, the regular expression for parsing the version from an image URI is too restrictive. It assumes the major version is always a single digit (^(\d)). This will fail for Ray versions 10 and above. To make this more robust for future versions, the regex should allow for one or more digits for the major version.
| imageURIVersionRe := regexp.MustCompile(`^(\d)\.(\d{2})\.(\d+)$`) | |
| matches := imageURIVersionRe.FindStringSubmatch(versionStr) | |
| if matches == nil { | |
| return "", "", fmt.Errorf("image URI version must match major(1 digit).minor(2 digits).patch(1+ digits): %s", versionStr) | |
| } | |
| imageURIVersionRe := regexp.MustCompile(`^(\d+)\.(\d{2})\.(\d+)$`) | |
| matches := imageURIVersionRe.FindStringSubmatch(versionStr) | |
| if matches == nil { | |
| return "", "", fmt.Errorf("image URI version must match major(1+ digits).minor(2 digits).patch(1+ digits): %s", versionStr) | |
| } |
| fmt.Println(" --workspace string Workspace name (required)") | ||
| fmt.Println(" --template-dir string Template directory (required)") | ||
| fmt.Println(" --config string Config file path (required)") |
There was a problem hiding this comment.
The usage message for the test command lists several flags (--workspace, --template-dir, --config) that are not implemented, and it's missing the implemented --build flag. The flag definitions for the unimplemented flags are also commented out in the code (lines 27-29).
This is confusing for users. To fix this, the usage message should reflect the actual implemented flags. The commented-out flag definitions should also be removed.
| fmt.Println(" --workspace string Workspace name (required)") | |
| fmt.Println(" --template-dir string Template directory (required)") | |
| fmt.Println(" --config string Config file path (required)") | |
| fmt.Println(" --build string Build file (default \"BUILD.yaml\")") |
2 participants
PR opened from wrong branch. Closing this PR and moving to #412