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

rename EvictionCause to RemovalCause #33

Merged
merged 2 commits into from
Sep 29, 2023

Conversation

uncle-lv
Copy link
Contributor

In most caches(rediscaffeine and etc.), eviction means the entry is removed due to maxsize limit. But on_delete callback will be executed in all removal cases. EvictionCause can lead to a misunderstanding, RemovalCause should be a better name.

@coveralls
Copy link

coveralls commented Sep 26, 2023

Coverage Status

coverage: 100.0%. remained the same when pulling 8a9cdca on uncle-lv:rename-EvictionCause into 51c88ba on dgilland:master.

@uncle-lv
Copy link
Contributor Author

Another reason is expired_count evicted_count deleted_count will be three different figures in statistics. But EvictionCause includes all three cases, what can lead to a misunderstanding too.

Copy link
Owner

@dgilland dgilland left a comment

Choose a reason for hiding this comment

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

Just a minor nit to align the docstring of the class with the class rename.

@@ -24,7 +24,7 @@
UNSET = object()


class EvictionCause(Enum):
class RemovalCause(Enum):
"""
An enum to represent the cause for the eviction of a cache entry.
Copy link
Owner

Choose a reason for hiding this comment

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

nit: Can replace "eviction" with "removal".

Suggested change
An enum to represent the cause for the eviction of a cache entry.
An enum to represent the cause for the removal of a cache entry.

@dgilland dgilland merged commit 0c31ca7 into dgilland:master Sep 29, 2023
6 checks passed
@uncle-lv
Copy link
Contributor Author

Updated usage.

Usage

set on_delete callback.

cache = Cache(on_delete=on_delete)

on_delete is a callable object.

Callable[[key: Hashable, value: Any, cause: RemovalCause],  None]

First parameter is the key of cache entry has been deleted.

Second is the value.

Third is the cause of cache entry removal.

RemovalCause is an enum.

class RemovalCause(Enum):
    DELETE = auto()
    SET = auto()
    EXPIRED = auto()
    FULL = auto()
    POPITEM = auto()
  • DELETE: indicates that the cache entry was deleted by delete() or delete_many() explicitly.
  • SET: indicates that the cache entry was replaced with a new value by set() or set_many().
  • EXPIRED: indicates that the cache entry was removed because it expired.
  • FULL: indicates that the cache entry was removed because cache has been full (reached the maximum size limit).
  • POPITEM: indicates that the cache entry was deleted by popitem().

Trigger

The table shows methods will trigger on_delete.

cause trigger
DELETE The cache entry was removed explicitly by delete() or delete_many()
SET The cache entry was replaced by set() or set_many()
POPITEM The cache entry was deleted by popitem()
EXPIRED Implicit. When the cache entry was removed because it expired
FULL Implicit. The cache entry was removed because cache has reached the maximum size limit

clear() never triggers on_delete.

@uncle-lv uncle-lv deleted the rename-EvictionCause branch October 3, 2023 13:22
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.

3 participants