Skip to content

Commit

Permalink
minor fix: better error reporting when type to cache isn't supported …
Browse files Browse the repository at this point in the history
…+ test
  • Loading branch information
karlicoss committed Mar 20, 2021
1 parent a413b1f commit 0aecf1d
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 17 deletions.
23 changes: 12 additions & 11 deletions src/cachew/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import typing
from pkg_resources import get_distribution, DistributionNotFound

try:
Expand All @@ -23,6 +22,8 @@
import stat
import tempfile
from pathlib import Path
import time
import sqlite3
import sys
import typing
from typing import (Any, Callable, Iterator, List, NamedTuple, Optional, Tuple,
Expand Down Expand Up @@ -427,7 +428,10 @@ def make(tp: Type, name: Optional[str]=None) -> 'NTBinder':
fields = ()
span = 1
else:
fields = tuple(NTBinder.make(tp=ann, name=fname) for fname, ann in tp.__annotations__.items())
annotations = getattr(tp, '__annotations__', None)
if annotations is None:
raise CachewException(f"{tp}: doesn't look like a supported type to cache. See https://github.com/karlicoss/cachew#features for the list of supported types.")
fields = tuple(NTBinder.make(tp=ann, name=fname) for fname, ann in annotations.items())
span = sum(f.span for f in fields) + (1 if optional else 0)
return NTBinder(
name=name,
Expand Down Expand Up @@ -557,9 +561,6 @@ def __init__(self, db_path: Path, cls: Type) -> None:
# NOTE: timeout is necessary so we don't lose time waiting during recursive calls
# by default, it's several seconds? you'd see 'test_recursive' test performance degrade

import sqlite3
import time

@event.listens_for(self.engine, 'connect')
def set_sqlite_pragma(dbapi_connection, connection_record):
# without wal, concurrent reading/writing is not gonna work
Expand Down Expand Up @@ -676,7 +677,7 @@ def bail(reason):
if is_union(arg):
return arg # meh?
if not is_dataclassish(arg):
return bail(f"{arg} is not NamedTuple")
return bail(f"{arg} is not NamedTuple/dataclass")
return arg


Expand All @@ -702,7 +703,7 @@ def cachew_error(e: Exception) -> None:
else:
logger = get_logger()
# todo add func name?
logger.error("Error while setting up cache, falling back to non-cached version")
logger.error("cachew: error while setting up cache, falling back to non-cached version")
logger.exception(e)


Expand Down Expand Up @@ -776,7 +777,7 @@ def cachew(
cn = cname(func)
# todo not very nice that ENABLE check is scattered across two places
if not settings.ENABLE or cache_path is None:
logger.info('[%s]: cache disabled', cn)
logger.info('[%s]: cache explicitly disabled (settings.ENABLE is False or cache_path is None)', cn)
return func

if cache_path is use_default_path:
Expand All @@ -786,7 +787,7 @@ def cachew(
# TODO fuzz infer_type, should never crash?
inferred = infer_type(func)
if isinstance(inferred, Failure):
msg = f"failed to infer cache type: {inferred}"
msg = f"failed to infer cache type: {inferred}. See https://github.com/karlicoss/cachew#features for the list of supported types."
if cls is None:
ex = CachewException(msg)
cachew_error(ex)
Expand Down Expand Up @@ -873,7 +874,7 @@ def cachew_wrapper(

cn = cname(func)
if not settings.ENABLE:
logger.info('[%s]: cache disabled', cn)
logger.info('[%s]: cache explicitly disabled (settings.ENABLE is False)', cn)
yield from func(*args, **kwargs)
return

Expand All @@ -887,7 +888,7 @@ def cachew_wrapper(
if callable(cache_path):
pp = cache_path(*args, **kwargs) # type: ignore
if pp is None:
logger.info('[%s]: cache disabled', cn)
logger.info('[%s]: cache explicitly disabled (cache_path is None)', cn)
yield from func(*args, **kwargs)
return
else:
Expand Down
56 changes: 50 additions & 6 deletions src/cachew/tests/test_cachew.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@

@pytest.fixture(autouse=True)
def throw_on_errors():
# a more reasonable default for tests
# NOTE: in tests we always throw on errors, it's a more reasonable default for testing.
# we still check defensive behaviour in test_defensive
settings.THROW_ON_ERROR = True
yield

Expand All @@ -40,7 +41,7 @@ def restore_settings():

@pytest.fixture
def tdir(tmp_path):
# TODO just use tmp_path instead?
# todo just use tmp_path instead?
yield Path(tmp_path)


Expand Down Expand Up @@ -191,7 +192,12 @@ def fun() -> Iterator[str]:
assert list(fun()) == ['string1', 'string2']


def test_cache_path(tdir):
def test_cache_path(tmp_path: Path) -> None:
'''
Tests various ways of specifying cache path
'''
tdir = tmp_path

calls = 0
def orig() -> Iterable[int]:
nonlocal calls
Expand Down Expand Up @@ -247,6 +253,29 @@ def orig() -> Iterable[int]:
# on the other hand, wouldn't want to delete some user file by accident


class UGood(NamedTuple):
x: int
class UBad:
pass

def test_unsupported_class(tmp_path: Path) -> None:
with pytest.raises(CachewException, match='.*failed to infer cache type.*'):
@cachew(cache_path=tmp_path)
def fun() -> List[UBad]:
return [UBad()]

# now something a bit nastier
# TODO hmm, should really throw at the definition time... but can fix later I suppose
@cachew(cache_path=tmp_path)
def fun2() -> Iterable[Union[UGood, UBad]]:
yield UGood(x=1)
yield UBad()
yield UGood(x=2)

with pytest.raises(CachewException, match=".*doesn't look like a supported type.*"):
list(fun2())


class TE2(NamedTuple):
value: int
uuu: UUU
Expand Down Expand Up @@ -535,13 +564,13 @@ def get_people_data() -> Iterator[Person]:
print(f"Cache db size for {N} entries: estimated size {one * N // 1024} Kb, actual size {cache_file.stat().st_size // 1024} Kb;")


def test_dataclass(tdir):
def test_dataclass(tmp_path: Path) -> None:
from dataclasses import dataclass
@dataclass
class Test:
field: int

@cachew(tdir)
@cachew(tmp_path)
def get_dataclasses() -> Iterator[Test]:
yield from [Test(field=i) for i in range(5)]

Expand Down Expand Up @@ -706,7 +735,7 @@ def orig(a: int, param=hh) -> Iterator[O]:
class U(NamedTuple):
x: Union[str, O]

def test_union(tmp_path: Path):
def test_union(tmp_path: Path) -> None:
@cachew(tmp_path)
def fun() -> Iterator[U]:
yield U('hi')
Expand All @@ -716,6 +745,21 @@ def fun() -> Iterator[U]:
assert list(fun()) == [U('hi'), U(O(123))]


def test_union_with_dataclass(tmp_path: Path) -> None:
from dataclasses import dataclass
# NOTE: empty dataclass doesn't have __annotations__ ??? not sure if need to handle it...
@dataclass
class DD:
x: int

@cachew(tmp_path)
def fun() -> Iterator[Union[int, DD]]:
yield 123
yield DD(456)

assert list(fun()) == [123, DD(456)]


def _concurrent_helper(cache_path: Path, count: int, sleep_s=0.1):
from time import sleep
@cachew(cache_path)
Expand Down

0 comments on commit 0aecf1d

Please sign in to comment.