Skip to content
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

Introduce skip_cache_func #652

Merged
merged 11 commits into from
Feb 9, 2023

Conversation

VitalyPetrov
Copy link
Contributor

@VitalyPetrov VitalyPetrov commented Jan 20, 2023

  • add an optional value_checker param to cache decorator classes cached, cached_stampede and multi_cached
  • add IDE-specific folders to gitignore
  • add black formatter to `requirements-dev.txt

Are there changes in behavior for the user?

Did not test in real production code

Related issue number

Fixes #649

Checklist

  • [+] I think the code is well written
  • [-] Unit tests for the changes exist
  • [+] Documentation reflects the changes
  • [-] Add a new news fragment into the CHANGES folder

@VitalyPetrov
Copy link
Contributor Author

Unit tests for that functionality is not done yet.
As for notes in CHANGES the PR seems to be too small to be released separately IMO. But the choice Is yours (maintainers).

Copy link
Member

@Dreamsorcerer Dreamsorcerer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

A couple of suggestions there, and we'll also need some tests for the new behaviour (maybe in tests/acceptance/tests_decorator.py so it gets tested against the different backends).

.gitignore Outdated Show resolved Hide resolved
aiocache/decorators.py Outdated Show resolved Hide resolved
aiocache/decorators.py Outdated Show resolved Hide resolved
aiocache/decorators.py Outdated Show resolved Hide resolved
aiocache/decorators.py Outdated Show resolved Hide resolved
requirements-dev.txt Outdated Show resolved Hide resolved
aiocache/decorators.py Outdated Show resolved Hide resolved
@Dreamsorcerer
Copy link
Member

Ignore the news fragment. I'm not sure where that checklist comes from, trying to see if I can edit it...

@VitalyPetrov
Copy link
Contributor Author

Updated the code, please check

@codecov
Copy link

codecov bot commented Jan 25, 2023

Codecov Report

Merging #652 (2e24baf) into master (0cedbb2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #652   +/-   ##
=======================================
  Coverage   99.67%   99.67%           
=======================================
  Files          35       35           
  Lines        3707     3743   +36     
=======================================
+ Hits         3695     3731   +36     
  Misses         12       12           
Flag Coverage Δ
unit 99.67% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
aiocache/decorators.py 100.00% <100.00%> (ø)
tests/acceptance/test_decorators.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c916bab...2e24baf. Read the comment docs.

@Dreamsorcerer
Copy link
Member

OK, looking good. If we can just get a couple of tests in (maybe in tests/acceptance/tests_decorator.py?), then we're good to merge.

@VitalyPetrov
Copy link
Contributor Author

Done with tests

@Dreamsorcerer Dreamsorcerer changed the title Feature/value checker Introduce skip_cache_func Feb 8, 2023
@Dreamsorcerer
Copy link
Member

Added cached_stampede to the tests to get full coverage, all good now.

Thanks a lot for that.

@Dreamsorcerer Dreamsorcerer merged commit 7c80a0f into aio-libs:master Feb 9, 2023
Dreamsorcerer added a commit that referenced this pull request Feb 9, 2023
Co-authored-by: Vitaly Petrov <[email protected]>
Co-authored-by: Sam Bull <[email protected]>
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.

Block writing cache in cached decorator if custom condition is not satisfied
2 participants