Skip to content

Commit

Permalink
refactor: Use functools.lru_cache instead of the stale `memoization…
Browse files Browse the repository at this point in the history
…` library (#1981)
  • Loading branch information
edgarrmondragon committed Jan 16, 2024
1 parent c8e227b commit 8377f11
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 31 deletions.
12 changes: 1 addition & 11 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ inflection = ">=0.5.1"
joblib = ">=1.0.1"
jsonpath-ng = ">=1.5.3"
jsonschema = ">=4.16.0"
memoization = { version = ">=0.3.2,<0.5.0", python = "<4" }
packaging = ">=23.1"
pendulum = [
{ version = ">=2.1.0,<3", python = "<3.8" },
Expand Down
10 changes: 5 additions & 5 deletions singer_sdk/helpers/_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@
import typing as t
from copy import deepcopy

from memoization import cached

from singer_sdk.helpers._typing import is_object_type

if t.TYPE_CHECKING:
from logging import Logger

from singer_sdk._singerlib import Catalog, SelectionMask

_MAX_LRU_CACHE = 500


@cached(max_size=_MAX_LRU_CACHE)
# TODO: this was previously cached using the `memoization` library. However, the
# `functools.lru_cache` decorator does not support non-hashable arguments.
# It is possible that this function is not a bottleneck, but if it is, we should
# consider implementing a custom LRU cache decorator that supports non-hashable
# arguments.
def get_selected_schema(
stream_name: str,
schema: dict,
Expand Down
4 changes: 2 additions & 2 deletions singer_sdk/helpers/jsonpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

import logging
import typing as t
from functools import lru_cache

import memoization
from jsonpath_ng.ext import parse

if t.TYPE_CHECKING:
Expand Down Expand Up @@ -39,7 +39,7 @@ def extract_jsonpath(
yield match.value


@memoization.cached
@lru_cache
def _compile_jsonpath(expression: str) -> jsonpath_ng.JSONPath:
"""Parse a JSONPath expression and cache the result.
Expand Down
5 changes: 2 additions & 3 deletions tests/core/rest/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
from __future__ import annotations

import typing as t
from functools import cached_property

import pytest
from memoization.memoization import cached
from requests.auth import HTTPProxyAuth

from singer_sdk.authenticators import APIAuthenticatorBase, SingletonMeta
Expand Down Expand Up @@ -49,8 +49,7 @@ class NaiveAuthenticator(APIAuthenticatorBase):
class CachedAuthStream(SimpleRESTStream):
"""A stream with Naive authentication."""

@property
@cached
@cached_property
def authenticator(self) -> NaiveAuthenticator:
"""Stream authenticator."""
return NaiveAuthenticator(stream=self)
Expand Down
9 changes: 0 additions & 9 deletions tests/core/test_mapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

from singer_sdk._singerlib import Catalog
from singer_sdk.exceptions import MapExpressionError
from singer_sdk.helpers._catalog import get_selected_schema
from singer_sdk.mapper import PluginMapper, RemoveRecordTransform, md5
from singer_sdk.streams.core import Stream
from singer_sdk.tap_base import Tap
Expand Down Expand Up @@ -563,19 +562,11 @@ def discover_streams(self):
return [MappedStream(self)]


@pytest.fixture
def _clear_schema_cache() -> None:
"""Schemas are cached, so the cache needs to be cleared between test invocations."""
yield
get_selected_schema.cache_clear()


@time_machine.travel(
datetime.datetime(2022, 1, 1, tzinfo=datetime.timezone.utc),
tick=False,
)
@pytest.mark.snapshot()
@pytest.mark.usefixtures("_clear_schema_cache")
@pytest.mark.parametrize(
"stream_maps,flatten,flatten_max_depth,snapshot_name",
[
Expand Down

0 comments on commit 8377f11

Please sign in to comment.