Have mobile-config track when the data for a mobile-radio changes #902
Conversation
jeffgrunewald
left a comment
jeffgrunewald
left a comment
There was a problem hiding this comment.
one concern i have is that we're converting all these hotspot info fields to integers when we pull them in, presumably for easier hashing, but not clear to me if the resulting in Mobile Info is stored somewhere or if its exclusively used for determining a hash to see when the hotspot changes. is there a followup change to actually wire this "hotspot tracked changes" into the query api to do a mobile info api query filtering based on hotspots updated since X? and if yes would those be pulling the metadata (or cached) versions of the data being hashed by integer representation then?
| refreshed_at: DateTime<Utc>, | ||
| location: Option<i64>, | ||
| is_full_hotspot: Option<i32>, | ||
| num_location_asserts: Option<i32>, |
There was a problem hiding this comment.
do we care about this field?
| device_type: String, | ||
| deployment_info: Option<String>, |
There was a problem hiding this comment.
we should be able to, at the very least cast these to serde_json::Values if not use serde_json to convert them all the way to a full struct type that mirrors the jsonb of the db field. would at least be more parseable than just taking the json as a string
| entity_key: EntityKey, | ||
| refreshed_at: DateTime<Utc>, | ||
| location: Option<i64>, | ||
| is_full_hotspot: Option<i32>, |
There was a problem hiding this comment.
this is really just a bool, right?
|
does it make any more sense to extend the existing struct that we convert metadata hotspot info into to include the additional fields added in the new |
| .filter_map(|result| async move { | ||
| if let Err(err) = &result { | ||
| tracing::error!(?err, "error when reading radio metadata"); | ||
| } | ||
| result.ok() | ||
| }) |
There was a problem hiding this comment.
I'd prefer not to hide such errors but rather return an error result and stop updating to avoid the possibility of finishing with an inconsistent result.
There was a problem hiding this comment.
This is indeed a code smell.. why is this masking the error?
There was a problem hiding this comment.
we do this in several places where we perform get_all operations by services/agents; typically the logic has been to allow the overall system to continue operating in a degraded state, reporting errors as they happen, rather than causing a single bad record in 10s of thousands to cause the entire operation like a cache refresh in the mobile-verifier or rewardable entity distribution to grind to a halt, usually in off hours
There was a problem hiding this comment.
fair.. the downside is that without looking at the logs (which no client does) you'd end up operating on partial data?
Add ability for mobile-config to track when the data for a mobile radio changes.