feat: Add Redis caching for navigation pagination #488
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,4 +117,4 @@ docs-image: | |
| .PHONY: docs | ||
| docs: docs-image | ||
| docker compose -f compose.docs.yml \ | ||
| run docs | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,152 @@ | ||
| """Utilities for connecting to and managing Redis connections.""" | ||
|
|
||
| import logging | ||
| from typing import Optional | ||
|
|
||
| from pydantic_settings import BaseSettings | ||
| from redis import asyncio as aioredis | ||
| from redis.asyncio.sentinel import Sentinel | ||
|
|
||
| redis_pool: Optional[aioredis.Redis] = None | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class RedisSentinelSettings(BaseSettings): | ||
| """Configuration for connecting to Redis Sentinel.""" | ||
|
|
||
| REDIS_SENTINEL_HOSTS: str = "" | ||
| REDIS_SENTINEL_PORTS: str = "26379" | ||
|
There was a problem hiding this comment. These should be lists of strings, along with validators. Validation should be in this class, not in the functions where we use this class. |
||
| REDIS_SENTINEL_MASTER_NAME: str = "master" | ||
| REDIS_DB: int = 15 | ||
|
|
||
| REDIS_MAX_CONNECTIONS: int = 10 | ||
| REDIS_RETRY_TIMEOUT: bool = True | ||
| REDIS_DECODE_RESPONSES: bool = True | ||
| REDIS_CLIENT_NAME: str = "stac-fastapi-app" | ||
| REDIS_HEALTH_CHECK_INTERVAL: int = 30 | ||
|
|
||
|
|
||
| class RedisSettings(BaseSettings): | ||
| """Configuration for connecting Redis.""" | ||
|
|
||
| REDIS_HOST: str = "" | ||
| REDIS_PORT: int = 6379 | ||
| REDIS_DB: int = 0 | ||
|
|
||
| REDIS_MAX_CONNECTIONS: int = 10 | ||
| REDIS_RETRY_TIMEOUT: bool = True | ||
| REDIS_DECODE_RESPONSES: bool = True | ||
| REDIS_CLIENT_NAME: str = "stac-fastapi-app" | ||
| REDIS_HEALTH_CHECK_INTERVAL: int = 30 | ||
|
|
||
|
|
||
| # Configure only one Redis configuration | ||
| sentinel_settings = RedisSentinelSettings() | ||
| standalone_settings = RedisSettings() | ||
|
|
||
|
|
||
| async def connect_redis() -> Optional[aioredis.Redis]: | ||
| """Return a Redis connection Redis or Redis Sentinel.""" | ||
| global redis_pool | ||
|
There was a problem hiding this comment. Using global variables is an anti-pattern |
||
|
|
||
| if redis_pool is not None: | ||
| return redis_pool | ||
|
|
||
| try: | ||
| if sentinel_settings.REDIS_SENTINEL_HOSTS: | ||
| hosts = [ | ||
| h.strip() | ||
| for h in sentinel_settings.REDIS_SENTINEL_HOSTS.split(",") | ||
| if h.strip() | ||
| ] | ||
| ports = [ | ||
| int(p.strip()) | ||
| for p in sentinel_settings.REDIS_SENTINEL_PORTS.split(",") | ||
| if p.strip() | ||
| ] | ||
|
|
||
| sentinel = Sentinel( | ||
| [(h, p) for h, p in zip(hosts, ports)], | ||
| decode_responses=sentinel_settings.REDIS_DECODE_RESPONSES, | ||
| ) | ||
|
|
||
| redis_pool = sentinel.master_for( | ||
| service_name=sentinel_settings.REDIS_SENTINEL_MASTER_NAME, | ||
| db=sentinel_settings.REDIS_DB, | ||
| decode_responses=sentinel_settings.REDIS_DECODE_RESPONSES, | ||
| retry_on_timeout=sentinel_settings.REDIS_RETRY_TIMEOUT, | ||
| client_name=sentinel_settings.REDIS_CLIENT_NAME, | ||
| max_connections=sentinel_settings.REDIS_MAX_CONNECTIONS, | ||
| health_check_interval=sentinel_settings.REDIS_HEALTH_CHECK_INTERVAL, | ||
| ) | ||
| logger.info("Connected to Redis Sentinel") | ||
|
|
||
| elif standalone_settings.REDIS_HOST: | ||
| pool = aioredis.ConnectionPool( | ||
| host=standalone_settings.REDIS_HOST, | ||
| port=standalone_settings.REDIS_PORT, | ||
| db=standalone_settings.REDIS_DB, | ||
| max_connections=standalone_settings.REDIS_MAX_CONNECTIONS, | ||
| decode_responses=standalone_settings.REDIS_DECODE_RESPONSES, | ||
| retry_on_timeout=standalone_settings.REDIS_RETRY_TIMEOUT, | ||
| health_check_interval=standalone_settings.REDIS_HEALTH_CHECK_INTERVAL, | ||
| ) | ||
| redis_pool = aioredis.Redis( | ||
| connection_pool=pool, client_name=standalone_settings.REDIS_CLIENT_NAME | ||
| ) | ||
| logger.info("Connected to Redis") | ||
| else: | ||
| logger.warning("No Redis configuration found") | ||
| return None | ||
|
|
||
| return redis_pool | ||
|
|
||
| except Exception as e: | ||
|
There was a problem hiding this comment. We should catch exceptions a bit more narrowly. Catching such general exceptions, you don't know what happened. t might not have been a connection error at all, but something else entirely. |
||
| logger.error(f"Failed to connect to Redis: {e}") | ||
| redis_pool = None | ||
| return None | ||
|
|
||
|
|
||
| async def save_self_link( | ||
| redis: aioredis.Redis, token: Optional[str], self_href: str | ||
| ) -> None: | ||
| """Save the self link for the current token with 30 min TTL.""" | ||
| if token: | ||
| await redis.setex(f"nav:self:{token}", 1800, self_href) | ||
|
There was a problem hiding this comment. We shouldn't hardcode such variables. They should be set through an environment variable. |
||
|
|
||
|
|
||
| async def get_prev_link(redis: aioredis.Redis, token: Optional[str]) -> Optional[str]: | ||
| """Get the previous page link for the current token (if exists).""" | ||
| if not token: | ||
| return None | ||
| return await redis.get(f"nav:self:{token}") | ||
|
|
||
|
|
||
| async def redis_pagination_links( | ||
| current_url: str, token: str, next_token: str, links: list | ||
| ) -> None: | ||
| """Handle Redis pagination.""" | ||
| redis = None | ||
| try: | ||
| redis = await connect_redis() | ||
| logger.info("Redis connection established successfully") | ||
| except Exception as e: | ||
| redis = None | ||
| logger.warning(f"Redis connection failed: {e}") | ||
|
|
||
| if redis: | ||
| if next_token: | ||
| await save_self_link(redis, next_token, current_url) | ||
|
|
||
| prev_link = await get_prev_link(redis, token) | ||
| if prev_link: | ||
| links.insert( | ||
| 0, | ||
| { | ||
| "rel": "prev", | ||
| "type": "application/json", | ||
| "method": "GET", | ||
| "href": prev_link, | ||
| }, | ||
| ) | ||
|
There was a problem hiding this comment. let's change the name of this function to something like redis_pagination_links. There was a problem hiding this comment. I have re named the function to There was a problem hiding this comment. Handle_prev_pagination_links? There was a problem hiding this comment. I would not restrict the name to |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,9 +21,11 @@ | |
| "pre-commit~=3.0.0", | ||
| "ciso8601~=2.3.0", | ||
| "httpx>=0.24.0,<0.28.0", | ||
| "stac-fastapi-core[redis]==6.5.1", | ||
| ], | ||
| "docs": ["mkdocs~=1.4.0", "mkdocs-material~=9.0.0", "pdocs~=1.2.0"], | ||
| "server": ["uvicorn[standard]~=0.23.0"], | ||
|
There was a problem hiding this comment. Looks good. Here don't change anything. just add in "redis": ["stac-fastapi-core[redis]==6.5.1"] to the extra_reqs. Then someone can go There was a problem hiding this comment. Thank you for elaborating @jonhealy1 I have moved "redis": ["stac-fastapi-core[redis]==6.5.1"] into There was a problem hiding this comment. Thanks @YuriZmytrakov. I think you forgot to add it to opensearch setup? There was a problem hiding this comment. This: "stac-fastapi-core[redis]==6.5.1" can be in the "dev" section and the "redis" section if we want. There was a problem hiding this comment. I think there’s no harm in adding it to There was a problem hiding this comment. You can leave it in dev but there needs to be a redis section as well in extra reqs in both es and os There was a problem hiding this comment. @jonhealy1 added in es/os in extra_reqs and in redis section, thank you! |
||
| "redis": ["stac-fastapi-core[redis]==6.5.1"], | ||
| } | ||
|
|
||
| setup( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this code into a function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonhealy1 I have moved this code into
handle_pagination_linksfunc that can be reused where needed.