PageETage based refresh support#23
Conversation
| eTagChanged, err := refreshClient.monitor.checkIfETagChanged(ctx) | ||
| if err != nil { | ||
| return false, fmt.Errorf("failed to check if watched settings have changed: %w", err) | ||
| return false, fmt.Errorf("failed to check if key value settings have changed: %w", err) |
There was a problem hiding this comment.
feels that the previous error message is just fine, "watched settings" is also accurate for "watch all" scenario.
There was a problem hiding this comment.
We have WatchedSettings refer to sentinel keys, using watched settings in watchAll scenarios is a little ambiguous. Besides, we also have feature flag refresh in the future, "key values" here can differentiate these scenarios.
| @@ -42,6 +42,8 @@ type AzureAppConfiguration struct { | |||
| watchedSettings []WatchedSetting | |||
|
|
|||
There was a problem hiding this comment.
There are blank lines between some fields, which seem to be used to categorize the fields, but I didn't get the underlying rule, it's likely just random to me for now.
There was a problem hiding this comment.
At the top, keyValues and feastureFlags(in the future) are settings from store. In the middle, options configured by user. Next, cache for refresh scenario. clientManager and resolver are objects talk to service.
There was a problem hiding this comment.
How about adding some comments for them?
2 participants
No description provided.