feat: implement organization context in the cli#12259
Conversation
d4d20af to
1ac16cf
Compare
| Use: "organizations { current }", | ||
| Short: "Organization related commands", | ||
| Aliases: []string{"organization", "org", "orgs"}, |
There was a problem hiding this comment.
Should we instead do something like coder ctx current?
There was a problem hiding this comment.
I'd be interested in discussing this further, ultimately I'd like for us to support config files for the CLI too, you might not want same behavior across all orgs. Having this support those use-cases as well would be nice.
There was a problem hiding this comment.
Good point. This command is marked as hidden, so we can tweak it before we release.
Multi orgs is still a bit out, but this is going to be really helpful in development.
| Use: "organizations { current }", | ||
| Short: "Organization related commands", | ||
| Aliases: []string{"organization", "org", "orgs"}, | ||
| Hidden: true, // Hidden until these commands are complete. |
There was a problem hiding this comment.
I marked this as hidden, I think we can defer the actual command name discussion and get the code in. Don't want to bikeshed and prevent code progress.
| } | ||
| } | ||
|
|
||
| pty.ExpectMatch("context canceled") |
| return codersdk.Organization{}, nil | ||
| return codersdk.Organization{}, err |
There was a problem hiding this comment.
@mafredri The unit test expecting context cancelled had to be changed because of this change. (#12259 (comment))
Before, if you failed to fetch the orgs, we'd return a nil uuid. I made this api failure fail the command now. So the unit tests fails differently now.
There was a problem hiding this comment.
I wouldn't expect TestTemplateCreate/WithVariablesFileWithoutRequiredValue to be testing for org failure though 🤔
There was a problem hiding this comment.
I didn't write the test, but there is a comment indicating this cli error is expected. We cancel the context before running the command.
coder/cli/templatecreate_test.go
Lines 238 to 244 in 98666fd
There was a problem hiding this comment.
If we're testing against a cancelled context, that test is useless. I think it should either be removed or fixed. 😅 It's definitely not testing what the test name is indicating.
mafredri
left a comment
There was a problem hiding this comment.
I'm not happy with that test that tests for context cancellation, but not letting that block this PR, so approved. 👍🏻
Yea, I'm not 100% sure what the test is intended to do because the prior behavior makes little sense to me as well. |
What this does
The cli now uses the config files to store what organization it is currently referencing.
Adds
coder organizations currentto display the currently selected organization.This is a hidden command until we finish all related commands
Discussion
Currently I have this under
coder organizations { current | switch | ... }. Should we name this something else? Likecoder ctx current,coder ctx switch` etc?This could maybe include different deployments and logins if we do
ctx.coder organizations currentBelow is all the format options.