Skip to content

Commit

Permalink
[DISCO-2028] Cache weather reports in Redis (#202)
Browse files Browse the repository at this point in the history
  • Loading branch information
linabutler committed Feb 24, 2023
2 parents 2cd054d + d62ff4c commit 9b00131
Show file tree
Hide file tree
Showing 21 changed files with 811 additions and 11 deletions.
14 changes: 14 additions & 0 deletions docs/ops.md
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,13 @@ Configuration for determining the location of users.
- `location.maxmind_database` (`MERINO_LOCATION__MAXMIND_DATABASE`) - Path to a
MaxMind GeoIP database file.

### [Redis](#redis)

Global Redis settings. The weather provider optionally uses Redis to cache weather suggestions.

- `redis.server` (`MERINO_REDIS__SERVER`) - The Redis server URL, in the form of
`redis://localhost:6379`.

### AccuWeather

Configuration for using AccuWeather as a weather backend
Expand Down Expand Up @@ -237,13 +244,20 @@ These are production providers that generate suggestions.
`accuweather`.
- `backend` (`MERINO_PROVIDERS__ACCUWEATHER__backend`) - The backend of the provider.
Either `accuweather` or `test`.
- `cache` (`MERINO_PROVIDERS__ACCUWEATHER__CACHE`) - The store used to cache weather reports.
Either `redis` or `none`. If `redis`, the [global Redis settings](#redis) must be set.
Defaults to `none`.
- `enabled_by_default` (`MERINO_PROVIDERS__ACCUWEATHER__ENABLED_BY_DEFAULT`) - Whether
this provider is enabled by default.
- `score` (`MERINO_PROVIDERS__ACCUWEATHER__SCORE`) - The ranking score for this provider
as a floating point number. Defaults to 0.3.
- `query_timeout_sec` (`MERINO_PROVIDERS__ACCUWEATHER__QUERY_TIMEOUT_SEC`) - A floating
point (in seconds) indicating the maximum waiting period when Merino queries
for weather forecasts. This will override the default query timeout.
- `cached_report_ttl_sec` (`MERINO_PROVIDERS__ACCUWEATHER__CACHED_REPORT_TTL_SEC`) - The
number of whole seconds (as an integer) indicating the time-to-live for a cached weather
report (current conditions and forecast) for a location. After the TTL expires, the provider
will fetch and cache the report again on the next suggestion request for that location.

#### Top Picks Provider
- Top Picks - Provides suggestions from a static domain list of the 1000 most visited websites.
Expand Down
1 change: 1 addition & 0 deletions merino/cache/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""Caching support."""
22 changes: 22 additions & 0 deletions merino/cache/none.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
"""No-operation adapter that disables caching."""

from datetime import timedelta
from typing import Optional, Union


class NoCacheAdapter: # pragma: no cover
"""A cache adapter that doesn't store or return anything."""

async def get(self, key: str) -> Optional[bytes]: # noqa: D102
return None

async def set(
self,
key: str,
value: Union[bytes, str],
ttl: Optional[timedelta] = None,
) -> None: # noqa: D102
pass

async def close(self) -> None: # noqa: D102
pass
33 changes: 33 additions & 0 deletions merino/cache/protocol.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
"""Protocol for cache adapters."""

from datetime import timedelta
from typing import Optional, Protocol


class CacheAdapter(Protocol):
"""A protocol describing a cache backend."""

async def get(self, key: str) -> Optional[bytes]: # pragma: no cover
"""Get the value associated with the key. Returns `None` if the key isn't in the cache.
Raises:
- `CacheAdapterError` for cache backend errors.
"""
...

async def set(
self,
key: str,
value: bytes,
ttl: Optional[timedelta] = None,
) -> None: # pragma: no cover
"""Store a key-value pair in the cache, with an optional time-to-live.
Raises:
- `CacheAdapterError` for cache backend errors.
"""
...

async def close(self) -> None: # pragma: no cover
"""Close the adapter and release any underlying resources."""
...
54 changes: 54 additions & 0 deletions merino/cache/redis.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
"""Redis cache adapter."""

from datetime import timedelta
from typing import Optional

from redis.asyncio import Redis, RedisError

from merino.exceptions import CacheAdapterError


class RedisAdapter:
"""A cache adapter that stores key-value pairs in Redis."""

redis: Redis

def __init__(self, redis: Redis):
self.redis = redis

async def get(self, key: str) -> Optional[bytes]:
"""Get the value associated with the key from Redis. Returns `None` if the key isn't in
Redis.
Raises:
- `CacheAdapterError` if Redis returns an error.
"""
try:
return await self.redis.get(key)
except RedisError as exc:
raise CacheAdapterError(
f"Failed to get `{repr(key)}` with error: `{exc}`"
) from exc

async def set(
self,
key: str,
value: bytes,
ttl: Optional[timedelta] = None,
) -> None:
"""Store a key-value pair in Redis, overwriting the previous value if set, and optionally
expiring after the time-to-live.
Raises:
- `CacheAdapterError` if Redis returns an error.
"""
try:
await self.redis.set(key, value, ex=ttl.seconds if ttl else None)
except RedisError as exc:
raise CacheAdapterError(
f"Failed to set `{repr(key)}` with error: `{exc}`"
) from exc

async def close(self) -> None:
"""Close the Redis connection."""
await self.redis.close()
9 changes: 9 additions & 0 deletions merino/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,21 @@
Validator("metrics.host", is_type_of=str),
Validator("metrics.port", gte=0, is_type_of=int),
Validator("providers.accuweather.enabled_by_default", is_type_of=bool),
# The Redis server URL is required when at least one provider wants to use Redis for caching.
Validator(
"redis.server",
is_type_of=str,
must_exist=True,
when=Validator("providers.accuweather.cache", must_exist=True, eq="redis"),
),
# Set the upper bound of query timeout to 5 seconds as we don't want Merino
# to wait for responses from Accuweather indefinitely.
Validator(
"providers.accuweather.query_timeout_sec", is_type_of=float, gte=0, lte=5.0
),
Validator("providers.accuweather.type", is_type_of=str, must_exist=True),
Validator("providers.accuweather.cache", is_in=["redis", "none"]),
Validator("providers.accuweather.cached_report_ttl_sec", is_type_of=int, gte=0),
Validator("providers.adm.backend", is_in=["remote-settings", "test"]),
Validator("providers.adm.cron_interval_sec", gt=0),
Validator("providers.adm.enabled_by_default", is_type_of=bool),
Expand Down
2 changes: 2 additions & 0 deletions merino/configs/default.toml
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,11 @@ traces_sample_rate = 0.1
[default.providers.accuweather]
type = "accuweather"
backend = "accuweather"
cache = "none"
enabled_by_default = false
score = 0.3
query_timeout_sec = 5.0
cached_report_ttl_sec = 7200 # 2 hours.

[default.accuweather]
# Our API key used to access the AccuWeather API.
Expand Down
2 changes: 1 addition & 1 deletion merino/configs/development.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ dev_logger = true
level = "DEBUG"
format = "pretty"

[development.providers.accuweather]
[development.accuweather]
api_key = "test"

[development.jobs.wikipedia_indexer]
Expand Down
18 changes: 18 additions & 0 deletions merino/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,21 @@ class InvalidProviderError(Exception):
"""Raised when an unknown provider encountered."""

pass


class CacheAdapterError(Exception):
"""Exception raised when a cache adapter operation fails."""

pass


class CacheEntryError(ValueError):
"""Exception raised for cache entries that can't be deserialized."""

pass


class CacheMissError(Exception):
"""Exception raised if an entry doesn't exist in the cache."""

pass
9 changes: 9 additions & 0 deletions merino/providers/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
from enum import Enum, unique

from dynaconf.base import Settings
from redis.asyncio import Redis

from merino.cache.none import NoCacheAdapter
from merino.cache.redis import RedisAdapter
from merino.config import settings
from merino.exceptions import InvalidProviderError
from merino.providers.adm.backends.fake_backends import FakeAdmBackend
Expand Down Expand Up @@ -52,9 +55,15 @@ def _create_provider(provider_id: str, setting: Settings) -> BaseProvider:
) # type: ignore [arg-type]
if setting.backend == "accuweather"
else FakeWeatherBackend(),
cache=RedisAdapter(
Redis.from_url(settings.redis.server)
) # type: ignore [arg-type]
if setting.cache == "redis"
else NoCacheAdapter(),
score=setting.score,
name=provider_id,
query_timeout_sec=setting.query_timeout_sec,
cached_report_ttl_sec=setting.cached_report_ttl_sec,
enabled_by_default=setting.enabled_by_default,
)
case ProviderType.ADM:
Expand Down
9 changes: 9 additions & 0 deletions merino/providers/weather/backends/accuweather.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,15 @@ def __init__(
self.url_current_conditions_path = url_current_conditions_path
self.url_forecasts_path = url_forecasts_path

def cache_inputs_for_weather_report(self, geolocation: Location) -> Optional[bytes]:
"""Return the inputs used to form the cache key for looking up and storing the current
conditions and forecast for a location.
"""
if geolocation.country is None or geolocation.postal_code is None:
return None

return (geolocation.country + geolocation.postal_code).encode("utf-8")

async def get_weather_report(
self, geolocation: Location
) -> Optional[WeatherReport]:
Expand Down
4 changes: 4 additions & 0 deletions merino/providers/weather/backends/fake_backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
class FakeWeatherBackend:
"""A fake backend that always returns empty results."""

def cache_inputs_for_weather_report(self, geolocation: Location) -> Optional[bytes]:
"""Doesn't return any cache key inputs."""
return None

async def get_weather_report(
self, geolocation: Location
) -> Optional[WeatherReport]:
Expand Down
8 changes: 8 additions & 0 deletions merino/providers/weather/backends/protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ class WeatherBackend(Protocol):
directly depend on.
"""

def cache_inputs_for_weather_report(
self, geolocation: Location
) -> Optional[bytes]: # pragma: no cover
"""Return the inputs used to form the cache key for looking up and storing weather
information for a location.
"""
...

async def get_weather_report(
self, geolocation: Location
) -> Optional[WeatherReport]: # pragma: no cover
Expand Down
Loading

0 comments on commit 9b00131

Please sign in to comment.