feat: Config updater for the base provider #117
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #117 +/- ##
==========================================
+ Coverage 75.00% 75.30% +0.30%
==========================================
Files 135 137 +2
Lines 5548 5608 +60
==========================================
+ Hits 4161 4223 +62
+ Misses 1042 1039 -3
- Partials 345 346 +1 ☔ View full report in Codecov by Sentry. |
| @@ -80,7 +76,8 @@ func (p *Provider[K, V]) startAPI(ctx context.Context, responseCh chan<- provide | |||
| // attemptAPIDataUpdate tries to update data by fetching and parsing API data. | |||
| // It logs any errors encountered during the process. | |||
| func (p *Provider[K, V]) attemptAPIDataUpdate(ctx context.Context, responseCh chan<- providertypes.GetResponse[K, V]) { | |||
| if len(p.ids) == 0 { | |||
| ids := p.GetIDs() | |||
There was a problem hiding this comment.
Unclear what p.GetIDs() is?
There was a problem hiding this comment.
IDs are effectively currency pairs, the base provider just calls them IDs to have some generality.
| // for a given provider. This allows the provider to be updated asynchronously. | ||
| ConfigUpdater[K providertypes.ResponseKey] interface { | ||
| // GetIDs is the channel that is used to update the set of IDs that the provider | ||
| // will fetch data for. This blocks until there is a viable update. |
There was a problem hiding this comment.
Any reason to choose a channel v. a RWMutex here? Don't you want all Providers to read the updated ids concurrently? Do we expect a ConfigUpdater per Provider?
There was a problem hiding this comment.
Right now, this is being done per provider
There was a problem hiding this comment.
yea the interface that the config client, which is the client that is responsible for updating all providers, will use is the ConfigUpdater - which is specific per provider.
| return err | ||
| case <-ctx.Done(): | ||
| return <-errCh | ||
| } |
There was a problem hiding this comment.
Why do we need to kill the providers if there is a config update? Don't we j need to update the set of IDs it's fetching for?
There was a problem hiding this comment.
It works cleanly for that with REST providers, but for websockets, things get more complicated. We would need to do unsubscribe/subscribe logic based on the diff we get in providers.
This just simplifies things, creating a new connection so we have a fresh set of subscriptions.
closes BLO-901
This PR introduces a
ConfigUpdaterinterface that allows the base provider to be updated asynchronously. Anytime a configuration update is received, this will trigger the provider to restart with the new configurations.