fix(integrations): close AsyncDaytona client in DaytonaEnvironment.close()#6307
Open
anxkhn wants to merge 1 commit into
Open
fix(integrations): close AsyncDaytona client in DaytonaEnvironment.close()#6307anxkhn wants to merge 1 commit into
anxkhn wants to merge 1 commit into
Conversation
…ose() DaytonaEnvironment.close() deleted the sandbox and then dropped the AsyncDaytona client by setting self._client = None, without awaiting the client's close(). That client owns the underlying aiohttp session plus the REST and toolbox API clients, so discarding the reference leaked sockets on every create/close cycle. The Daytona SDK documents that AsyncDaytona.close() should be called to release those HTTP sessions and avoid resource leaks (or the client used as an async context manager). Await self._client.close() before clearing the reference. close() is idempotent, so the existing double-close no-op behavior is preserved. The unit test now gives the mock client an AsyncMock close() and asserts DaytonaEnvironment.close() awaits it exactly once.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
2. Or, if no issue exists, describe the change:
Problem:
DaytonaEnvironment.close()insrc/google/adk/integrations/daytona/_daytona_environment.pydeletes the sandbox and then drops theAsyncDaytonaclient by settingself._client = None, without awaiting the client'sclose():That client is an
AsyncDaytonainstance created in_create_sandbox(); it owns the underlying aiohttpClientSessionplus the REST and toolbox API clients. Just dropping the reference never releases those, so a socket/session leak accumulates on every create/close cycle. This matters for long-lived agents and for repeated test runs that set up and tear down an environment.The Daytona Python SDK documents that
AsyncDaytona.close()should be called to properly close the underlying HTTP sessions and avoid resource leaks (or the client can be used as an async context manager). The current code runs none of that.Solution:
Await
self._client.close()before clearing the reference, guarded on the client not beingNone:AsyncDaytona.close()is idempotent (it closes the API clients and the shared aiohttp session, all of which are no-ops when already closed), so the existing "secondclose()is a no-op" behavior is preserved.Testing Plan
Unit Tests:
The existing
test_close_deletes_sandbox_and_is_idempotenttest used aMagicMockclient and never asserted the client was closed, so it did not catch the leak. This PR gives the mock client anAsyncMockclose()and assertsDaytonaEnvironment.close()awaits it exactly once (and that it stays a no-op, still awaited only once, on a secondclose()).I confirmed the test actually guards the fix: with the production
await self._client.close()line removed, the updated test fails withAssertionError: Expected close to have been awaited once. Awaited 0 times.; with the fix in place it passes.pytestresults:No regressions in the wider integrations suite:
Manual End-to-End (E2E) Tests:
Not run. Exercising this path end to end requires a live Daytona account and API key to provision a real remote sandbox, which is not available in this environment. The change is confined to the client-teardown step of
close()and is covered by the unit test above.Checklist
Additional context
DaytonaEnvironmentis a recently added integration (feat(integrations): Add DaytonaEnvironment for remote sandbox workspaces, commitdf6baf4a). This is a small follow-up correctness fix to its teardown path.