feat: update edge to new delta format #698
Conversation
Dependency ReviewThe following issues were found:
License Issuesserver/Cargo.toml
Denied Licenses: GPL-1.0, GPL-2.0, GPL-3.0, LGPL-2.1, LGPL-3.0, AGPL-3.0 OpenSSF Scorecard
Scanned Files
|
server/src/feature_cache.rs
Outdated
| @@ -80,7 +80,7 @@ impl FeatureCache { | |||
| .and_modify(|existing_features| { | |||
| existing_features.modify_in_place(delta); | |||
| }) | |||
| .or_insert(client_features); | |||
| .or_insert(client_features.modify_and_copy(delta)); | |||
There was a problem hiding this comment.
We can not put the values directly in, but need to run it through types method that parses events.
There was a problem hiding this comment.
Seems a bit unfortunate that we have to construct a new ClientFeatures just so modify_and_copy can clone it.
Running through the types code, it looks like modify_in_place and modify_and_copy are sharing the bulk of their code. Ideally the bulk of that method would be extracted to something that they could both share and a third method could be created that builds a ClientFeatures from a delta
There was a problem hiding this comment.
Good point, I thought about that also at one point, but did not want to make it complex. I will take a look now.
There was a problem hiding this comment.
Regarding the construction of new ClientFeatures, we need to construct it anyways at one point if the value is not in cache.
| @@ -403,15 +403,24 @@ impl FeatureRefresher { | |||
| .unleash_client | |||
| .get_client_features_delta(ClientFeaturesRequest { | |||
| api_key: refresh.token.token.clone(), | |||
| etag: refresh.etag.clone(), | |||
| etag: None, | |||
There was a problem hiding this comment.
No point to pass in this etag, because etags are different for old api and delta api.
There was a problem hiding this comment.
I'm curious, don't we still need an etag here?
Make edge support the new delta format.