Skip to content
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "@azure/app-configuration-importer-file-source",
"author": "Microsoft Corporation",
"description": "A client library for importing/exporting key-values between configuration file sources and Azure App Configuration service",
"version": "1.1.3-preview",
"version": "2.0.0-preview",
"sdk-type": "client",
"keywords": [
"node",
Expand Down Expand Up @@ -33,7 +33,7 @@
},
"dependencies": {
"@azure/app-configuration": "^1.9.0",
"@azure/app-configuration-importer": "1.1.3-preview"
"@azure/app-configuration-importer": "2.0.0-preview"
},
"devDependencies": {
"@microsoft/api-extractor": "^7.22.2",
Expand Down
2 changes: 1 addition & 1 deletion libraries/azure-app-configuration-importer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "@azure/app-configuration-importer",
"author": "Microsoft Corporation",
"description": "A client library for importing/exporting key-values between configuration sources and Azure App Configuration service",
"version": "1.1.3-preview",
"version": "2.0.0-preview",
"sdk-type": "client",
"keywords": [
"node",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import { ImportMode } from "./enums";
import { OperationTimeoutError, ArgumentError } from "./errors";
import { AdaptiveTaskManager } from "./internal/adaptiveTaskManager";
import { ImportProgress, KeyLabelLookup } from "./models";
import { ImportProgress, KeyLabelLookup, ConfigurationChanges } from "./models";
import { isConfigSettingEqual } from "./internal/utils";
import { v4 as uuidv4 } from "uuid";
import { Constants } from "./internal/constants";
Expand Down Expand Up @@ -43,27 +43,78 @@
* @param timeout - Seconds of entire import progress timeout
* @param progressCallback - Callback for report the progress of import
* @param importMode - Determines the behavior when importing key-values. The default value, 'All' will import all key-values in the input file to App Configuration. 'Ignore-Match' will only import settings that have no matching key-value in App Configuration.
* @param dryRun - When dry run is enabled, no updates will be performed to App Configuration. Instead, any updates that would have been performed in a normal run will be printed to the console for review
* @returns Promise<void>
*/
public async Import(
configSettingsSource: ConfigurationSettingsSource,
timeout: number,
strict = false,
progressCallback?: (progress: ImportProgress) => unknown,
importMode?: ImportMode,
dryRun?: boolean
) {
if (importMode == undefined) {
importMode = ImportMode.IgnoreMatch;
}
if (dryRun == undefined) {
dryRun = false;
}
importMode = ImportMode.IgnoreMatch
): Promise<void> {
this.validateImportMode(importMode);

// Generate correlation ID for operations
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Generate correlationRequestId for operations in the same activity

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

const customCorrelationRequestId: string = uuidv4();
const customHeadersOption: OperationOptions = {
requestOptions: {
customHeaders: {
[Constants.CorrelationRequestIdHeader]: customCorrelationRequestId
}
}
};

const configSettings = await configSettingsSource.GetConfigurationSettings();
const configurationChanges: ConfigurationChanges = await this.GetConfigurationChanges(configSettingsSource, strict, importMode, customHeadersOption);
Copy link
Member

@RichardChen820 RichardChen820 Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to imagine, how the GetConfigurationChanges() and Import() will be used at client side. One possible situation is, client calls GetConfigurationChanges to preview what are the settings to be changed, then it may ask user 's promotion to apply the change or not, if user confirm to apply the change by calling Import(), In this case, I think Import() should no longer need to call the GetConfigurationChanges() again, just apply the chanages directly.

So seems we can't just blindly call GetConfigurationChanges in Import, Import should have another overload that can accept ConfigurationChanges as an argument.

Thoughts? @MaryanneNjeri @ChristineWanjau

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RichardChen820 I agree, and Import would no longer need to take parameters like strict or import mode ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I think of the current design, the caller would call GetConfigurationChanges twice, once when they call GetConfigurationChanges and second time when they want to Import.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point. This is the most likely flow in the client side especially portal. We are planning to preview the changes to the caller then they can click on apply immediately after.

Just one concern I have is consistency issues, what if the pre-calculated changes may no longer be accurate?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, it completely does not make sense, and also have risk that GetConfigurationChanges may return different values in these two attempts

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we provide a preview, only performing the changes exactly as shown in the preview can be considered consistent.

Copy link
Contributor Author

@ChristineWanjau ChristineWanjau Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This leads to a result where we delete an item that the customer was never told would be deleted. This is the real inconsistency from the customer’s point of view

I see, this makes sense.
Considering we won't delete anything added into the store before import happens, to some extent there is not much risk involved.

Copy link
Contributor Author

@ChristineWanjau ChristineWanjau Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just confirm so we have a second overload of the Import() function that takes in ConfigurationChanges.

public async Import(
  configSettingsSource: ConfigurationSettingsSource,
  timeout: number,
  strict?: boolean,
  progressCallback?: (progress: ImportProgress) => unknown,
  importMode?: ImportMode
): Promise<void>;

/**
 * Import pre-calculated configuration changes into the Azure App Configuration service
 * 
 * 
 * @param configurationChanges - Pre-calculated changes from GetConfigurationChanges()
 * @param timeout
 * @param progressCallback
 * @returns Promise<void>
 */
public async Import(
  configurationChanges: ConfigurationChanges,
  timeout: number,
  progressCallback?: (progress: ImportProgress) => unknown
): Promise<void>;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! I have updated the function


await this.applyUpdatesToServer([...configurationChanges.Added, ...configurationChanges.Modified], configurationChanges.Deleted, timeout, customHeadersOption, progressCallback);
}

/**
* Get configuration changes between source settings and Azure App Configuration service without any applying changes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does it mean "without any applying changes" ? What do you think of "Get configuration changes between source settings and existing settings in Azure App Configuration service...`

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant - "without applying any changes". Don't you think its beneficial to mention this so that users can understand they can call it safely just to preview changes?

Copy link
Contributor Author

@ChristineWanjau ChristineWanjau Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggesting -"Get configuration changes between source settings and existing settings in Azure App Configuration service without applying any changes"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me

*
* Example usage:
* ```ts
* const fileData = fs.readFileSync("mylocalPath").toString();
* const configurationChanges = await client.getConfigurationChanges(
* new StringConfigurationSettingsSource({data:fileData, format: ConfigurationFormat.Json}),
* false,
* ImportMode.All,
* options
* );
* ```
* @param configSettingsSource - A ConfigurationSettingsSource instance.
* @param strict - Use strict mode to delete settings not in source.
* @param importMode - Determines the behavior when analyzing key-values.
* 'All' will include all key-values.
* 'Ignore-Match' will exclude settings that have matching key-values in App Configuration.
* @param customHeadersOption - Custom headers for the operation.
* @returns ConfigurationChanges object containing Added, Modified, and Deleted settings
*/
public async GetConfigurationChanges(
configSettingsSource: ConfigurationSettingsSource,
strict = false,
importMode = ImportMode.IgnoreMatch,
customHeadersOption?: OperationOptions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this used in this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The customHeadersOptions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. It does look like needed for this function.

Copy link
Contributor Author

@ChristineWanjau ChristineWanjau Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use customHeadersOption to pass the correlationRequestId to link requests involved in the Import process. When the Import API calls the GetConfigurationChanges API, we pass the same correlationRequestId to associate both the read and write operations.
Additionally, we could use this to link the call to request the diff and Import on the portal for better traceability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline. Updated to generate correlationrequestid if not passed to GetConfigurationChanges

): Promise<ConfigurationChanges> {
this.validateImportMode(importMode);

// Generate correlation ID
if (!customHeadersOption) {
const customCorrelationRequestId: string = uuidv4();
customHeadersOption = {
requestOptions: {
customHeaders: {
[Constants.CorrelationRequestIdHeader]: customCorrelationRequestId
}
}
};
}

const configSettings = await configSettingsSource.GetConfigurationSettings();

const configurationSettingToDelete: ConfigurationSetting<string>[] = [];
const configurationSettingToModify: SetConfigurationSettingParam<string | FeatureFlagValue | SecretReferenceValue>[] = [];
const configurationSettingToAdd: SetConfigurationSettingParam<string | FeatureFlagValue | SecretReferenceValue>[] = [];
const srcKeyLabelLookUp: KeyLabelLookup = {};

configSettings.forEach((config: SetConfigurationSettingParam<string | FeatureFlagValue | SecretReferenceValue>) => {
Expand All @@ -73,59 +124,43 @@
srcKeyLabelLookUp[config.key][config.label || ""] = true;
});

// generate correlationRequestId for operations in the same activity
const customCorrelationRequestId: string = uuidv4();
const customHeadersOption: OperationOptions = {
requestOptions: {
customHeaders: {
[Constants.CorrelationRequestIdHeader]: customCorrelationRequestId
}
}
};

if (strict || importMode == ImportMode.IgnoreMatch) {
for await (const existing of this.configurationClient.listConfigurationSettings({...configSettingsSource.FilterOptions, ...customHeadersOption})) {
configurationSettingToAdd.push(...configSettings);

const isKeyLabelPresent: boolean = srcKeyLabelLookUp[existing.key] && srcKeyLabelLookUp[existing.key][existing.label || ""];
for await (const existing of this.configurationClient.listConfigurationSettings({...configSettingsSource.FilterOptions, ...customHeadersOption})) {
const isKeyLabelPresent: boolean = srcKeyLabelLookUp[existing.key] && srcKeyLabelLookUp[existing.key][existing.label || ""];
if (strict && !isKeyLabelPresent) {
configurationSettingToDelete.push(existing);
}

const incoming = configSettings.find(configSetting => configSetting.key == existing.key &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to suggest instead of adding settings then removing them, maybe we shouldn't add them in the first place, maybe we can have something like this

if (importMode == ImportMode.IgnoreMatch) { 
    const incoming = configSettings.find(configSetting => configSetting.key == existing.key && 
            configSetting.label == existing.label);
    if (!incoming) { 
       configSettingToAdd.push(existing);
  } else if (incoming && !isConfigSettingEqual(incoming, existing)) { 
   configSettingToModify.push(existing);
 } 
}

Doesn't have to be exactly like this just an idea

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit tricky to add settings in the loop because we are looping through existing ones. I have made updates to the logic please let me know what you think

configSetting.label === existing.label);

if (incoming) {
const settingsAreEqual: boolean = isConfigSettingEqual(incoming, existing);

if (strict && !isKeyLabelPresent) {
configurationSettingToDelete.push(existing);
if (!settingsAreEqual) {
configurationSettingToModify.push(incoming);
// Remove from add list since it's a modification, not an addition
const addIndex = configurationSettingToAdd.indexOf(incoming);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can simplify this to configurationSettingsToAdd.splice(configurationSettingToAdd.indexOf(incoming), 1).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

if (addIndex !== -1) {
configurationSettingToAdd.splice(addIndex, 1);
}
}

if (importMode == ImportMode.IgnoreMatch) {
const incoming = configSettings.find(configSetting => configSetting.key == existing.key &&
configSetting.label == existing.label);

if (incoming && isConfigSettingEqual(incoming, existing)) {
configSettings.splice(configSettings.indexOf(incoming), 1);
else if (importMode === ImportMode.IgnoreMatch) {
// Remove unchanged settings from add list
const addIndex = configurationSettingToAdd.indexOf(incoming);
if (addIndex !== -1) {
configurationSettingToAdd.splice(addIndex, 1);
}
}
}
}

if (dryRun) {
this.printUpdatesToConsole(configSettings, configurationSettingToDelete);
}
else {
await this.applyUpdatesToServer(configSettings, configurationSettingToDelete, timeout, customHeadersOption, progressCallback);
}
}

private printUpdatesToConsole(
settingsToAdd: SetConfigurationSettingParam<string | FeatureFlagValue | SecretReferenceValue>[],
settingsToDelete: ConfigurationSetting<string>[]
): void {
console.log("The following settings will be removed from App Configuration:");
for (const setting of settingsToDelete) {

console.log(JSON.stringify({key: setting.key, label: setting.label, contentType: setting.contentType, tags: setting.tags}));
}

console.log("\nThe following settings will be written to App Configuration:");
for (const setting of settingsToAdd) {

console.log(JSON.stringify({key: setting.key, label: setting.label, contentType: setting.contentType, tags: setting.tags}));
}
return {
Added: configurationSettingToAdd,
Modified: configurationSettingToModify,
Deleted: configurationSettingToDelete
};
}

private async applyUpdatesToServer(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about changing the argument name settingsToAdd to settingsToPut since it can involve add and modify?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

Expand All @@ -146,7 +181,7 @@
await this.executeTasksWithTimeout(importTaskManager, timeout, progressCallback);
}

private newAdaptiveTaskManager<T>(task: (setting: T) => Promise<any>, configurationSettings: Array<T>) {

Check warning on line 184 in libraries/azure-app-configuration-importer/src/appConfigurationImporter.ts

View workflow job for this annotation

GitHub Actions / build

Unexpected any. Specify a different type
let index = 0;
return new AdaptiveTaskManager(() => {
if (index == configurationSettings.length) {
Expand Down
2 changes: 1 addition & 1 deletion libraries/azure-app-configuration-importer/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export {
} from "./importOptions";
export * from "./enums";
export * from "./errors";
export { ImportProgress as ImportResult } from "./models";
export { ImportProgress as ImportResult, ConfigurationChanges } from "./models";
export { StringConfigurationSettingsSource } from "./settingsImport/stringConfigurationSettingsSource";
export { ConfigurationSettingsSource } from "./settingsImport/configurationSettingsSource";
export { IterableConfigurationSettingsSource } from "./settingsImport/iterableConfigurationSettingsSource";
Expand Down
13 changes: 13 additions & 0 deletions libraries/azure-app-configuration-importer/src/models.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import {
SecretReferenceValue,
ConfigurationSetting,
SetConfigurationSettingParam,
FeatureFlagValue
} from "@azure/app-configuration";

/**
* @internal
*/
Expand Down Expand Up @@ -42,4 +49,10 @@ export interface KeyLabelLookup {
[key: string]: {
[label: string] : boolean
}
}

export interface ConfigurationChanges {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a config hasn't change, do we also return it as part of the ConfigurationChanges

Copy link
Contributor Author

@ChristineWanjau ChristineWanjau Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only when import mode is equal to ALL; if a config hasn't changed, then it will be part of the changes

Deleted: ConfigurationSetting<string>[];
Modified: SetConfigurationSettingParam<string | FeatureFlagValue | SecretReferenceValue>[];
Added: SetConfigurationSettingParam<string | FeatureFlagValue | SecretReferenceValue>[];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about rename these fields to ToDelete ToModify, ToAdd? The current names seem to imply that the change has been made.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. Updated

}
Loading
Loading