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

Create new segment for AWS profile prompt segment #299

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fernandocarletti
Copy link

This change adds the $AWS_PROFILE as a new segment in the prompt. I took the previous existing AWS Vault as base and reused the colors.

@fernandocarletti fernandocarletti changed the title Create new segment for AWS profile Create new segment for AWS profile prompt segment Apr 13, 2021
@darrenkidd
Copy link
Contributor

Hey @fernandocarletti - nice work!

I authored the AWS Vault segment, and have since thought about doing the AWS profile as well - but you've done the work for me! Cheers!

However, I'm wondering whether we should merge the two together i.e. a generic "AWS segment" that activated on either the vanilla AWS CLI tools or the AWS vault tool. Somewhat prevents crazy config explosion and seems to make sense to keep all the same kinds of things nicely grouped together. Thoughts?

@fernandocarletti
Copy link
Author

Hey @darrenkidd! If I recall correctly, this was actually my initial approach to this, but I don't remember exactly why I moved away from it. But I agree with you, it makes more sense to have it in a single configuration.

I can change the implementation and get your input once it is ready. Sounds good to you?

@darrenkidd
Copy link
Contributor

I can change the implementation and get your input once it is ready. Sounds good to you?

Sounds great!

IMO I think we should prioritise $AWS_PROFILE first i.e. use that and then go looking for $AWS_VAULT or others if we don't find it. It's the more standard one.

I also think we should move to $theme_display_aws_profile as the main var and remove $theme_display_aws_vault_profile. For backwards compatibility we'll need to keep the latter around in our prompt function for a while, though.

From memory the AWS CLI also puts session expiry information into env vars too, so we should be able to leverage that now as well? Can rename color_aws_vault_expired to color_aws_expired then?

@fernandocarletti
Copy link
Author

@darrenkidd it is finally done! Can you take a look if it fits what you imagined? Best!

@darrenkidd
Copy link
Contributor

Nice, @fernandocarletti. I've had a skim but need a bit more time to think and digest. I just want to double-check it handles both tools well with no surprises!

@kamilpp
Copy link

kamilpp commented Feb 25, 2022

Hi, any chances to get this into mainstream anytime soon? This is a small change while quite useful, would be great to get it merged :)

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