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

Better support for sub-classed storage engines #10

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

anentropic
Copy link
Contributor

@anentropic anentropic commented Oct 22, 2018

We wanted to make a Storage sub-class that uses Django cache backend rather than in-memory dict.

So we wanted StorageBase to actually implement the token bucket algorithm so as not to just copy and paste all the code from MemoryStorage into our own sub-class.

The changes in this PR allowed us to make a Django cache storage class that looks like:

from django.core.cache import cache
from token_bucket import StorageBase


class DjangoCacheBucketProvider(object):

    def __init__(self, key_template='token_bucket_{key}'):
        self.key_template = key_template

    def _key(self, key):
        return self.key_template.format(key=key)

    def __getitem__(self, key):
        val = cache.get(self._key(key))
        if val is None:
            raise KeyError(key)
        return val

    def __setitem__(self, key, val):
        cache.set(
            self._key(key),
            val
        )


class DjangoCacheStorage(StorageBase):

    bucket_provider_cls = DjangoCacheBucketProvider

@codecov
Copy link

codecov bot commented Nov 1, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@61f4e63). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #10   +/-   ##
=========================================
  Coverage          ?   98.57%           
=========================================
  Files             ?        8           
  Lines             ?      211           
  Branches          ?        0           
=========================================
  Hits              ?      208           
  Misses            ?        3           
  Partials          ?        0
Impacted Files Coverage Δ
token_bucket/storage_base.py 100% <100%> (ø)
token_bucket/storage.py 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 61f4e63...158324f. Read the comment docs.

@vytas7
Copy link
Member

vytas7 commented Jun 30, 2021

Hi @anentropic !
And sorry that we're not checking this repository too often.

You bring a very valid point here; indeed, it would make sense to decouple storage from the algorithm itself.

I'm planning to release 0.3.0 featuring other cleanup and minor fixes/improvements as is, and then circle back on decoupling storage.
Ideally, it would be good to get an async story for token_bucket too while at it 😈

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.

2 participants