Skip to content

Commit

Permalink
fix(infra): correct application config merging issue
Browse files Browse the repository at this point in the history
Do not merge arrays, just replace. Prevents always merging
the default predefined models into deployment config.
  • Loading branch information
JeremyJonas authored and sperka committed Nov 7, 2023
1 parent cda00e8 commit eed035a
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 11 deletions.
5 changes: 2 additions & 3 deletions packages/galileo-cdk/src/core/app/context/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ import { Stack, Stage } from 'aws-cdk-lib';
import { StringParameter } from 'aws-cdk-lib/aws-ssm';
import { IConstruct } from 'constructs';
import * as fs from 'fs-extra';
import { merge } from 'lodash';
import { DEFAULT_APPLICATION_CONFIG } from './defaults';
import { APPLICATION_CONFIG_JSON, ApplicationConfig, IApplicationContext } from './types';
import { resolveBoolean } from './utils';
import { mergeApplicationConfigs, resolveBoolean } from './utils';
import {
getRootStack,
safeResourceName as _safeResourceName,
Expand Down Expand Up @@ -88,7 +87,7 @@ export class ApplicationContext implements IApplicationContext {

// merge the supplied application config with the defaults to ensure any new features/updates are applied
// use default array merge of replace
this.config = merge({}, DEFAULT_APPLICATION_CONFIG, ssmConfig || this.readConfig(this.configPath));
this.config = mergeApplicationConfigs(DEFAULT_APPLICATION_CONFIG, ssmConfig || this.readConfig(this.configPath));

this.applicationName = this.config.app.name;
}
Expand Down
16 changes: 16 additions & 0 deletions packages/galileo-cdk/src/core/app/context/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
PDX-License-Identifier: Apache-2.0 */

import { IEmbeddingModelInfo } from '@aws/galileo-sdk/lib/models';
import { mergeWith } from 'lodash';
import { ApplicationConfig } from './types';

export function resolveList(
value?: string | string[],
Expand Down Expand Up @@ -62,3 +64,17 @@ export function sortRagEmbeddingModels(embeddingModels: IEmbeddingModelInfo[]):
return 1;
});
}

/**
* Merge multiple application config definitions into a single config.
* - Objects are recursively merged
* - Arrays are replaced
* @param configs List of ApplicationConfig objects to merge
* @returns Returns single merged config
*/
export function mergeApplicationConfigs(...configs: Partial<ApplicationConfig>[]): ApplicationConfig {
return mergeWith({}, ...configs, (_objValue: any, _srcValue: any) => {
if (Array.isArray(_srcValue)) return _srcValue;
return undefined; // fallback
});
}
5 changes: 3 additions & 2 deletions packages/galileo-cli/src/internals/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ export * from '../../../../demo/infra/src/application/tags';
import path from 'node:path';
import chalk from 'chalk';
import fs from 'fs-extra';
import { merge } from 'lodash';
import { formatBedrockModelUUID } from '../../../galileo-cdk/src/ai/llms/framework/bedrock/utils';
import {
DEFAULT_APPLICATION_CONFIG,
APPLICATION_CONFIG_JSON,
ApplicationConfig,
// @ts-ignore - sdk is esm
} from '../../../galileo-cdk/src/core/app/context';
import { mergeApplicationConfigs } from '../../../galileo-cdk/src/core/app/context/utils';

export const APP_CONFIG_DIR = path.resolve(__dirname, '../../../../demo/infra');

Expand All @@ -37,7 +37,8 @@ export namespace helpers {
export const resolveAppConfig = (file?: string): ApplicationConfig => {
file = resolveConfigPath(file);
if (fs.existsSync(file)) {
return merge({}, DEFAULT_APPLICATION_CONFIG, fs.readJsonSync(file, { encoding: 'utf-8' }));
const jsonConfig = fs.readJsonSync(file, { encoding: 'utf-8' });
return mergeApplicationConfigs(DEFAULT_APPLICATION_CONFIG, jsonConfig);
}
return DEFAULT_APPLICATION_CONFIG;
};
Expand Down
9 changes: 3 additions & 6 deletions packages/galileo-cli/src/lib/prompts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,9 @@ namespace galileoPrompts {
}

if (sampleDatasets.includes(SampleDataSets.SUPREME_COURT_CASES)) {
if (
context.appConfig.rag.indexing?.pipeline?.maxInstanceCount ||
0 < 10 ||
context.appConfig.rag.managedEmbeddings.autoscaling?.maxCapacity ||
0 < 5
) {
const maxInstanceCount = context.appConfig.rag.indexing?.pipeline?.maxInstanceCount ?? 0;
const maxCapacity = context.appConfig.rag.managedEmbeddings.autoscaling?.maxCapacity ?? 0;
if (maxInstanceCount < 10 || maxCapacity < 5) {
if (
(
await prompts({
Expand Down

0 comments on commit eed035a

Please sign in to comment.