Skip to content

Conversation

@Mark90
Copy link
Contributor

@Mark90 Mark90 commented Apr 17, 2025

In version 1.10.0 cached_result was changed to not allow using it on a function without args/kwargs.

This was an oversight: it works perfectly fine thanks to the function name itself being prefixed in the cache key.

Re-instated that behavior, now using func.__qualname__ instead of func.__name__ to have more context in the key.

Example

class Client:

    @cached_result(...)  # without `key_name`
    async def get_foo(self):
        ...

    @cached_result(...)  # without `key_name`
    async def get_bar(self, identifier: int):
        ...

Before this PR

  • Client.get_foo: raises ValueError: Cannot generate cache key without args/kwargs")
  • Client.get_bar: generates cache keys {prefix}:{version}:get_bar:((1, ), frozenset())

After this PR

  • Client.get_foo: generates cache keys {prefix}:{version}:client.get_foo
  • Client.get_bar: generated cache keys {prefix}:{version}:client.get_bar:((1, ), frozenset())

@codecov
Copy link

codecov bot commented Apr 17, 2025

Codecov Report

Attention: Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.

Project coverage is 81.32%. Comparing base (fc02479) to head (cfe7889).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
nwastdlib/asyncio_cache.py 66.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #47      +/-   ##
==========================================
- Coverage   81.64%   81.32%   -0.32%     
==========================================
  Files          13       13              
  Lines         512      514       +2     
  Branches       70       71       +1     
==========================================
  Hits          418      418              
- Misses         73       74       +1     
- Partials       21       22       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Mark90 Mark90 force-pushed the allow-empty-args branch 2 times, most recently from a86dc1b to 79d9adf Compare April 17, 2025 09:24
@Mark90 Mark90 force-pushed the allow-empty-args branch from 2683f3a to cfe7889 Compare April 17, 2025 09:36
@Mark90 Mark90 marked this pull request as ready for review April 17, 2025 09:45
@pboers1988 pboers1988 merged commit 0d6d04d into main Apr 17, 2025
3 of 5 checks passed
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.

3 participants