Conversation
devslovecoffee
left a comment
devslovecoffee
left a comment
There was a problem hiding this comment.
No issues found.
devslovecoffee
left a comment
devslovecoffee
left a comment
There was a problem hiding this comment.
No issues found.
devslovecoffee
left a comment
devslovecoffee
left a comment
There was a problem hiding this comment.
No issues found.
| #[serde(deny_unknown_fields)] | ||
| pub struct EntityCachingRedisConfig { | ||
| #[serde(default = "EntityCachingRedisConfig::default_url")] | ||
| pub url: url::Url, |
There was a problem hiding this comment.
⚡ Suggestion
Consider adding validation logic in the EntityCachingRedisConfig struct to ensure that the url is a valid Redis URL when deserializing, which can prevent runtime errors.
| entity_caching: entity_caching.as_ref().map(|config| match config { | ||
| EntityCachingConfig::Disabled => EntityCaching::Disabled, | ||
| EntityCachingConfig::Enabled { ttl } => EntityCaching::Enabled { ttl: *ttl }, | ||
| EntityCachingConfig::Enabled { ttl, .. } => EntityCaching::Enabled { ttl: *ttl }, |
There was a problem hiding this comment.
⚡ Suggestion
Ensure that the handling of additional fields in the pattern matching is consistent across all usages to avoid potential bugs.
|
|
||
| #[derive(Debug, Clone, PartialEq, serde::Deserialize)] | ||
| #[serde(deny_unknown_fields)] | ||
| pub struct EntityCachingRedisConfig { |
There was a problem hiding this comment.
⚡ Suggestion
Consider adding a method to EntityCachingRedisConfig to validate the configuration, ensuring that the url is correctly formatted and the key_prefix is not empty, which can prevent runtime errors.
1 participant
✨
Description by Cal
PR Description
This PR introduces new fields and configurations related to caching and storage, including updates to entity caching configurations and the addition of Redis support.
Diagrams of code changes
sequenceDiagram participant User participant System participant EntityCachingConfig participant RedisConfig User->>System: Request to enable entity caching System->>EntityCachingConfig: Set enabled to true EntityCachingConfig->>System: Return updated config System->>RedisConfig: Configure Redis settings RedisConfig->>System: Return Redis config System->>User: Response with caching enabledKey Issues
None
Files Changed
File: /engine/crates/engine-config-builder/src/from_sdl_config.rs
Updated pattern matching in `EntityCachingConfig::Enabled` to include `storage` field.File: /engine/crates/engine/response/src/lib.rs
Added `used_fields` field with default value in `GraphqlOperationAnalyticsAttributes` struct.File: /engine/crates/parser-sdl/src/federation.rs
Added `EntityCacheStorage` enum and implemented conversion functions for Redis configurations.File: /engine/crates/parser-sdl/src/rules/subgraph_directive.rs
Refactored setting `entity_caching` in `SubgraphDirectiveVisitor` to use pattern matching.File: /engine/crates/runtime-local/src/lib.rs
Added conditional compilation for `redis` module.File: /gateway/crates/config/src/entity_caching.rs
Added `storage` and `redis` fields with configurations in `EntityCachingConfig` struct.Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context.
Type of change