Skip to content

fix: avoid retaining routing parameter instances in cache - #17961

Merged
parthea merged 2 commits into
googleapis:mainfrom
mingi3314:fix/routing-parameter-instance-cache
Aug 4, 2026
Merged

fix: avoid retaining routing parameter instances in cache#17961
parthea merged 2 commits into
googleapis:mainfrom
mingi3314:fix/routing-parameter-instance-cache

Conversation

@mingi3314

Copy link
Copy Markdown
Contributor

functools.cache keeps method arguments, including self, in its unbounded cache. As a result, RoutingParameter instances remain alive after routing resolution.

Use the existing instance-scoped utils.cached_property for the compiled regex and routing key while preserving the to_regex() method API. Add a regression test that verifies a populated cache does not retain the instance.

Testing:

  • pytest packages/gapic-generator/tests/unit/schema/wrappers/test_routing.py -q (Python 3.10 and 3.14)
  • pytest packages/gapic-generator/tests/unit/schema/wrappers -q (Python 3.14)

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request replaces the use of @functools.cache with @utils.cached_property on RoutingParameter methods in wrappers.py to prevent memory leaks caused by instances being retained in the cache. A unit test has been added to verify that RoutingParameter instances can be garbage collected properly. There are no review comments, and I have no additional feedback to provide.

@parthea parthea self-assigned this Jul 31, 2026
@parthea parthea added kokoro:force-run Add this label to force Kokoro to re-run the tests. kokoro:run Add this label to force Kokoro to re-run the tests. labels Jul 31, 2026
@parthea parthea assigned daniel-sanche and ohmayr and unassigned parthea Jul 31, 2026
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jul 31, 2026
Comment thread packages/gapic-generator/tests/unit/schema/wrappers/test_routing.py
@parthea parthea assigned mingi3314 and unassigned daniel-sanche and ohmayr Jul 31, 2026
@mingi3314
mingi3314 requested a review from parthea August 3, 2026 00:29
@mingi3314
mingi3314 force-pushed the fix/routing-parameter-instance-cache branch from dbc181c to 353ace6 Compare August 4, 2026 00:17
@parthea parthea added kokoro:force-run Add this label to force Kokoro to re-run the tests. kokoro:run Add this label to force Kokoro to re-run the tests. labels Aug 4, 2026
@parthea parthea assigned parthea and unassigned mingi3314 Aug 4, 2026
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Aug 4, 2026
@parthea

parthea commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Thanks @mingi3314!

@parthea
parthea enabled auto-merge (squash) August 4, 2026 17:27
@parthea
parthea merged commit f64ada2 into googleapis:main Aug 4, 2026
103 of 105 checks passed
@release-please release-please Bot mentioned this pull request Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants