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

fix(bedrock): adopt best practices #751

Merged
merged 14 commits into from
Oct 21, 2024
Merged

Conversation

aws-rafams
Copy link
Contributor

Fixes #733


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

@krokoko
Copy link
Collaborator

krokoko commented Oct 16, 2024

Thanks @aws-rafams ! Did a first pass and looks good to me overall. Left one comment about type aliasing.
Next steps: I will test the package in TS and Python for couple of use cases, and put back my results here
Once done, if everything works as expected, will approve

@krokoko
Copy link
Collaborator

krokoko commented Oct 16, 2024

First results from testing in TS:

  • Agent created with guardrails (TS)

image

The agent is correctly created, as well as the guardrails

image

image

Guardrails are correctly working (permissions are correctly applied):

image

image

  • Guardrails imported from existing resource created through the console: works but there is not much to do with it since it cannot be added to an agent

@krokoko
Copy link
Collaborator

krokoko commented Oct 16, 2024

Testing in Python: everything seems to work, except the GuardrailSampleTopics which don't seem to be exported
image

@krokoko krokoko self-requested a review October 16, 2024 23:08
Copy link
Collaborator

@krokoko krokoko left a comment

Choose a reason for hiding this comment

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

Thanks @aws-rafams ! Overall it is good, I tested in both TS and Python
Fixed a readme issue and added Python code snippets

Only blocking issue is the GuardrailSampleTopics which doesn't seem to be exported correctly In Python (cannot access it). It is working fine in Ts. Once fixed, I will approve.

My other comments are non blocking, and could be some work part of a subsequent PR.

Thank you !!

@krokoko krokoko self-requested a review October 19, 2024 00:00
Copy link
Collaborator

@krokoko krokoko left a comment

Choose a reason for hiding this comment

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

Tested in both Ts and Py following recent changes:

  • Sample Topics are exported correctly, tested to create and add to guardrails
  • Create guardrails using the constructor and attach them to an agent
  • Create CfnGuardrail using the L1 CDK construct, import them using fromCfnGuardrail and attach it to an agent
  • Create guardrails through the console, import them using fromGuardrailAttributes and attach it to an agent

Every scenario was working as expected. Thanks @aws-rafams , great job !

@krokoko krokoko merged commit 4e453ae into awslabs:main Oct 21, 2024
14 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(bedrock): guardrails - module refactoring to follow best practices from CDK
4 participants