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

feat: adds origin to config whoami command #307

Merged
merged 1 commit into from
Feb 26, 2021

Conversation

EverlastingBugstopper
Copy link
Contributor

@EverlastingBugstopper EverlastingBugstopper commented Feb 23, 2021

fixes #273

rover config whoami now shows the user the profile that their credentials are stored in, OR if Rover is reading their credentials from the $APOLLO_KEY environment variable.

output:

$ rover config whoami --profile graph
Checking identity of your API key against the registry.
Key Type: GRAPH
Graph Title: averys-giant-sandy-box
Unique Graph ID: averys-giant-sandy-box
Origin: --profile graph
$ APOLLO_KEY=mygraphkey rover config whoami
Checking identity of your API key against the registry.
Key Type: GRAPH
Graph Title: averys-giant-sandy-box
Unique Graph ID: averys-giant-sandy-box
Origin: $APOLLO_KEY

i am a tad worried that adding houston as a dep to rover-client is a bad design decision, but I thought long and hard about how else we could do this and I came up empty handed. If anybody has any suggestions on how to maybe decouple those (or if you think it's fine) I'm all ears!

Also, do we think exposing the actual file location is good here? Or do we think we should just say the name of the profile itself? I'd imagine it'd be useful to know the file location, but also showing it like this might give folks the wrong idea. We'd much rather them interact with this stuff through our config subcommand suite, so maybe we shouldn't lift the curtains on that abstraction.


after this i think i'm gonna try to lump in rover config show with whoami!

@EverlastingBugstopper EverlastingBugstopper added the feature 🎉 new commands, flags, functionality, and improved error messages label Feb 23, 2021
@EverlastingBugstopper EverlastingBugstopper added this to the March 9 milestone Feb 23, 2021
Copy link
Member

@lrlna lrlna left a comment

Choose a reason for hiding this comment

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

I think it's fine to bring in Houston in like this, as it's essentially used for specificity.

I am not entirely sure about the showing the physical path, which you pointed out in your description, because I feel like users may not know what to do with that information? Since they are usually adding keys with the profile command, I am wondering if we can very specific with:

Profile: default

or

Profile: work_dev

@EverlastingBugstopper
Copy link
Contributor Author

Yeah I think using the profile name is better here - I'll push an update.

`rover config whoami` now shows the user the file path
that their credentials are stored in, OR if Rover is
reading their credentials from the $APOLLO_KEY environment
variable
@EverlastingBugstopper
Copy link
Contributor Author

EverlastingBugstopper commented Feb 26, 2021

How do we feel about

Checking identity of your API key against the registry.
Key Type: GRAPH
Graph Title: averys-giant-sandy-box
Unique Graph ID: averys-giant-sandy-box
Origin: --profile graph

instead of Profile: graph?

@JakeDawkins
Copy link
Contributor

I like that better. Since we reference profiles the same way everywhere, that seems very reasonable as a hint on how to change it. I think it feels a little weird for the default, but I could handle that

@EverlastingBugstopper EverlastingBugstopper merged commit 956d226 into main Feb 26, 2021
@EverlastingBugstopper EverlastingBugstopper deleted the avery/whoami-origin branch February 26, 2021 17:46
@EverlastingBugstopper
Copy link
Contributor Author

@lrlna I merged this but if you're strongly opposed to the Origin: --profile graph please let me know and we can patch it up nice and quick 😄

@lrlna
Copy link
Member

lrlna commented Mar 2, 2021

Ohh that looks good to me! Not opposed at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🎉 new commands, flags, functionality, and improved error messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

whoami should display origin of API key
3 participants