Fix cache_memoize to work with non-sequence data.

Review Request #11856 — Created Oct. 17, 2021 and submitted

david
Djblets
release-2.x
11857
djblets

The size checks in cache_memoize were causing it to fail when the
cached function was returning something other than a sequence. This
change fixes it so we only do those length checks for string types.

While I was in here I fixed up a few small style issues.

  • Used with a change that required this.
  • Ran unit tests.
Summary
Fix cache_memoize to work with non-sequence data.
Description From Last Updated

E501 line too long (80 > 79 characters)

reviewbotreviewbot

We can't get rid of use_generator without a major version bump. Let's leave it in and put up a deprecation ...

chipx86chipx86

Can you add a "Version Changed" here to notify that non-string/list types are now supported?

chipx86chipx86

This comment is old and wrong about some details, though the general problem remains. Maybe worth updating? Looking at the ...

chipx86chipx86

While here, can you change this to unicode?

chipx86chipx86

Same here.

chipx86chipx86

Minor thing, but want to change these as well to be cache_func and cache_called?

chipx86chipx86
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

david
chipx86
  1. 
      
  2. djblets/cache/backend.py (Diff revision 2)
     
     

    We can't get rid of use_generator without a major version bump. Let's leave it in and put up a deprecation notice in the docs and call, schedule it for removal in 3.0.

  3. djblets/cache/backend.py (Diff revision 2)
     
     
     
     

    Can you add a "Version Changed" here to notify that non-string/list types are now supported?

  4. djblets/cache/backend.py (Diff revision 2)
     
     
     
     
     

    This comment is old and wrong about some details, though the general problem remains. Maybe worth updating?

    Looking at the code, it's not so much that python-memcached/pylibmc fails silently anymore, or even has an exception for this. It actually returns a result. We just don't know about it. Django, however, does, and clears out the key.

    (Interestingly, pylibmc attempts to compress the data to make it fit, before deciding whether to give up.)

    Original problem remains: We don't know, we can't find out through setting the data, and we still have to check. As the comment says, it's still not good enough. Pylibmc checks against encoded UTF-8 data, for strings, and Pickled data for anything not otherwise handled (non-ints/longs/strings).

    Crappy situation. I really wish Django returned a result or something for us. It'd be easier to more effectively support other backends, too.

    1. Tried to make the comment clearer.

  5. djblets/cache/backend.py (Diff revision 2)
     
     

    While here, can you change this to unicode?

  6. djblets/cache/backend.py (Diff revision 2)
     
     

    Same here.

  7. djblets/cache/tests/test_backend.py (Diff revision 2)
     
     
     
     

    Minor thing, but want to change these as well to be cache_func and cache_called?

  8. 
      
david
chipx86
  1. Ship It!
  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.x (9a078ad)
Loading...