feat(rayapp) Anyscale CLI pr2 build id conversion

[elliot-barn]

Adding build id conversion to anyscale cli to convert the slugified build_id to an image_uri

[gemini-code-assist]

Summary of Changes

Hello @elliot-barn, I'm Gemini Code Assist¹! 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 rayapp package by integrating new utilities for interacting with the Anyscale CLI. The primary goal is to provide a robust mechanism for converting Anyscale's internal build identifiers into standard image URIs and extracting Ray versions, which is essential for automating deployments and managing different Ray environments. The changes also include a generalized method for executing Anyscale CLI commands and comprehensive test coverage to ensure reliability.

Highlights

• New Anyscale CLI Utilities: Introduced a new rayapp package containing an AnyscaleCLI struct and associated methods for programmatically interacting with the Anyscale command-line interface.
• Build ID to Image URI Conversion: Added a convertBuildIdToImageURI function that translates Anyscale-specific build IDs (e.g., 'anyscaleray2441-py312-cu128') into a standard image URI format (e.g., 'anyscale/ray:2.44.1-py312-cu128') and extracts the corresponding Ray version.
• Robust CLI Execution: Implemented runAnyscaleCLI to execute Anyscale CLI commands, capturing output while also displaying it to the terminal, and handling cases where the Anyscale CLI is not installed.
• Comprehensive Unit Testing: Included extensive unit tests for all new functionalities, covering the CLI interaction, build ID conversion logic, and edge cases, utilizing a mock Anyscale executable for isolated testing.

🧠 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 Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces functionality for converting Anyscale build IDs into image URIs, along with a wrapper for the Anyscale CLI. The changes are well-tested and the logic for build ID conversion is sound. I've identified a few minor issues: a debug print statement that should be removed from the CLI wrapper, a path construction in the tests that isn't portable, and an opportunity to simplify error checking in tests for better readability. Overall, great work on adding this new capability.

[gemini-code-assist]

This fmt.Println appears to be a debug statement. Since runAnyscaleCLI is an unexported function within a library package, printing directly to stdout can lead to unexpected output for consumers of this package. It's best to remove this or replace it with a structured logger if this output is needed for debugging.

[gemini-code-assist]

Using string concatenation with + "/" to build file paths is not portable and will likely fail on Windows. It's better to use filepath.Join to ensure the tests can run on different operating systems.

Suggested change

	mockScript := tmp + "/anyscale"
	mockScript := filepath.Join(tmp, "anyscale")

[gemini-code-assist]

The error checking logic can be simplified for better readability. Using errors.Is to check the test table's wantErr value is a bit indirect. For a sentinel error like errAnyscaleNotInstalled, a direct comparison with == is more idiomatic and clearly expresses the intent to check for a specific error instance.

Suggested change

	if errors.Is(tt.wantErr, errAnyscaleNotInstalled) {
	if !errors.Is(err, errAnyscaleNotInstalled) {
	t.Errorf("expected errAnyscaleNotInstalled, got: %v", err)
	}
	} else if !strings.Contains(err.Error(), tt.wantErr.Error()) {
	t.Errorf("error %q should contain %q", err.Error(), tt.wantErr.Error())
	}
	if tt.wantErr == errAnyscaleNotInstalled {
	if !errors.Is(err, errAnyscaleNotInstalled) {
	t.Errorf("expected errAnyscaleNotInstalled, got: %v", err)
	}
	} else if !strings.Contains(err.Error(), tt.wantErr.Error()) {
	t.Errorf("error %q should contain %q", err.Error(), tt.wantErr.Error())
	}

[gitar-bot]

Code Review ⚠️ Changes requested 0 resolved / 2 findings

Well-structured addition of Anyscale CLI wrapper with comprehensive tests. One debug print statement should be removed before merging.

⚠️ Quality: Debug print statement left in production code

📄 rayapp/anyscale_cli.go:42

The line fmt.Println("anyscale cli args: ", args) appears to be a debug statement that was accidentally left in the code. This will print to stdout every time the CLI is invoked, which:

1. Pollutes the output with implementation details
2. May interfere with output parsing by callers
3. Doesn't follow the codebase convention of errors bubbling up without inline logging

Suggested fix: Remove this line or replace it with proper structured logging if debugging output is intentionally desired.

💡 Edge Case: Version string not validated for numeric characters

📄 rayapp/anyscale_cli.go:85-91

The convertBuildIdToImageURI function validates the length of the version string but doesn't verify that the characters are actually digits. Input like anyscalerayABCD-py312 would pass validation and produce an invalid version A.BC.D.

This may be acceptable if you control all inputs, but for robustness consider adding validation:

if _, err := strconv.Atoi(versionStr); err != nil {
    return "", "", fmt.Errorf("version string must be numeric: %s", versionStr)
}

Alternatively, if this is an internal-only function with trusted input, document this assumption.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[gitar-bot]

⚠️ Quality: Debug print statement left in production code

Details

The line fmt.Println("anyscale cli args: ", args) appears to be a debug statement that was accidentally left in the code. This will print to stdout every time the CLI is invoked, which:

1. Pollutes the output with implementation details
2. May interfere with output parsing by callers
3. Doesn't follow the codebase convention of errors bubbling up without inline logging

Suggested fix: Remove this line or replace it with proper structured logging if debugging output is intentionally desired.

---

Was this helpful? React with 👍 / 👎

[gitar-bot]

I've addressed the code review findings:

1. ✅ Removed the debug fmt.Println statement that was polluting CLI output
2. ✅ Added validation to ensure version strings contain only numeric characters, with a corresponding test case

All tests pass for the anyscale_cli functionality:

$ go test ./rayapp -run "TestNewAnyscaleCLI|TestIsAnyscaleInstalled|TestRunAnyscaleCLI|TestConvertBuildIdToImageURI" -v
...
PASS
ok      github.com/ray-project/rayci/rayapp     0.007s

The code now follows project conventions (no inline logging, proper error handling with validation) and is ready for review.

[gitar-bot]

I've addressed both code review findings:

1. ✅ Removed debug print statement - Removed fmt.Println("anyscale cli args: ", args) from runAnyscaleCLI method (line 36)
2. ✅ Added numeric validation - Added validation to ensure version strings contain only numeric characters using strconv.Atoi, with a corresponding test case for non-numeric input

All tests pass:

$ go test ./rayapp -run "TestNewAnyscaleCLI|TestIsAnyscaleInstalled|TestRunAnyscaleCLI|TestConvertBuildIdToImageURI" -v
PASS
ok      github.com/ray-project/rayci/rayapp     0.007s

The code now follows project conventions (no inline logging, proper error handling with validation).