[Mobile Config] Add updated_at to GatewayInfoV2 (response) #921
Conversation
kurotych
changed the title
kurotych
requested review from
michaeldjeffrey,
jeffgrunewald,
andymck and
bbalser
mobile_config/src/gateway_info.rs
Outdated
| pub updated_at: Option<DateTime<Utc>>, | ||
| pub created_at: Option<DateTime<Utc>>, | ||
| pub refreshed_at: Option<DateTime<Utc>>, |
There was a problem hiding this comment.
can you elaborate on the difference between updated_at and refreshed_at -- perhaps consider adding as comments as well.
There was a problem hiding this comment.
These should be documented in the proto definition. Refreshed means when last the chain was consulted, regardless of whether or not the data has changed. Updated specifically refers to the last time the data was changed
There was a problem hiding this comment.
That's where i got confused - the proto only includes updated_at but not refreshed_at
https://github.com/helium/proto/blob/b5564eb32d1fb11d7a0570a1c2311b9ed39d297e/src/service/mobile_config.proto#L81-L84
So refreshed_at is only related to bookkeeping/internals of this service?
There was a problem hiding this comment.
These should be documented in the proto definition
There is no refreshed_at in proto definition it is internal variable.
updated_at is documented in proto
So
refreshed_atis only related to bookkeeping/internals of this service?
Yes refreshed_at is internal thing. I intentionally left only updated_at to not confuse clients.
Also I don't see any purposes for clients to have refreshed_at field
Added comments co code.
The updated_at field in the response is required to complete the workflow for fetching only the most recently changed hotspots.
This solution is unoptimized since updated_radios is fetched every time. The performance (for v2 endpoints) might be impacted.
The
updated_radioshashmap should be cached in future versions.helium/proto#436