perf: cache inspect.signature() at decoration time (#97)
All checks were successful
Build Docker Image / build (pull_request) Successful in 1m24s
All checks were successful
Build Docker Image / build (pull_request) Successful in 1m24s
Move inspect.signature(func) calls from inside wrapper functions to the outer decorator function so the introspection cost is paid once at decoration time instead of on every invocation. - logged_command: sig, param_names, and exclude_set computed at decoration time; wrapper.__signature__ reuses the pre-computed sig - cached_api_call: sig moved to decorator scope; bound_args still computed per-call (requires runtime args) - cached_single_item: same as cached_api_call Closes #97 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
6c49233392
commit
678f7d71e0
@ -11,14 +11,14 @@ from functools import wraps
|
||||
from typing import List, Optional, Callable, Any
|
||||
from utils.logging import set_discord_context, get_contextual_logger
|
||||
|
||||
cache_logger = logging.getLogger(f'{__name__}.CacheDecorators')
|
||||
period_check_logger = logging.getLogger(f'{__name__}.PeriodCheckDecorators')
|
||||
cache_logger = logging.getLogger(f"{__name__}.CacheDecorators")
|
||||
period_check_logger = logging.getLogger(f"{__name__}.PeriodCheckDecorators")
|
||||
|
||||
|
||||
def logged_command(
|
||||
command_name: Optional[str] = None,
|
||||
log_params: bool = True,
|
||||
exclude_params: Optional[List[str]] = None
|
||||
exclude_params: Optional[List[str]] = None,
|
||||
):
|
||||
"""
|
||||
Decorator for Discord commands that adds comprehensive logging.
|
||||
@ -54,7 +54,12 @@ def logged_command(
|
||||
- Function must be an async method with (self, interaction, ...) signature
|
||||
- Preserves Discord.py command registration compatibility
|
||||
"""
|
||||
|
||||
def decorator(func):
|
||||
sig = inspect.signature(func)
|
||||
param_names = list(sig.parameters.keys())[2:] # Skip self, interaction
|
||||
exclude_set = set(exclude_params or [])
|
||||
|
||||
@wraps(func)
|
||||
async def wrapper(self, interaction, *args, **kwargs):
|
||||
# Auto-detect command name if not provided
|
||||
@ -63,10 +68,6 @@ def logged_command(
|
||||
# Build context with safe parameter logging
|
||||
context = {"command": cmd_name}
|
||||
if log_params:
|
||||
sig = inspect.signature(func)
|
||||
param_names = list(sig.parameters.keys())[2:] # Skip self, interaction
|
||||
exclude_set = set(exclude_params or [])
|
||||
|
||||
for i, (name, value) in enumerate(zip(param_names, args)):
|
||||
if name not in exclude_set:
|
||||
context[f"param_{name}"] = value
|
||||
@ -74,7 +75,13 @@ def logged_command(
|
||||
set_discord_context(interaction=interaction, **context)
|
||||
|
||||
# Get logger from the class instance or create one
|
||||
logger = getattr(self, 'logger', get_contextual_logger(f'{self.__class__.__module__}.{self.__class__.__name__}'))
|
||||
logger = getattr(
|
||||
self,
|
||||
"logger",
|
||||
get_contextual_logger(
|
||||
f"{self.__class__.__module__}.{self.__class__.__name__}"
|
||||
),
|
||||
)
|
||||
trace_id = logger.start_operation(f"{func.__name__}_command")
|
||||
|
||||
try:
|
||||
@ -91,8 +98,9 @@ def logged_command(
|
||||
raise
|
||||
|
||||
# Preserve signature for Discord.py command registration
|
||||
wrapper.__signature__ = inspect.signature(func) # type: ignore
|
||||
wrapper.__signature__ = sig # type: ignore
|
||||
return wrapper
|
||||
|
||||
return decorator
|
||||
|
||||
|
||||
@ -122,6 +130,7 @@ def requires_draft_period(func):
|
||||
- Should be placed before @logged_command decorator
|
||||
- league_service must be available via import
|
||||
"""
|
||||
|
||||
@wraps(func)
|
||||
async def wrapper(self, interaction, *args, **kwargs):
|
||||
# Import here to avoid circular imports
|
||||
@ -133,10 +142,12 @@ def requires_draft_period(func):
|
||||
current = await league_service.get_current_state()
|
||||
|
||||
if not current:
|
||||
period_check_logger.error("Could not retrieve league state for draft period check")
|
||||
period_check_logger.error(
|
||||
"Could not retrieve league state for draft period check"
|
||||
)
|
||||
embed = EmbedTemplate.error(
|
||||
"System Error",
|
||||
"Could not verify draft period status. Please try again later."
|
||||
"Could not verify draft period status. Please try again later.",
|
||||
)
|
||||
await interaction.response.send_message(embed=embed, ephemeral=True)
|
||||
return
|
||||
@ -148,12 +159,12 @@ def requires_draft_period(func):
|
||||
extra={
|
||||
"user_id": interaction.user.id,
|
||||
"command": func.__name__,
|
||||
"current_week": current.week
|
||||
}
|
||||
"current_week": current.week,
|
||||
},
|
||||
)
|
||||
embed = EmbedTemplate.error(
|
||||
"Not Available",
|
||||
"Draft commands are only available in the offseason."
|
||||
"Draft commands are only available in the offseason.",
|
||||
)
|
||||
await interaction.response.send_message(embed=embed, ephemeral=True)
|
||||
return
|
||||
@ -161,7 +172,7 @@ def requires_draft_period(func):
|
||||
# Week <= 0, allow command to proceed
|
||||
period_check_logger.debug(
|
||||
f"Draft period check passed - week {current.week}",
|
||||
extra={"user_id": interaction.user.id, "command": func.__name__}
|
||||
extra={"user_id": interaction.user.id, "command": func.__name__},
|
||||
)
|
||||
return await func(self, interaction, *args, **kwargs)
|
||||
|
||||
@ -169,7 +180,7 @@ def requires_draft_period(func):
|
||||
period_check_logger.error(
|
||||
f"Error in draft period check: {e}",
|
||||
exc_info=True,
|
||||
extra={"user_id": interaction.user.id, "command": func.__name__}
|
||||
extra={"user_id": interaction.user.id, "command": func.__name__},
|
||||
)
|
||||
# Re-raise to let error handling in logged_command handle it
|
||||
raise
|
||||
@ -204,11 +215,14 @@ def cached_api_call(ttl: Optional[int] = None, cache_key_suffix: str = ""):
|
||||
- Class must have self.cache (CacheManager instance)
|
||||
- Class must have self._generate_cache_key, self._get_cached_items, self._cache_items methods
|
||||
"""
|
||||
|
||||
def decorator(func: Callable) -> Callable:
|
||||
sig = inspect.signature(func)
|
||||
|
||||
@wraps(func)
|
||||
async def wrapper(self, *args, **kwargs) -> List[Any]:
|
||||
# Check if caching is available (service has cache manager)
|
||||
if not hasattr(self, 'cache') or not hasattr(self, '_generate_cache_key'):
|
||||
if not hasattr(self, "cache") or not hasattr(self, "_generate_cache_key"):
|
||||
# No caching available, execute original method
|
||||
return await func(self, *args, **kwargs)
|
||||
|
||||
@ -216,20 +230,19 @@ def cached_api_call(ttl: Optional[int] = None, cache_key_suffix: str = ""):
|
||||
method_name = f"{func.__name__}{cache_key_suffix}"
|
||||
|
||||
# Convert args and kwargs to params list for consistent cache key
|
||||
sig = inspect.signature(func)
|
||||
bound_args = sig.bind(self, *args, **kwargs)
|
||||
bound_args.apply_defaults()
|
||||
|
||||
# Skip 'self' and convert to params format
|
||||
params = []
|
||||
for param_name, param_value in bound_args.arguments.items():
|
||||
if param_name != 'self' and param_value is not None:
|
||||
if param_name != "self" and param_value is not None:
|
||||
params.append((param_name, param_value))
|
||||
|
||||
cache_key = self._generate_cache_key(method_name, params)
|
||||
|
||||
# Try to get from cache
|
||||
if hasattr(self, '_get_cached_items'):
|
||||
if hasattr(self, "_get_cached_items"):
|
||||
cached_result = await self._get_cached_items(cache_key)
|
||||
if cached_result is not None:
|
||||
cache_logger.debug(f"Cache hit: {method_name}")
|
||||
@ -240,13 +253,14 @@ def cached_api_call(ttl: Optional[int] = None, cache_key_suffix: str = ""):
|
||||
result = await func(self, *args, **kwargs)
|
||||
|
||||
# Cache the result if we have items and caching methods
|
||||
if result and hasattr(self, '_cache_items'):
|
||||
if result and hasattr(self, "_cache_items"):
|
||||
await self._cache_items(cache_key, result, ttl)
|
||||
cache_logger.debug(f"Cached {len(result)} items for {method_name}")
|
||||
|
||||
return result
|
||||
|
||||
return wrapper
|
||||
|
||||
return decorator
|
||||
|
||||
|
||||
@ -265,23 +279,25 @@ def cached_single_item(ttl: Optional[int] = None, cache_key_suffix: str = ""):
|
||||
async def get_player(self, player_id: int) -> Optional[Player]:
|
||||
# Original method implementation
|
||||
"""
|
||||
|
||||
def decorator(func: Callable) -> Callable:
|
||||
sig = inspect.signature(func)
|
||||
|
||||
@wraps(func)
|
||||
async def wrapper(self, *args, **kwargs) -> Optional[Any]:
|
||||
# Check if caching is available
|
||||
if not hasattr(self, 'cache') or not hasattr(self, '_generate_cache_key'):
|
||||
if not hasattr(self, "cache") or not hasattr(self, "_generate_cache_key"):
|
||||
return await func(self, *args, **kwargs)
|
||||
|
||||
# Generate cache key
|
||||
method_name = f"{func.__name__}{cache_key_suffix}"
|
||||
|
||||
sig = inspect.signature(func)
|
||||
bound_args = sig.bind(self, *args, **kwargs)
|
||||
bound_args.apply_defaults()
|
||||
|
||||
params = []
|
||||
for param_name, param_value in bound_args.arguments.items():
|
||||
if param_name != 'self' and param_value is not None:
|
||||
if param_name != "self" and param_value is not None:
|
||||
params.append((param_name, param_value))
|
||||
|
||||
cache_key = self._generate_cache_key(method_name, params)
|
||||
@ -293,7 +309,9 @@ def cached_single_item(ttl: Optional[int] = None, cache_key_suffix: str = ""):
|
||||
cache_logger.debug(f"Cache hit: {method_name}")
|
||||
return self.model_class.from_api_data(cached_data)
|
||||
except Exception as e:
|
||||
cache_logger.warning(f"Error reading single item cache for {cache_key}: {e}")
|
||||
cache_logger.warning(
|
||||
f"Error reading single item cache for {cache_key}: {e}"
|
||||
)
|
||||
|
||||
# Cache miss - execute original method
|
||||
cache_logger.debug(f"Cache miss: {method_name}")
|
||||
@ -306,11 +324,14 @@ def cached_single_item(ttl: Optional[int] = None, cache_key_suffix: str = ""):
|
||||
await self.cache.set(cache_key, cache_data, ttl)
|
||||
cache_logger.debug(f"Cached single item for {method_name}")
|
||||
except Exception as e:
|
||||
cache_logger.warning(f"Error caching single item for {cache_key}: {e}")
|
||||
cache_logger.warning(
|
||||
f"Error caching single item for {cache_key}: {e}"
|
||||
)
|
||||
|
||||
return result
|
||||
|
||||
return wrapper
|
||||
|
||||
return decorator
|
||||
|
||||
|
||||
@ -326,6 +347,7 @@ def cache_invalidate(*cache_patterns: str):
|
||||
async def update_player(self, player_id: int, updates: dict) -> Optional[Player]:
|
||||
# Original method implementation
|
||||
"""
|
||||
|
||||
def decorator(func: Callable) -> Callable:
|
||||
@wraps(func)
|
||||
async def wrapper(self, *args, **kwargs):
|
||||
@ -333,16 +355,23 @@ def cache_invalidate(*cache_patterns: str):
|
||||
result = await func(self, *args, **kwargs)
|
||||
|
||||
# Invalidate specified cache patterns
|
||||
if hasattr(self, 'cache'):
|
||||
if hasattr(self, "cache"):
|
||||
for pattern in cache_patterns:
|
||||
try:
|
||||
cleared = await self.cache.clear_prefix(f"sba:{self.endpoint}_{pattern}")
|
||||
cleared = await self.cache.clear_prefix(
|
||||
f"sba:{self.endpoint}_{pattern}"
|
||||
)
|
||||
if cleared > 0:
|
||||
cache_logger.info(f"Invalidated {cleared} cache entries for pattern: {pattern}")
|
||||
cache_logger.info(
|
||||
f"Invalidated {cleared} cache entries for pattern: {pattern}"
|
||||
)
|
||||
except Exception as e:
|
||||
cache_logger.warning(f"Error invalidating cache pattern {pattern}: {e}")
|
||||
cache_logger.warning(
|
||||
f"Error invalidating cache pattern {pattern}: {e}"
|
||||
)
|
||||
|
||||
return result
|
||||
|
||||
return wrapper
|
||||
|
||||
return decorator
|
||||
Loading…
Reference in New Issue
Block a user