[Fix] Remove weakref usage in RequestSigner #3590
Conversation
Remove weakref.proxy usage in RequestSigner. The weakref was added for Python 2.6 memory leak prevention and is no longer needed. Also revert MAX_GROWTH_BYTES from 13MB back to 10MB in test_resource_leaks.py now that Python 3.14 support is official.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3590 +/- ##
========================================
Coverage 93.17% 93.17%
========================================
Files 68 68
Lines 15411 15412 +1
========================================
+ Hits 14359 14360 +1
Misses 1052 1052 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| # that the memory can't increase by more than 13MB. | ||
| # that the memory can't increase by more than 10MB. | ||
|
|
||
| # TODO: Attempt to bring this back to 10MB once Python 3.14 releases | ||
| MAX_GROWTH_BYTES = 13 * 1024 * 1024 | ||
| MAX_GROWTH_BYTES = 10 * 1024 * 1024 |
There was a problem hiding this comment.
Since these are two unrelated fixes, let's try to keep them in separate PRs. From the looks of it, the tests are still using elevated memory, so this change can't be done currently.
Revert MAX_GROWTH_BYTES from 10MB back to 13MB in test_resource_leaks.py as tests are still using elevated memory and the reduction cannot be implemented currently. The weakref removal changes remain as they are still valid.
adev-code
changed the title
|
Based on the CI test failures, it looks like the weakref line needs to stay in place. Removing it causes test failures, which confirms that it's a required part of the implementation. The weakref helps free up memory more quickly. Without it, memory consumption grows substantially over time, making the removal not worthwhile. Closing. |
3 participants
Remove weakref.proxy usage in RequestSigner. The weakref was added for Python 2.6 memory leak prevention and is no longer needed.