Conversation
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 expands the capabilities of 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive Anyscale CLI wrapper and a workspace test runner for templates. The changes are extensive, adding new functionality and thorough testing. My review focuses on correctness, resource management, and usability. I've identified a critical resource leak in the test runner, a double-close bug, and a few medium-severity issues related to redundant output, unhandled errors, and misleading CLI usage information. Overall, this is a solid contribution that will be even better with these issues addressed.
|
this PR is too big to review as a whole.. |
Extract tailWriter into its own file with a bytes.Buffer-based double-buffer design. Initial memory footprint is near zero (no upfront 1MB allocation); buffers grow lazily as data is written. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Replace bytes.Buffer with a fixed-size circular buffer (tailWriter) so memory usage never exceeds maxOutputBufferSize during command execution. The previous approach truncated after the command finished, allowing unbounded growth during long-running commands. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove the inline ring-buffer tailWriter from anyscale_cli.go and its tests from anyscale_cli_test.go. The tailWriter now lives in tail_writer.go (from the tailwriter-extract branch) with a lazy double-buffer implementation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
e2ad936 to
c32de39
Compare
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>
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>
c32de39 to
4a3594e
Compare
This is a fork of PR #390. The following PR description is copied from the original PR.
Adds comprehensive Anyscale CLI wrapper and workspace test runner:
Anyscale CLI (anyscale_cli.go)
Workspace Test Runner (test.go)
Utilities (util.go)