feat: implement delta cache in edge #736
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
|
This is looking good, we're going to need to change the logic for how Edge returns ETags in order to bring the experience for SDKs inline with what they see from Unleash. That can be done in a separate PR though, so won't let that block this. For ETags:
|
server/src/delta_cache.rs
Outdated
| } | ||
|
|
||
| fn add_base_event_from_hydration(&mut self, hydration_event: DeltaHydrationEvent) { | ||
| if let Some(last_feature) = hydration_event.features.last().cloned() { |
There was a problem hiding this comment.
This if let Some makes it looks like you always expect there to be a last feature and that that's a system constraint. If that's the case, an unwrap or expect would be more appropriate here
server/src/delta_cache.rs
Outdated
| } else { | ||
| self.hydration_event.features.push(feature); | ||
| } | ||
| self.hydration_event.event_id = event_id; |
There was a problem hiding this comment.
I think having this repeated event setting line in in 4 out of the 5 enum cases is a little fragile to future updates. You also went to all that trouble of creating a get_event_id method, so can I suggest we do this rather
- Remove the calls to
self.hydration_event.event_id = event_idin the match arms - Add a
if !matches!(event, DeltaEvent::Hydration { .. }) {
self.hydration_event.event_id = event.get_event_id();
}
At the top of the method
…into delta-cache
Edge needs to start keeping all the deltas in cache. This is a cache where these deltas are kept. Tried to keep it consistant with delta cache inside Unleash https://github.com/Unleash/unleash/blob/4a72580b2477c91883680fd03adc8e31ef85f48f/src/lib/features/client-feature-toggles/delta/delta-cache.ts#L7.