sqlalchemy / sqlalchemy/dogpile.cache

Issue with decorate>=5.0.5 in dogpile.cache.region

Open
#208 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug region
Dominant language
Python
Stars
299
Forks
50
PR merge metrics
No merged PRs in 30d

Description

The latest releases of decorate changed the handling of positional args in some cases, see https://github.com/micheles/decorator/commit/04bb6454ac4f7560759ec1a3e15756a5485067ac. This is triggering failures in some consumers, e.g. openstacksdk, see https://storyboard.openstack.org/#!/story/2009114 and https://github.com/micheles/decorator/issues/127. What works for me in local unit testing is adding the kwsyntax=True option like

diff --git a/dogpile/cache/region.py b/dogpile/cache/region.py
index ef0dbc4..561e208 100644
--- a/dogpile/cache/region.py
+++ b/dogpile/cache/region.py
@@ -1619,7 +1619,7 @@ class CacheRegion:
             # Use `decorate` to preserve the signature of :param:`user_func`.
 
             return decorate(
-                user_func, partial(get_or_create_for_user_func, key_generator)
+                user_func, partial(get_or_create_for_user_func, key_generator), kwsyntax=True
             )
 
         return cache_decorator
@@ -1859,7 +1859,7 @@ class CacheRegion:
             # Use `decorate` to preserve the signature of :param:`user_func`.
 
             return decorate(
-                user_func, partial(get_or_create_for_user_func, key_generator)
+                user_func, partial(get_or_create_for_user_func, key_generator), kwsyntax=True
             )
 
         return cache_decorator

But maybe there is also a way to adopt to the new behaviour more smoothly. The above patch will break when using an old version of decorate and I don't know whether there's a better solution then wrapping in a try block and repeating the call without the added option if necessary.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in dogpile/cache/region.py at the two CacheRegion decorator calls around lines 1619 and 1859, then review the linked decorator change and the existing local unit-test coverage. Verify the behavior with current and older decorate versions, and consider the open question about preserving compatibility; done means the affected consumers no longer fail without breaking supported dependency versions.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.