-
Notifications
You must be signed in to change notification settings - Fork 302
Closed
Labels
bugSomething isn't workingSomething isn't workingpriority/P1Important / Should-HaveImportant / Should-Have
Milestone
Description
Description / Change would like to make:
NewCacheBackend currently instantiates backends without validating the given CacheConfig, which is error-prone.
semantic-router/src/semantic-router/pkg/cache/cache_factory.go
Lines 10 to 23 in 7a0221b
| // NewCacheBackend creates a cache backend instance from the provided configuration | |
| func NewCacheBackend(config CacheConfig) (CacheBackend, error) { | |
| if !config.Enabled { | |
| // Create a disabled cache backend | |
| observability.Debugf("Cache disabled - creating disabled in-memory cache backend") | |
| return NewInMemoryCache(InMemoryCacheOptions{ | |
| Enabled: false, | |
| }), nil | |
| } | |
| switch config.BackendType { | |
| case InMemoryCacheType, "": | |
| // Use in-memory cache as the default backend | |
| observability.Debugf("Creating in-memory cache backend - MaxEntries: %d, TTL: %ds, Threshold: %.3f", |
This issue proposes adding a ValidateCacheConfig(Config) call at the start of NewCacheBackend . This ensures invalid configs fail fast and keeps backend creation safe and consistent.
semantic-router/src/semantic-router/pkg/cache/cache_factory.go
Lines 62 to 79 in 7a0221b
| // ValidateCacheConfig validates cache configuration parameters | |
| func ValidateCacheConfig(config CacheConfig) error { | |
| if !config.Enabled { | |
| return nil // Skip validation for disabled cache | |
| } | |
| // Check similarity threshold range | |
| if config.SimilarityThreshold < 0.0 || config.SimilarityThreshold > 1.0 { | |
| return fmt.Errorf("similarity_threshold must be between 0.0 and 1.0, got: %f", config.SimilarityThreshold) | |
| } | |
| // Check TTL value | |
| if config.TTLSeconds < 0 { | |
| return fmt.Errorf("ttl_seconds cannot be negative, got: %d", config.TTLSeconds) | |
| } | |
| // Check max entries for in-memory cache | |
| if config.BackendType == InMemoryCacheType || config.BackendType == "" { |
Would the team agree with introducing this validation step?
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't workingpriority/P1Important / Should-HaveImportant / Should-Have