-
Notifications
You must be signed in to change notification settings - Fork 158
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
Simplify cache namespace and key encoding logic for v1 #669
Comments
Oh, right. That's because this test is making a bare call to My hunch is that this will break the ability to override the default namespace in a cache decorator that is backed by an I'll need to look into how best to resolve this. |
After preliminary review, I think it will be ok to proceed as started in this branch. The other project I mentioned subclasses the Regarding the RedLock and backend implementations, even where namespace is not explicitly provided, I think it makes sense to fall back on the cache's
I'm inclined to modify this test to remove this pathological behavior, where a custom TL;DR I'm moving forward with the originally suggested changes. |
Yes, I think we want to have the default namespace, so changing this behaviour seems fine. The test should still pass in the 3rd paramater, where a non-empty namespace is used. We could also consider checking for |
Moving the namespace logic into a single function (
BaseCache.buildkey()
rather thanBaseCache.add()
,get()
,etc.) makes a lot of sense.This change causes tests/ut/test_base.py::TestBaseCache::test_alt_build_key_override_namespace[None-key] to fail: (
AssertionError: assert 'test:key' == 'key'
). This makes sense because the namespace gets overridden byself.namespace
before the customkey_builder
gets called. The strange part is that I think this test should have already failed before I made these changes.I'll dig into that and the knock-on impacts to backends and decorators in a few days.
The text was updated successfully, but these errors were encountered: