feat: start storing actual deltas in edge #742
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
…eash-edge into store-deltas-in-edge
server/src/builder.rs
Outdated
| @@ -135,7 +140,8 @@ pub(crate) fn build_offline_mode( | |||
| client_features.clone(), | |||
| ) | |||
| } | |||
| Ok((token_cache, features_cache, engine_cache)) | |||
| // TODO: possibly need to resolve delta cache for offline mode? | |||
There was a problem hiding this comment.
Personal feeling but I'd skip this one for now. I don't see much value in sending deltas when that event stream is frozen
| features, | ||
| segments, | ||
| }, | ||
| 100, |
There was a problem hiding this comment.
Can we put this in a nicely named const somewhere rather?
| ), | ||
| ); | ||
| } else { | ||
| warn!( |
There was a problem hiding this comment.
Can this actually happen? Should this happen if the code is correct? This feels like we've broken some system constraint here. What does this mean for downstream SDKs if this happens?
There was a problem hiding this comment.
No, it the code is correct, it should never happen.
| @@ -249,6 +254,33 @@ impl FeatureRefresher { | |||
| } | |||
| } | |||
|
|
|||
| pub(crate) async fn delta_events_for_filter( | |||
There was a problem hiding this comment.
I think this is more or less a copy paste of features_for_filter. That's sometimes okay but I think in this case the subsumption and strict mode are fairly important system constraints that I think should change in lock step if they ever need to.
Is it tremendously difficult to get them to share the important stuff?
server/src/main.rs
Outdated
| @@ -80,6 +80,7 @@ async fn main() -> Result<(), anyhow::Error> { | |||
| let token_validator_schedule = token_validator.clone(); | |||
| let lazy_feature_cache = features_cache.clone(); | |||
| let lazy_token_cache = token_cache.clone(); | |||
| // TODO : do we need lazy delta cache? | |||
There was a problem hiding this comment.
Nope, if it compiles then you don't need it. The lazy stuff is to handle cases where something is used more than once later in this function and we need a reference
Might need this if we add this to the persistent store layer
| // TODO : do we need lazy delta cache? |
This is basic PoC delta caching and endpoint for /delta.
Next PRs will include: