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

Callable timeout #401

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

thejeremyjohn
Copy link

Changes:

  • added support for callable timeout in @cached and @memoized
  • modified try-except import strategy re pytest-xprocess

A callable timeout serves a similar purpose to CachedResponse (introduced in PR #296) but does not replace that functionality entirely. These are the differences:

callable timeout CachedResponse
@cached OR @memoized @cached
Any rv type Response-like rv type
only knows the decorated function's output knows all locals within the decorated function

A CachedResponse always takes priority over callable timeout if it is the sole rv type so there's no point in using both in that case. However if the rv type is conditional they could be complimentary.

NOTE: Prior to this PR, tests/test_backend_cache.py::TestRedisCache::test_generic_inc_dec are skipped if pytest-xprocess can't be imported but with that import fixed (1st commit in this PR), they actually fail. (I did not investigate further. FWIW I'm running Redis version=7.0.0) That is why the below box is unchecked, but all other tests pass.

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@northernSage
Copy link
Member

Hey @thejeremyjohn, thanks for the contribution! It looks like we are failing some tests, could you please take a look?

@thejeremyjohn
Copy link
Author

Hi @northernSage. Yw -- I'm glad to do it! Yeah, that's consistent with what I noted in the PR. Whatever is failing there is not related to the callable timeout changes.

It's only tripping because those tests are running rather than being skipped due to pytest-xprocess not importing, which I addressed in the first commit in this PR. I could undo that such that those tests are again skipped; that's no problem for me since it's not connected to the callable timeout enhancement.

@thejeremyjohn
Copy link
Author

@northernSage Please let me know if that ^^^ is the preferred way to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants