Conversation
travis79
left a comment
travis79
left a comment
There was a problem hiding this comment.
Let's be sure and mark this as a temporary solution pending sqlite being reinstated, either as a comment, changelog, or both. Other than that, there were a couple of important things, and a couple of things that were more nit-sized remarks, but this is looking pretty good!
|
|
||
| #[cfg(not(feature = "sqlite"))] | ||
| Vec::new() | ||
| block_on_dispatcher(); |
There was a problem hiding this comment.
This is a subtle change in behavior from before that will still be here if we go back to sqlite. Is this blocking so you can get any pings submitted right before the getter is called? Might be worth a comment as it will make any calling threads block (potentially).
There was a problem hiding this comment.
Yep, it's blocking because I ran into issues with testing. It should have been included the first time and I just missed it.
There was a problem hiding this comment.
Okay, let's add to the fn doc then calling out specifically that this blocks on the dispatcher so anyone calling it might see that in intellisense/etc and wouldn't have to go looking for it. Something like this would better document the behavior in a visible way:
/// Blocks and awaits the Glean Dispatcher before returning a Vec containing the stored pings
This branch has not been deployed
2 participants
This updates the RKV database to temporarily store submitted pings in memory when storing submitted pings is enabled.