Skip to content
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

New serverless pattern: appsync-events-bedrock-cdk #2508

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

gbiagini97
Copy link
Contributor

@gbiagini97 gbiagini97 commented Nov 7, 2024

Issue #, if available:
#2509

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@gbiagini97 gbiagini97 changed the title New serverless pattern: stream bedrock completions via appsync events websockets New serverless pattern: appsync-events-bedrock-cdk Nov 7, 2024
Copy link
Contributor

@marcojahn marcojahn left a comment

Choose a reason for hiding this comment

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

I am not able to deploy, running step 4 from deployement instructions (npm run deploy) results in an error:

11:01:18 AM | CREATE_FAILED        | AWS::AppSync::ApiKey           | ApiKey
API key must be valid for a minimum of 1 days. (Service: AWSAppSync; Status Code: 400; Error Code: ApiKeyValidityOutOfBou
ndsException; Request ID: 4c4651cb-809a-4c09-a27c-39f3645fa860; Proxy: null)

The current pattern is broken after 12.12.

  ApiKey:
    Type: AWS::AppSync::ApiKey
    Properties:
      ApiId: !GetAtt EventsApi.ApiId
      Description: Api Key for Events Api
      Expires: 1733989774

As the API_KEY can be only configured for up to 365 days [1] please add creation/update process do the deployment documentation, as otherwise the code will break (requiring users to debug) after 365 days.

[1] https://docs.aws.amazon.com/appsync/latest/devguide/security-authz.html#api-key-authorization

appsync-events-bedrock-cdk/cdk-outputs.json Outdated Show resolved Hide resolved
appsync-events-bedrock-cdk/README.md Outdated Show resolved Hide resolved
appsync-events-bedrock-cdk/README.md Outdated Show resolved Hide resolved
appsync-events-bedrock-cdk/README.md Outdated Show resolved Hide resolved
appsync-events-bedrock-cdk/README.md Outdated Show resolved Hide resolved
appsync-events-bedrock-cdk/lib/events-api-cfn.yaml Outdated Show resolved Hide resolved
appsync-events-bedrock-cdk/src/chat.ts Show resolved Hide resolved
appsync-events-bedrock-cdk/utils/apigwRequest.ts Outdated Show resolved Hide resolved
appsync-events-bedrock-cdk/utils/appsyncRequest.ts Outdated Show resolved Hide resolved
@gbiagini97
Copy link
Contributor Author

@marcojahn sorry for the late. I addressed your points and removed the components defined via plain CFN. Now it's CDK-only.

@gbiagini97 gbiagini97 requested a review from marcojahn January 15, 2025 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants