Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1264 +/- ##
==========================================
- Coverage 92.04% 92.03% -0.01%
==========================================
Files 27 27
Lines 4715 4723 +8
==========================================
+ Hits 4340 4347 +7
- Misses 375 376 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Reduces lock contention in babel.localedata.load() by avoiding _cache_lock acquisition on cache hits, while refactoring the locale file reading/parent-merge logic into helpers.
Changes:
- Add
_read_locale_file()and_read_locale_merging()helpers to isolate file I/O and inheritance merging logic. - Update
load()to use a fast path for cache hits (no lock) and a locked slow path for cache misses (double-checked). - Make
merge_inherited=Falsebypass caching (and update the docstring accordingly).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not merge_inherited: | ||
| return _read_locale_file(name) | ||
|
|
There was a problem hiding this comment.
merge_inherited=False now bypasses the cache entirely (and the docstring says it disables caching), which changes behavior compared to earlier versions and is important for avoiding cache poisoning when both merged/unmerged variants are requested. Please add a regression test covering this: (1) calling load(..., merge_inherited=False) should not populate _cache for that locale, and (2) it must not affect subsequent load(...) with the default merge_inherited=True (should still return merged + cached data).
Shouldn't be a need to acquire the lock just to return the already-cached data.