-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
CSC Review Fixes - expose delete functions, rename attribute names, add AbstractCache class #3110
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #3110 +/- ##
==========================================
- Coverage 92.06% 91.74% -0.33%
==========================================
Files 128 128
Lines 33220 33320 +100
==========================================
- Hits 30583 30568 -15
- Misses 2637 2752 +115 ☔ View full report in Codecov by Sentry. |
redis/_cache.py
Outdated
@@ -160,7 +160,27 @@ class EvictionPolicy(Enum): | |||
RANDOM = "random" | |||
|
|||
|
|||
class _LocalCache: | |||
class AbstractCache: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's docstring to help our users understand why and how to use it. Acorss functions
redis/_cache.py
Outdated
@@ -160,7 +160,27 @@ class EvictionPolicy(Enum): | |||
RANDOM = "random" | |||
|
|||
|
|||
class _LocalCache: | |||
class AbstractCache: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT: Have a look at Abstract base classes, and maybe inherit from ABC?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This matches we we discussed with @vladvildanov and looks reasonable! Please consider making the cache truly abstract.
…dd AbstractCache class (#3110) * CSC review fixes * cahnge cache_max_size default value * use ABC and add docstring
…dd AbstractCache class (#3110) * CSC review fixes * cahnge cache_max_size default value * use ABC and add docstring
…dd AbstractCache class (#3110) * CSC review fixes * cahnge cache_max_size default value * use ABC and add docstring
Pull Request check-list
Please make sure to review and check all of these items:
NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.
Description of change
Please provide a description of the change here.