-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(conversation): use functionMap for custom handler IFunction reference #2922
Changes from all commits
649a702
76e79fe
9df386a
3daf7b9
b289b62
fd26ee3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,7 +37,8 @@ | |
"immer": "^9.0.12" | ||
}, | ||
"devDependencies": { | ||
"@aws-amplify/graphql-transformer-test-utils": "1.0.3" | ||
"@aws-amplify/graphql-transformer-test-utils": "1.0.3", | ||
"esbuild": "^0.24.0" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. required for test cases |
||
}, | ||
"peerDependencies": { | ||
"aws-cdk-lib": "^2.158.0", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,12 +3,13 @@ import { IndexTransformer, PrimaryKeyTransformer } from '@aws-amplify/graphql-in | |
import { ModelTransformer } from '@aws-amplify/graphql-model-transformer'; | ||
import { validateModelSchema } from '@aws-amplify/graphql-transformer-core'; | ||
import { AppSyncAuthConfiguration, ModelDataSourceStrategy } from '@aws-amplify/graphql-transformer-interfaces'; | ||
import { DeploymentResources, testTransform } from '@aws-amplify/graphql-transformer-test-utils'; | ||
import { DeploymentResources, testTransform, TransformManager } from '@aws-amplify/graphql-transformer-test-utils'; | ||
import { parse } from 'graphql'; | ||
import { ConversationTransformer } from '..'; | ||
import { BelongsToTransformer, HasManyTransformer, HasOneTransformer } from '@aws-amplify/graphql-relational-transformer'; | ||
import * as fs from 'fs-extra'; | ||
import * as path from 'path'; | ||
import { Code, Function, IFunction, Runtime } from 'aws-cdk-lib/aws-lambda'; | ||
import { GenerationTransformer } from '@aws-amplify/graphql-generation-transformer'; | ||
import { toUpper } from 'graphql-transformer-common'; | ||
|
||
|
@@ -44,6 +45,35 @@ describe('ConversationTransformer', () => { | |
const schema = parse(out.schema); | ||
validateModelSchema(schema); | ||
}); | ||
|
||
it('uses functionMap for custom handler', () => { | ||
const routeName = 'pirateChat'; | ||
const inputSchema = getSchema('conversation-route-custom-handler.graphql', { ROUTE_NAME: routeName }); | ||
|
||
const transformerManager = new TransformManager(); | ||
const stack = transformerManager.getTransformScope(); | ||
const customHandler = new Function(stack, 'conversation-handler', { | ||
runtime: Runtime.NODEJS_18_X, | ||
code: Code.fromInline('exports.handler = async (event) => { return "Hello World"; }'), | ||
handler: 'index.handler', | ||
}); | ||
|
||
const functionMap = { | ||
[`Fn${routeName}`]: customHandler, | ||
}; | ||
|
||
const out = transform(inputSchema, {}, defaultAuthConfig, functionMap, transformerManager); | ||
expect(out).toBeDefined(); | ||
|
||
const expectedCustomHandlerArn = out.rawRootStack.resolve(customHandler.functionArn); | ||
const conversationLambdaStackName = `${toUpper(routeName)}ConversationDirectiveLambdaStack`; | ||
const conversationLambdaDataSourceName = `Fn${routeName}LambdaDataSource`; | ||
const conversationLambdaDataSourceFunctionArnRef = | ||
out.stacks[conversationLambdaStackName].Resources?.[conversationLambdaDataSourceName].Properties.LambdaConfig.LambdaFunctionArn.Ref; | ||
const lambdaDataSourceFunctionArn = | ||
out.rootStack.Resources?.[conversationLambdaStackName].Properties?.Parameters?.[conversationLambdaDataSourceFunctionArnRef]; | ||
expect(lambdaDataSourceFunctionArn).toEqual(expectedCustomHandlerArn); | ||
Comment on lines
+68
to
+75
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I welcome suggestions for a more succinct way to do this, like using CDK test helpers -- I was unable to find a better way. |
||
}); | ||
}); | ||
|
||
describe('invalid schemas', () => { | ||
|
@@ -114,6 +144,8 @@ function transform( | |
inputSchema: string, | ||
dataSourceStrategies?: Record<string, ModelDataSourceStrategy>, | ||
authConfig: AppSyncAuthConfiguration = defaultAuthConfig, | ||
functionMap?: Record<string, IFunction>, | ||
transformerManager?: TransformManager, | ||
): DeploymentResources { | ||
const modelTransformer = new ModelTransformer(); | ||
const authTransformer = new AuthTransformer(); | ||
|
@@ -129,7 +161,7 @@ function transform( | |
hasManyTransformer, | ||
hasOneTransformer, | ||
belongsToTransformer, | ||
new ConversationTransformer(modelTransformer, hasManyTransformer, belongsToTransformer, authTransformer), | ||
new ConversationTransformer(modelTransformer, hasManyTransformer, belongsToTransformer, authTransformer, functionMap), | ||
new GenerationTransformer(), | ||
authTransformer, | ||
]; | ||
|
@@ -139,6 +171,7 @@ function transform( | |
authConfig, | ||
transformers, | ||
dataSourceStrategies, | ||
transformerManager, | ||
}); | ||
|
||
return out; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
type Mutation { | ||
ROUTE_NAME( | ||
conversationId: ID!, | ||
content: [ContentBlockInput], | ||
aiContext: AWSJSON, | ||
toolConfiguration: ToolConfigurationInput | ||
): ConversationMessage | ||
@conversation( | ||
aiModel: "anthropic.claude-3-haiku-20240307-v1:0", | ||
functionName: "FnROUTE_NAME", | ||
systemPrompt: "You are a helpful chatbot. Answer questions to the best of your ability.", | ||
) | ||
} |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -5,7 +5,7 @@ import { APPSYNC_JS_RUNTIME, TransformerResolver } from '@aws-amplify/graphql-tr | |||
import { ResolverResourceIDs, FunctionResourceIDs, ResourceConstants, toUpper } from 'graphql-transformer-common'; | ||||
import * as cdk from 'aws-cdk-lib'; | ||||
import { conversation } from '@aws-amplify/ai-constructs'; | ||||
import { IFunction, Function } from 'aws-cdk-lib/aws-lambda'; | ||||
import { IFunction } from 'aws-cdk-lib/aws-lambda'; | ||||
import { getModelDataSourceNameForTypeName, getTable } from '@aws-amplify/graphql-transformer-core'; | ||||
import { initMappingTemplate } from '../resolvers/init-resolver'; | ||||
import { authMappingTemplate } from '../resolvers/auth-resolver'; | ||||
|
@@ -24,6 +24,8 @@ type KeyAttributeDefinition = { | |||
|
||||
// TODO: add explanation for the tool model queries | ||||
export class ConversationResolverGenerator { | ||||
constructor(private readonly functionNameMap?: Record<string, IFunction>) {} | ||||
|
||||
generateResolvers(directives: ConversationDirectiveConfiguration[], ctx: TransformerContextProvider): void { | ||||
for (const directive of directives) { | ||||
this.processToolsForDirective(directive, ctx); | ||||
|
@@ -89,7 +91,7 @@ export class ConversationResolverGenerator { | |||
capitalizedFieldName: string, | ||||
): { functionDataSourceId: string; referencedFunction: IFunction } { | ||||
if (directive.functionName) { | ||||
return this.setupExistingFunctionDataSource(directive.functionName, functionStack); | ||||
return this.setupExistingFunctionDataSource(directive.functionName); | ||||
} else { | ||||
return this.setupDefaultConversationHandler(functionStack, capitalizedFieldName, directive.aiModel); | ||||
} | ||||
|
@@ -101,28 +103,18 @@ export class ConversationResolverGenerator { | |||
* @param functionStack - The CDK stack to add the function to | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: delete
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks. will remove in follow up. |
||||
* @returns An object containing the function data source ID and the referenced function | ||||
*/ | ||||
private setupExistingFunctionDataSource( | ||||
functionName: string, | ||||
functionStack: cdk.Stack, | ||||
): { functionDataSourceId: string; referencedFunction: IFunction } { | ||||
private setupExistingFunctionDataSource(functionName: string): { functionDataSourceId: string; referencedFunction: IFunction } { | ||||
const functionDataSourceId = FunctionResourceIDs.FunctionDataSourceID(functionName); | ||||
const referencedFunction = Function.fromFunctionAttributes(functionStack, `${functionDataSourceId}Function`, { | ||||
functionArn: this.lambdaArnResource(functionName), | ||||
}); | ||||
|
||||
if (!this.functionNameMap) { | ||||
throw new Error('Function name map is not provided'); | ||||
} | ||||
const referencedFunction = this.functionNameMap[functionName]; | ||||
if (!referencedFunction) { | ||||
throw new Error(`Function ${functionName} not found in function name map`); | ||||
} | ||||
return { functionDataSourceId, referencedFunction }; | ||||
} | ||||
|
||||
/** | ||||
* Generates the Lambda ARN resource string | ||||
* @param name - The name of the Lambda function | ||||
* @returns The Lambda ARN resource string | ||||
*/ | ||||
private lambdaArnResource(name: string): string { | ||||
// eslint-disable-next-line no-template-curly-in-string | ||||
return cdk.Fn.sub('arn:aws:lambda:${AWS::Region}:${AWS::AccountId}:function:${name}', { name }); | ||||
} | ||||
|
||||
/** | ||||
* Sets up a default conversation handler function | ||||
* @param functionStack - The CDK stack to add the function to | ||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is from adding
esbuild
as a dev dependency in the conversation transformer.