Azure / AppConfiguration-JavaScriptProvider

The configuration provider for consuming data in Azure App Configuration from JavaScript applications like Node or browser apps.
https://github.com/Azure/AppConfiguration
MIT License
9 stars 2 forks source link

Support dynamic refresh #21

Closed Eskibear closed 9 months ago

Eskibear commented 11 months ago

Design

New APIs:

interface AzureAppConfiguration {
  ...
  refresh(): Promise<void>; // api to trigger refresh
  onRefresh(listener: () => any, thisArg?: any): void; // success callback
}

interface AzureAppConfigurationOptions { 
    ... 
    refreshOptions?: RefreshOptions; 
}

interface RefreshOptions {
  enabled: boolean; // default to false
  refreshIntervalInMs?: number; // default to 10s
  watchedSettings?: WatchedSetting[]
}

interface WatchedSetting  { 
    key: string; 
    label?: string;
}

Refresh Behavior: image

zhiyuanliang-ms commented 9 months ago

Should we make sure that the refresh() method can only be executed once at a time? Something like: https://github.com/Azure/AppConfiguration-DotnetProvider/blob/main/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs#L175 Or https://github.com/Azure/azure-sdk-for-python/blob/main/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py#L480

Another question is that: Should refresh() method be async?

RichardChen820 commented 9 months ago

Should we make sure that the refresh() method can only be executed once at a time? Something like: https://github.com/Azure/AppConfiguration-DotnetProvider/blob/main/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationProvider.cs#L175 Or https://github.com/Azure/azure-sdk-for-python/blob/main/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py#L480

Another question is that: Should refresh() method be async?

JS is single-thread model, I don't think we should worry about it. Correct me if I was wrong

Eskibear commented 9 months ago

Basically, JavaScript is single-threaded (except Web Workers or Worker Threads And the package is not expected to run on workers doing message communication... So, no worry.

avanigupta commented 9 months ago

I left some comments on this commit but they're not showing up on the main PR for some reason: https://github.com/Azure/AppConfiguration-JavaScriptProvider/commit/5d80ccca527000e6cb2083f93557172d28463582

Updated Yan: All resolved except one, highlighted in https://github.com/Azure/AppConfiguration-JavaScriptProvider/pull/21#pullrequestreview-1814662264