Support passing runtime envs to explorer and trainer #446
Conversation
Summary of ChangesHello @pan-x-c, 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 flexibility of the system by introducing the ability to pass custom runtime environments to the Ray actors for the Explorer and Trainer components. This change allows for more dynamic and isolated execution contexts, which can be crucial for managing dependencies or specific environment variables required by these services. 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
|
![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 support for passing runtime_env to the explorer and trainer Ray actors. The changes are clear and directly implement the intended functionality. My review includes two main points. First, for consistency with the existing codebase, I've recommended using Dict from the typing module for type hints. Second, I've suggested adding test cases to verify that the runtime_env is correctly applied to the actors, which will help ensure the new functionality is robust and prevent future regressions. Overall, this is a good and useful addition.
| def get_actor(cls, config: Config, runtime_env: Optional[dict] = None): | ||
| """Get a Ray actor for the explorer.""" | ||
| return ( | ||
| ray.remote(cls) | ||
| .options( | ||
| name=config.explorer.name, | ||
| namespace=ray.get_runtime_context().namespace, | ||
| runtime_env=runtime_env, | ||
| ) | ||
| .remote(config) | ||
| ) |
There was a problem hiding this comment.
The change to support runtime_env is a good addition. However, the new functionality is not covered by tests. To ensure its correctness and prevent future regressions, please consider adding a test case that verifies the runtime_env is correctly passed to the Ray actor. For example, you could pass an environment variable and check its value inside the actor.
| def get_actor(cls, config: Config, runtime_env: Optional[dict] = None): | ||
| """Get a Ray actor for the trainer.""" | ||
| return ( | ||
| ray.remote(cls) | ||
| .options(name=config.trainer.name, namespace=ray.get_runtime_context().namespace) | ||
| .options( | ||
| name=config.trainer.name, | ||
| namespace=ray.get_runtime_context().namespace, | ||
| runtime_env=runtime_env, | ||
| ) | ||
| .remote(config) | ||
| ) |
There was a problem hiding this comment.
Similar to the explorer, this change to support runtime_env for the trainer actor is useful. To improve the robustness of the code, it would be beneficial to add a test case to ensure it works as expected.
1 participant
Description
As the title says
Checklist
Please check the following items before code is ready to be reviewed.