feat(rayapp) Anyscale CLI pr3 compute config#398
Conversation
Add foundational components for Anyscale CLI wrapper: - AnyscaleCLI struct and NewAnyscaleCLI() constructor - errAnyscaleNotInstalled error sentinel for installation checks - isAnyscaleInstalled() helper to verify CLI availability - runAnyscaleCLI() base method for executing anyscale commands Includes comprehensive test coverage with setupMockAnyscale test helper. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add convertBuildIdToImageURI function that parses build IDs like "anyscaleray2441-py312-cu128" into image URI "anyscale/ray:2.44.1-py312-cu128" and Ray version "2.44.1". Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add WorkspaceState type with constants (StateTerminated, StateStarting, StateRunning) and String() method for representing workspace states. Add compute config operations: - parseComputeConfigName: parse AWS config path to config name - CreateComputeConfig: create compute config if it doesn't exist - GetComputeConfig: retrieve compute config details by name Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @elliot-barn, 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 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. 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces new functionality for interacting with the Anyscale CLI, specifically for managing compute configurations. It includes functions to create and retrieve compute configs, along with utility functions for parsing build IDs and compute config names. The changes are well-tested with comprehensive unit tests covering various scenarios, including error handling and edge cases. Overall, the code is well-structured and follows good practices for Go development.
I am having trouble creating individual review comments. Click here to see my feedback.
rayapp/anyscale_cli.go (58)
The fmt.Println statement here is likely for debugging purposes. In production code, it's generally better to use a structured logging library (e.g., log package or a more advanced one) or remove debug prints entirely to avoid polluting standard output, which can interfere with programmatic parsing of CLI output.
rayapp/anyscale_cli_test.go (100-101)
Comparing error messages using strings.Contains(err.Error(), tt.wantErr.Error()) can be brittle. If the error message changes slightly in the future, this test might break even if the underlying logic is still correct. Consider defining specific sentinel errors in anyscale_cli.go and using errors.Is for more robust error comparison in tests.
Code Review 👍 Approved with suggestions 0 resolved / 3 findingsWell-structured CLI wrapper with comprehensive tests. Found 3 minor edge case and quality suggestions that would improve robustness but don't block the PR. 💡 Edge Case: WorkspaceState.String() returns empty for unknown states📄 rayapp/anyscale_cli.go:31-33 The Consider adding a default case or validation: func (ws WorkspaceState) String() string {
if name, ok := WorkspaceStateName[ws]; ok {
return name
}
return fmt.Sprintf("UNKNOWN(%d)", ws)
}This provides better debugging information if an unknown state is encountered. 💡 Quality: Debug output printed to stdout may pollute logsThe Consider one of these alternatives:
type AnyscaleCLI struct {
Verbose bool
}
func (ac *AnyscaleCLI) runAnyscaleCLI(args []string) (string, error) {
if ac.Verbose {
fmt.Println("anyscale cli args: ", args)
}
// ...
}💡 Edge Case: No validation for numeric version string in build ID conversion📄 rayapp/anyscale_cli.go:101-109 The Consider adding validation: // After extracting versionStr
for _, c := range versionStr {
if c < '0' || c > '9' {
return "", "", fmt.Errorf("version string contains non-numeric character: %s", versionStr)
}
}Alternatively, document that non-numeric version strings will be passed through as-is if that's intentional. OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
Adding compute config CLI operations to anyscale cli
Method: GetComputeConfig
CLI Command: anyscale compute-config get -n
Purpose: Check if compute config exists
────────────────────────────────────────
Method: CreateComputeConfig
CLI Command: anyscale compute-config create -n -f
Purpose: Create compute config if needed
────────────────────────────────────────