Skip to content
Open
Show file tree
Hide file tree
Changes from 9 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.1.3-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.1.3-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.1.3-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,74 @@
* @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
): Promise<void> {
if (importMode === undefined) {
importMode = ImportMode.IgnoreMatch;
}

Copy link
Member

Choose a reason for hiding this comment

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

[nitpick] Unnecessary blank line

this.validateImportMode(importMode);

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

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
*
* 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,
dryRun?: boolean
) {
customHeadersOption?: OperationOptions
): Promise<ConfigurationChanges> {
if (importMode == undefined) {
importMode = ImportMode.IgnoreMatch;
}
if (dryRun == undefined) {
dryRun = false;
}

this.validateImportMode(importMode);

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 +120,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
}
}
};
configurationSettingToAdd.push(...configSettings);

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

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 &&
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);
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(
Expand All @@ -146,7 +177,7 @@
await this.executeTasksWithTimeout(importTaskManager, timeout, progressCallback);
}

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

Check warning on line 180 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 {
Deleted: ConfigurationSetting<string>[];
Modified: SetConfigurationSettingParam<string | FeatureFlagValue | SecretReferenceValue>[];
Added: SetConfigurationSettingParam<string | FeatureFlagValue | SecretReferenceValue>[];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

In portal how will we distinguish key values that haven't changed when import mode is All ? I guess I was expecting an additional optional property for key values that haven't changed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If they haven't changed and import mode is All, they will be under modified. Could you elaborate more on the additional optional property? Not sure I follow

Copy link
Contributor

Choose a reason for hiding this comment

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

Usualy we have different icons to differentiate if a kv is added, deleted, modified or no change, icon for no change and modified is different if import mode All and kvs that haven't changed will be under modified how will we differentiate them from kvs that are modified in portal ?

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 think if a key-value hasn't changed and import mode is ALL. We write the key-value again which means the etag will change. In my opinion it still counts as a modified key-value.

Copy link
Contributor Author

@ChristineWanjau ChristineWanjau Dec 4, 2025

Choose a reason for hiding this comment

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

I imported a keyvalue from a file first then imported a second time with import mode ALL. See history;
image

Copy link
Contributor

Choose a reason for hiding this comment

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

In the update status for kv that isn't changed we show the status as "Equal" or "Updated" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From Portal, I do understand that it might be confusing to see "Updated" for a key-value that hasn't changed. Although only the etag would change, this is not visible to the user. Probably the user just cares about the key, label, value and content type, tag properties.

Although, I am wondering if we show equal in the diff on portal, if it will still be confusing if those key-values will still be written.

What do you think? Should we have another category in configuration changes to differentiate between key-values who have actually been modified (value, content-type and tags) and key-values to be refreshed (unchanged but will have a new etag)?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should have a category for unchanged key value also based on the current design you have, etag is more of a version identifier for a key and its updated anytime the kv is modified. Even in the diffs we currently show etag is not part of the diff its the other kv properties.
cc @RichardChen820 thoughts ?

Copy link
Member

@RichardChen820 RichardChen820 Dec 5, 2025

Choose a reason for hiding this comment

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

Why we need to differentiate them? If ImportMode is All, as long as this key-value exists, we will always put a new revision for it, whether its value has changed or not.

Copy link
Contributor Author

@ChristineWanjau ChristineWanjau Dec 5, 2025

Choose a reason for hiding this comment

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

If a key hasn't changed but Import mode is All it will be under modified, wondering if showing Updated/Modified on Portal will be confusing if the key-value hasn't changed.

We were planning on showing the diff this way on Portal. However, the design is not yet finalized.
image

Loading
Loading