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

assistant: Improve Amazon Bedrock configuration instructions #25699

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

Conversation

5herlocked
Copy link
Contributor

Based on question, discussion here -- #16544 (comment)

Plus the fact that it wasn't clear to anyone who isn't familiar with AWS, updating the UX to more friendly and guided.

Release Notes:

  • N/A

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Feb 26, 2025
@zed-industries-bot
Copy link

zed-industries-bot commented Feb 26, 2025

Messages
📖

This PR includes links to the following GitHub Issues: #16544
If this PR aims to close an issue, please include a Closes #ISSUE line at the top of the PR body.

Generated by 🚫 dangerJS against 2d881db

@maxdeviant maxdeviant changed the title bedrock: Updated UX bedrock: Update UX Feb 26, 2025
@maxdeviant
Copy link
Member

@5herlocked Could you please include a screenshot of the changes?

@5herlocked
Copy link
Contributor Author

5herlocked commented Feb 27, 2025

image

Old


image

New


In my head I'd like the labels vs. the editors to be justified but I haven't given it much thought

@maxdeviant maxdeviant self-assigned this Feb 27, 2025
@danilo-leal
Copy link
Contributor

Oops, hadn't seen this before I opened #25745 — also touched on this there!

I like your changes, and mine are also hopefully an improvement, but I'm feeling like the ultimate/best fix for this is figuring out a more classic, web-like input component with label, input, and placeholder.

@5herlocked
Copy link
Contributor Author

5herlocked commented Feb 27, 2025

So long-term, we would actually prefer if the user never have to use this.
We're working on a crate that will integrate into the variety of ways people interact with AWS, think SSO, CLI, etc.

But I love your changes because it makes it feel a lot more "cozy"

# Conflicts:
#	crates/language_models/src/provider/bedrock.rs
@5herlocked
Copy link
Contributor Author

5herlocked commented Feb 27, 2025

image

Merged the two, if we like it, we can send it -- imo looks pretty close to very good.

@danilo-leal danilo-leal changed the title bedrock: Update UX assistant: Improve Amazon Bedrock configuration instructions Feb 27, 2025
@danilo-leal
Copy link
Contributor

danilo-leal commented Feb 27, 2025

Pushed an additional update here—looking ok for me!

Felt like keeping the "XXX" as placeholder because they (at least should) indicate the correct number of characters. Unsure whether we have actual, web-like validation to actually accept just a specific number, but at least indicating that is helpful.

@5herlocked
Copy link
Contributor Author

Yeah definitely good shout --

If we want to actually have an exact number of Placeholder 'X's - Access Key Id is usually around 16 characters and Secret Access Key is around 40 characters.

If we wanted validation down the line -- https://github.com/awslabs/git-secrets/blob/5357e18bc27b42a827b6780564ea873a72ca1f01/git-secrets#L234

which also contains AWS approved examples for AAID and SK's

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants