[go: up one dir, main page]

perf: misc client cache improvements by ankush [frappe] PR#29070

From: https://github.com/frappe/frappe/pull/29070
Date: 2025-01-07 16:14:43+05:30

  • perf: misc client cache improvements (#29070)



Diagnostics

pre-commit failed for source commit: fdba41c68213f9819429828225f01248e01d4153
[WARNING] top-level `default_stages` uses deprecated stage names (commit) which will be removed in a future version.  run: `pre-commit migrate-config` to automatically fix this.

frappe/cache_manager.py:94:33: RUF005 Consider `[user, *common_default_keys]` instead of concatenation
   |
92 | def clear_defaults_cache(user=None):
93 |     if user:
94 |         frappe.cache.hdel("defaults", [user] + common_default_keys)
   |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF005
95 |     elif frappe.flags.in_install != "frappe":
96 |         frappe.cache.delete_value("defaults")
   |
   = help: Replace with `[user, *common_default_keys]`

frappe/cache_manager.py:143:3: F821 Undefined name `clear_meta_cache`
    |
141 |         for pattern in wildcard_keys:
142 |             to_del += frappe.cache.get_keys(pattern)
143 |         clear_meta_cache()
    |         ^^^^^^^^^^^^^^^^ F821
144 |
145 |     frappe.cache.delete_value(to_del)
    |

frappe/database/database.py:68:21: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
   |
66 |     MAX_COLUMN_LENGTH = 64
67 |
68 |     OPTIONAL_COLUMNS = ["_user_tags", "_comments", "_assign", "_liked_by"]
   |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF012
69 |     DEFAULT_SHORTCUTS = ["_Login", "__user", "_Full Name", "Today", "__today", "now", "Now"]
70 |     STANDARD_VARCHAR_COLUMNS = ("name", "owner", "modified_by")
   |

frappe/database/database.py:69:22: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
   |
68 |     OPTIONAL_COLUMNS = ["_user_tags", "_comments", "_assign", "_liked_by"]
69 |     DEFAULT_SHORTCUTS = ["_Login", "__user", "_Full Name", "Today", "__today", "now", "Now"]
   |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF012
70 |     STANDARD_VARCHAR_COLUMNS = ("name", "owner", "modified_by")
71 |     DEFAULT_COLUMNS = ["name", "creation", "modified", "modified_by", "owner", "docstatus", "idx"]
   |

frappe/database/database.py:71:20: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
   |
69 |     DEFAULT_SHORTCUTS = ["_Login", "__user", "_Full Name", "Today", "__today", "now", "Now"]
70 |     STANDARD_VARCHAR_COLUMNS = ("name", "owner", "modified_by")
71 |     DEFAULT_COLUMNS = ["name", "creation", "modified", "modified_by", "owner", "docstatus", "idx"]
   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF012
72 |     CHILD_TABLE_COLUMNS = ("parent", "parenttype", "parentfield")
73 |     MAX_WRITES_PER_TRANSACTION = 200_000
   |

frappe/database/schema.py:54:17: RUF005 Consider `[*frappe.db.DEFAULT_COLUMNS]` instead of concatenation
   |
53 |     def get_column_definitions(self):
54 |         column_list = [] + frappe.db.DEFAULT_COLUMNS
   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF005
55 |         ret = []
56 |         for k in list(self.columns):
   |
   = help: Replace with `[*frappe.db.DEFAULT_COLUMNS]`

frappe/tests/test_db.py:344:20: RUF015 Prefer `next(...)` over single element slice
    |
343 |         # Testing read
344 |         self.assertEqual(list(frappe.get_all("ToDo", fields=[random_field], limit=1)[0])[0], random_field)
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF015
345 |         self.assertEqual(
346 |             list(frappe.get_all("ToDo", fields=[f"`{random_field}` as total"], limit=1)[0])[0], "total"
    |
    = help: Replace with `next(...)`

frappe/tests/test_db.py:346:4: RUF015 Prefer `next(...)` over single element slice
    |
344 |         self.assertEqual(list(frappe.get_all("ToDo", fields=[random_field], limit=1)[0])[0], random_field)
345 |         self.assertEqual(
346 |             list(frappe.get_all("ToDo", fields=[f"`{random_field}` as total"], limit=1)[0])[0], "total"
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF015
347 |         )
    |
    = help: Replace with `next(...)`

frappe/tests/test_db.py:351:4: RUF015 Prefer `next(...)` over single element slice
    |
349 |           # Testing read for distinct and sql functions
350 |           self.assertEqual(
351 |               list(
    |  _____________^
352 | |                 frappe.get_all(
353 | |                     "ToDo",
354 | |                     fields=[f"`{random_field}` as total"],
355 | |                     distinct=True,
356 | |                     limit=1,
357 | |                 )[0]
358 | |             )[0],
    | |________________^ RUF015
359 |               "total",
360 |           )
    |
    = help: Replace with `next(...)`

frappe/tests/test_db.py:362:4: RUF015 Prefer `next(...)` over single element slice
    |
360 |           )
361 |           self.assertEqual(
362 |               list(
    |  _____________^
363 | |                 frappe.get_all(
364 | |                     "ToDo",
365 | |                     fields=[f"`{random_field}`"],
366 | |                     distinct=True,
367 | |                     limit=1,
368 | |                 )[0]
369 | |             )[0],
    | |________________^ RUF015
370 |               random_field,
371 |           )
    |
    = help: Replace with `next(...)`

frappe/tests/test_db.py:373:4: RUF015 Prefer `next(...)` over single element slice
    |
371 |         )
372 |         self.assertEqual(
373 |             list(frappe.get_all("ToDo", fields=[f"count(`{random_field}`)"], limit=1)[0])[0],
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF015
374 |             "count" if frappe.conf.db_type == "postgres" else f"count(`{random_field}`)",
375 |         )
    |
    = help: Replace with `next(...)`

frappe/utils/redis_wrapper.py:407:15: F821 Undefined name `namedtuple`
    |
407 | CachedValue = namedtuple("CachedValue", ["value", "expiry"])
    |               ^^^^^^^^^^ F821
408 | CacheStatistics = namedtuple(
409 |     "CacheStatistics", ["hits", "misses", "capacity", "used", "utilization", "hit_ratio", "healthy"]
    |

frappe/utils/redis_wrapper.py:408:19: F821 Undefined name `namedtuple`
    |
407 | CachedValue = namedtuple("CachedValue", ["value", "expiry"])
408 | CacheStatistics = namedtuple(
    |                   ^^^^^^^^^^ F821
409 |     "CacheStatistics", ["hits", "misses", "capacity", "used", "utilization", "hit_ratio", "healthy"]
410 | )
    |

frappe/utils/redis_wrapper.py:448:15: F821 Undefined name `threading`
    |
446 |         self.local_ttl = ttl
447 |         # This guards writes to self.cache, reads are done without a lock.
448 |         self.lock = threading.RLock()
    |                     ^^^^^^^^^ F821
449 |         self.cache: dict[bytes, CachedValue] = {}
    |

frappe/utils/redis_wrapper.py:487:7: F821 Undefined name `time`
    |
485 |         try:
486 |             val = self.cache[key]
487 |             if time.monotonic() < val.expiry and self.healthy:
    |                ^^^^ F821
488 |                 self.hits += 1
489 |                 return val.value
    |

frappe/utils/redis_wrapper.py:512:53: F821 Undefined name `time`
    |
510 |             # got is invalidated, so we should not store it in local cache.
511 |             if key in self.cache:
512 |                 self.cache[key] = CachedValue(value=val, expiry=time.monotonic() + self.local_ttl)
    |                                                                 ^^^^ F821
513 |
514 |         return val
    |

frappe/utils/redis_wrapper.py:521:52: F821 Undefined name `time`
    |
519 |         self.redis.set_value(key, val, shared=True)
520 |         with self.lock:
521 |             self.cache[key] = CachedValue(value=val, expiry=time.monotonic() + self.local_ttl)
    |                                                             ^^^^ F821
522 |         # XXX: We need to tell redis that we indeed read this key we just wrote
523 |         # This is an edge case:
    |

frappe/utils/redis_wrapper.py:572:4: F821 Undefined name `time`
    |
570 |                 self.healthy = False
571 |                 raise
572 |             time.sleep(1)
    |             ^^^^ F821
573 |         else:
574 |             self.healthy = False
    |

Found 18 errors.
No fixes available (7 hidden fixes can be enabled with the `--unsafe-fixes` option).

1 file reformatted, 10 files left unchanged




Suspicious changes found (Dokos Only / dokos):
* fb546b2fb1f6dc5f93702215205cdba76e74a5ab perf: misc client cache improvements (#29070)
```diff

```



Checkout instructions
# Checkout locally
git fetch upstream
git switch ft-pr-29070

# Alternatively, re-take the changes
git switch develop
ft take ft-pr-29070

# Make changes then rebase
git rebase -i develop

# Fix or ignore conflicts
git checkout --theirs .
git rebase --continue

# Force-push changes
git push --force-with-lease

Merge request reports

Loading