fix(agents): use shallow copy in BaseGenerator.with_params()#2319
fix(agents): use shallow copy in BaseGenerator.with_params()#2319kevinmessiaen merged 2 commits intomainfrom
Conversation
Summary of ChangesHello, 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 addresses a critical bug where attempting to deep-copy generator instances equipped with rate limiters would lead to a TypeError because asyncio.Semaphore objects are not picklable. The solution involves a two-pronged approach: first, by switching to a shallow copy operation in the with_params method of BaseGenerator where a deep copy is unnecessary, and second, by explicitly defining a deepcopy method for BaseRateLimiter to gracefully handle deep-copy requests by returning the instance itself, thus avoiding errors with unpicklable asyncio primitives. This ensures the stability of generator instances when their parameters are updated. 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. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request effectively addresses a critical TypeError that occurred during deep-copying of BaseGenerator instances containing asyncio.Semaphore within rate limiters. The change to use a shallow copy in BaseGenerator.model_copy() and the addition of a defensive __deepcopy__ method in BaseRateLimiter are appropriate and robust solutions. These changes improve the stability and maintainability of the generator and rate limiter components.
| @override | ||
| def __deepcopy__(self, memo: dict[int, Any] | None = None) -> Self: | ||
| """Return self so that deep-copying a generator doesn't duplicate | ||
| rate-limiter state (which contains unpicklable asyncio primitives).""" | ||
| return self |
There was a problem hiding this comment.
The addition of the __deepcopy__ method to BaseRateLimiter is a crucial defensive measure. By returning self, it correctly handles deep-copy attempts for rate limiters, preventing crashes due to unpicklable asyncio primitives. This directly addresses the root cause of the TypeError mentioned in the PR description.
`model_copy(deep=True)` crashes with `TypeError: cannot pickle '_asyncio.Future'` when the generator has a rate limiter with max_concurrent set, because asyncio.Semaphore cannot be deep-copied. Switch to a shallow copy (sufficient since params is replaced, not mutated) and add a defensive `__deepcopy__` on BaseRateLimiter to prevent future deep-copy attempts from crashing. Closes ENG-1474 Made-with: Cursor
850e393 to
0e1ab98
Compare
model_copy(deep=True)crashes withTypeError: cannot pickle '_asyncio.Future'when the generator has a rate limiter with max_concurrent set, because asyncio.Semaphore cannot be deep-copied.Switch to a shallow copy (sufficient since params is replaced, not mutated) and add a defensive
__deepcopy__on BaseRateLimiter to prevent future deep-copy attempts from crashing.Closes ENG-1474
Made-with: Cursor
Description
Related Issue
Type of Change
Checklist
CODE_OF_CONDUCT.mddocument.CONTRIBUTING.mdguide.pdm.lockrunningpdm update-lock(only applicable whenpyproject.tomlhas beenmodified)