Decorator namespace key

[padraic-shafer]

What do these changes do?

Keys with namespaces now work for decorators get/set

• Include build_key(key, namespace) in decorators.cached:
  • get_from_cache()
  • set_in_cache()

Are there changes in behavior for the user?

• Keys with namespaces now work for the @cached() decorator

Related issue number

• build_key(key, namespace) is missing from some modules/classes #569

Checklist

• [X ] I think the code is well written
• [X ] Unit tests for the changes exist
• [X ] Documentation reflects the changes
• Add a new news fragment into the CHANGES folder
  • name it <issue_id>.<type> (e.g. 588.bugfix)
  • if you don't have an issue_id change it to the pr id after creating the PR
  • ensure type is one of the following:
    • .feature: Signifying a new feature.
    • .bugfix: Signifying a bug fix.
    • .doc: Signifying a documentation improvement.
    • .removal: Signifying a deprecation or removal of public API.
    • .misc: A ticket has been closed, but it is not of interest to users.
  • Make sure to use full sentences with correct case and punctuation, for example: Fix issue with non-ascii contents in doctest text files.

[Dreamsorcerer]

Tests are failing.

[padraic-shafer]

Tests are failing.

Noted. Changes are in progress now...I'm planning to push the corrections within the next hour.

[Dreamsorcerer]

Nope, still failing. You're not testing on 3.7 are you? Because only half the tests run on 3.7.

[padraic-shafer]

I've now updated all decorators to use the namespace if provided.

To make this work, I updated TestCachedStampede.test_cached_stampede to use the namespace in the call signature of the mock cache.get() and cache.set().

This is the intended behavior after the changes I made in this branch to the decorators.cached. But I don't know whether there was a good reason to explicitly exclude the namespace in this test previously.

I made similar changes in tests/ut/ so that mock_cache uses namespace="test".

[Dreamsorcerer]

Looks like we have what we need to make a new release now. Think you can finish this off in the next couple of days?

[padraic-shafer]

Looks like we have what we need to make a new release now. Think you can finish this off in the next couple of days?

Yes, I can make that work.

[padraic-shafer]

In reference to this comment in Issue #569, here is a summary of the responses included in this PR.

1. N/A
2. ...
   a. See examples/alt_key_builder.py and updates to examples/cached_decorator.py
   b. BaseCache.build_key uses namespace argument if provided, otherwise it uses self.namespace.
   c. Cache locks use client.build_key rather than client._build_key.
3. ...
   a. Deferred. This might get submitted as a separate issue / PR at a later time.
   b. namespace is an explicit parameter to decorator caches.
4. Deferred. This might get submitted as a separate issue / PR at a later time.
5. Warn when decorator parameters are unused because alias takes precedence.
6. Deferred. This might get submitted as a separate issue / PR at a later time.
7. N/A

[padraic-shafer]

@Dreamsorcerer I think this is ready to go, but let me know if you see additional changes needed.

[padraic-shafer]

All comments appear to be resolved

[padraic-shafer]

@Dreamsorcerer Please have another look, and let me know if all is good.

[Dreamsorcerer]

Now, if you're happy to do some more, it would be great if you can create a PR which reverts all the extra namespace conversions in here and does it something like this:

def __init__(self, key_builder):
    self._build_key = key_builder or lambda k, ns: "{}{}".format(ns or "", k)

def build_key(self, key, namespace=None):
    return self._build_key(key, namespace or self.namespace)

Then we can do it like this for v1 and introduce some backwards incompatible changes.

Should also be able to fold in the _ensure_key() logic and remove that function. Plus, MemcachedBackend can do the .encode() in build_key() as well.

In other words, the key_builder should not need to know about any of these edge cases, it should just receive a str and return a str.

[padraic-shafer]

Nice! I like those ideas.

I should have some time over the next couple weeks to give this a try.

[Dreamsorcerer]

@pshafer-als Any chance you'll get round to that soon?

[padraic-shafer]

That's a fair question. Realistically, I think it will be ~2 weeks before I can secure sufficient time to work on this in earnest. I think it will be a few hours of work, but probably spread over a 1-2 week period.

If you need to move on, I understand. Otherwise, I'm still interested.

[Dreamsorcerer]

That's good, just making sure the task doesn't get lost.

[padraic-shafer]

Started a branch for this work, and a new thread for discussion.