Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: accurate translation caching #18595

Merged
merged 2 commits into from Oct 27, 2022

Conversation

ankush
Copy link
Member

@ankush ankush commented Oct 27, 2022

Translation asset caching in redis is kinda hacky with following bad patterns:

Changes:

  • use generators
  • make all functions stateless, no more manual cache management. (except internal local.cache)
  • Cache the entire merged dictionary. Computation saving > few megs of memory.
  • Call get_all_translations only once in _

@ankush ankush requested review from surajshetty3416 and a team as code owners October 27, 2022 09:37
@ankush ankush requested review from phot0n and removed request for a team October 27, 2022 09:37
@ankush ankush force-pushed the refactor/translation_caching branch from 295a62b to bc65753 Compare October 27, 2022 09:40

# Backward compatibility
get_full_dict = get_all_translations
load_lang = get_translations_from_apps
Copy link
Member Author

Choose a reason for hiding this comment

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

These were horrible names for what they were doing. No other choice 🤞

@ankush ankush force-pushed the refactor/translation_caching branch from bc65753 to 83f8e3b Compare October 27, 2022 09:43
@ankush ankush added the backport version-14-hotfix backport to version 14 label Oct 27, 2022
- use generator
- use sane name for cache key
- avoid manual handling of frappe.local state just use cache() interface
@ankush ankush force-pushed the refactor/translation_caching branch from 83f8e3b to b13cb7a Compare October 27, 2022 10:29
@codecov
Copy link

codecov bot commented Oct 27, 2022

Codecov Report

Merging #18595 (b13cb7a) into develop (5c2b917) will decrease coverage by 0.09%.
The diff coverage is 87.71%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #18595      +/-   ##
===========================================
- Coverage    63.16%   63.06%   -0.10%     
===========================================
  Files          747      747              
  Lines        67248    67557     +309     
  Branches      5995     5995              
===========================================
+ Hits         42477    42605     +128     
- Misses       21312    21493     +181     
  Partials      3459     3459              
Flag Coverage Δ
server-mariadb 67.17% <84.61%> (-0.03%) ⬇️
server-postgres 67.18% <88.46%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@ankush ankush merged commit 3ddac5f into frappe:develop Oct 27, 2022
@ankush ankush deleted the refactor/translation_caching branch October 27, 2022 10:58
mergify bot pushed a commit that referenced this pull request Oct 27, 2022
* refactor: rename poorly named functions

* refactor: getting translation from apps

- use generator
- use sane name for cache key
- avoid manual handling of frappe.local state just use cache() interface

(cherry picked from commit 3ddac5f)

# Conflicts:
#	frappe/tests/utils.py
ankush added a commit that referenced this pull request Oct 27, 2022
* refactor: accurate translation caching (#18595)

* refactor: rename poorly named functions

* refactor: getting translation from apps

- use generator
- use sane name for cache key
- avoid manual handling of frappe.local state just use cache() interface

(cherry picked from commit 3ddac5f)

# Conflicts:
#	frappe/tests/utils.py

* chore: conflicts

Co-authored-by: Ankush Menat <ankush@frappe.io>
Co-authored-by: Ankush Menat <ankushmenat@gmail.com>
ankush added a commit that referenced this pull request Oct 28, 2022
* refactor: rename poorly named functions

* refactor: getting translation from apps

- use generator
- use sane name for cache key
- avoid manual handling of frappe.local state just use cache() interface

(cherry picked from commit 3ddac5f)

# Conflicts:
#	frappe/__init__.py
#	frappe/core/doctype/translation/test_translation.py
#	frappe/tests/utils.py
#	frappe/translate.py
ankush added a commit that referenced this pull request Oct 28, 2022
* refactor: accurate translation caching (#18595)

* refactor: rename poorly named functions

* refactor: getting translation from apps

- use generator
- use sane name for cache key
- avoid manual handling of frappe.local state just use cache() interface

(cherry picked from commit 3ddac5f)

# Conflicts:
#	frappe/__init__.py
#	frappe/core/doctype/translation/test_translation.py
#	frappe/tests/utils.py
#	frappe/translate.py

* chore: conflicts

* fix: dont translate error log title

* fix: import frappe.build

Co-authored-by: Ankush Menat <ankush@frappe.io>
Co-authored-by: Ankush Menat <ankushmenat@gmail.com>
ankush added a commit that referenced this pull request Nov 3, 2022
ankush added a commit that referenced this pull request Nov 3, 2022
mergify bot pushed a commit that referenced this pull request Nov 3, 2022
ankush added a commit that referenced this pull request Nov 3, 2022
This reverts commit 30125b9.

(cherry picked from commit e4d12f0)

Co-authored-by: Ankush Menat <ankush@frappe.io>
frappe-pr-bot pushed a commit that referenced this pull request Nov 3, 2022
## [13.43.1](v13.43.0...v13.43.1) (2022-11-03)

### Reverts

* Revert "refactor: accurate translation caching (backport #18595)" ([91bada5](91bada5)), closes [#18595](#18595)
stephenBDT pushed a commit to alias/frappe that referenced this pull request Nov 7, 2022
…e#18599)

* refactor: accurate translation caching (frappe#18595)

* refactor: rename poorly named functions

* refactor: getting translation from apps

- use generator
- use sane name for cache key
- avoid manual handling of frappe.local state just use cache() interface

(cherry picked from commit 3ddac5f)

# Conflicts:
#	frappe/tests/utils.py

* chore: conflicts

Co-authored-by: Ankush Menat <ankush@frappe.io>
Co-authored-by: Ankush Menat <ankushmenat@gmail.com>
frappe-pr-bot pushed a commit that referenced this pull request Nov 8, 2022
## [13.43.2](v13.43.1...v13.43.2) (2022-11-08)

### Bug Fixes

* don't translate default argument ([#18760](#18760)) ([8038ed7](8038ed7))
* module 'frappe' has no attribute 'build' ([#18761](#18761)) ([a867d56](a867d56))
* not able to click checkbox in file grid view ([ad85843](ad85843))
* send all messages on boot instead of scanning ([#18764](#18764)) ([640724d](640724d))
* show 0 for empty currency, float, & duration fields in list view ([2290fb8](2290fb8))
* translations (backport [#18765](#18765)) ([#18781](#18781)) ([ca631ba](ca631ba))

### Performance Improvements

* ensure cache works for `non_standard_user_types` when empty ([#18744](#18744)) ([708ee35](708ee35))

### Reverts

* Revert "refactor: accurate translation caching (backport #18595) (#18598)" (#18741) ([e4d12f0](e4d12f0)), closes [#18595](#18595) [#18598](#18598) [#18741](#18741)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport version-14-hotfix backport to version 14
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant