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

docs: add a notice of creating bot stack related to frontend WAF #670

Open
wants to merge 9 commits into
base: v2
Choose a base branch
from

Conversation

fbukevin
Copy link

Issue #, if available:
#633

Description of changes:
Just add a notice to README to inform future user notice the bedrock region setting. I think this information in README should be helpful for other users(either the official document or the Caution explicitly).

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

Respecting to this isssue:
aws-samples#633

I think this information in README should be helpful for other users(either the official document or the Caution explicitly).
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
DEPLOYMENT.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
There are two ways to deploy a chatbot:

- Super-easy deployment: This method is designed for the very first user of BrChat.
- Using CDK: Notice that it requires large amount of space to register container layers. Expand your disk of VM for Cloud9 before you deploy.
Copy link
Contributor

Choose a reason for hiding this comment

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

Still not fixed?

Copy link
Author

Choose a reason for hiding this comment

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

Ah sorry, I removed the previous one, but omitted the later one.

README.md Outdated
- Super-easy deployment: This method is designed for the very first user of BrChat.
- Using CDK: Notice that it requires large amount of space to register container layers. Expand your disk of VM for Cloud9 before you deploy.

Both method require usig CDK config file in cdk/cdk.js. To use custom bootstrap, please refer to [deploy.md](deploy.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

only cdk deployment requires modification of cdk.json

Copy link
Author

Choose a reason for hiding this comment

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

If you want to use another qualifier, even using bin.sh requires modification cdk.json.
We had struggle with it very much. Because in deploy.yml, it calls cdk/cdk.json.

Copy link
Contributor

Choose a reason for hiding this comment

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

As I mentioned earlier, the target users for super-easy deployment is "the very first user of BrChat". If need to change options which can only be modified via cdk.json (can not change via arguments on bin.sh), it should be an advanced usage so recommend to use deployment by CDK.

Copy link
Author

@fbukevin fbukevin Jan 14, 2025

Choose a reason for hiding this comment

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

Thanks for your confirmation. Actually, that's not work easily. Just as a caution, in the same account, the code of this project sometime will encounters resource confliction when deploying multiple chatbot. We collaborate with AWS support and addressed this issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

Understood. Could you clarify what you're ultimately trying to achieve with this PR?

fbukevin and others added 3 commits January 9, 2025 12:19
* Add dead letter queue to API stack, to limit retries

* fix DLQ name

Change SQS queue DLQ name according to https://qiita.com/tmokmss/items/721a99e9a62499d6d54a. also changed "DLQ" => "Dlq" because not using capitalize-acronym rule.
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