feat: Option to list all workspaces #416
Conversation
From an admin POV, this will be helpful for various reasons.
| { | ||
| name: "simple list", | ||
| command: []string{"workspaces", "ls", "--all"}, | ||
| assert: func(r result) { r.success(t) }, |
There was a problem hiding this comment.
Sanity check: This isn't actually checking the output - just that the command exited with 0?
There was a problem hiding this comment.
Essentially, yeah. This mini test framework "runs" the commands in-process and checks if the command fn returned an error.
internal/cmd/workspaces.go
Outdated
| } | ||
| if provider != "" { | ||
| if provider != "" || all { |
There was a problem hiding this comment.
This means the --all flag only works with the --provider flag. I think that's OK for now, but it needs to be made obvious in the flag description (e.g. "requires --provider flag") and there should be a check at the top of the command to ensure --provider is set if --all is set.
$ ./coder envs ls --all
fatal: resource not found
There was a problem hiding this comment.
Doesn't it mean it works regardless of the provider flag? If I call coder with just --all, provider is "", and the logic becomes if "" != "" || true -> if false || true -> if true. This was my intent - as an admin, I want to see all workspaces.
But looking at the actual logic/implementation for getWorkspacesByProvider, I think your point still stands. I can fix that.
|
This looks great, just needs a few changes and it'll be solid 😄 |
goodspark
changed the title
Co-authored-by: Dean Sheather <[email protected]>
goodspark
force-pushed
the
ls-all-workspaces
branch
from
56b570a to
e670978
Compare
internal/cmd/workspaces.go
Outdated
| } else if provider != "" { | ||
| var err error | ||
| workspaces, err = getWorkspacesByProvider(ctx, client, provider, user) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } else { | ||
| var err error | ||
| workspaces, err = getWorkspaces(ctx, client, user) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } |
There was a problem hiding this comment.
Shuffled this around because it seemed like the first fetch for the user-only workspaces was ignored if providers was also passed.
internal/cmd/workspaces.go
Outdated
| if provider != "" { | ||
| var workspaces []coder.Workspace | ||
| if all { | ||
| var err error |
There was a problem hiding this comment.
You shouldn't need the var errs here or in the other if branches since it's already defined above, then you can condense the if err check to be underneath the if branches, so each branch is just the workspaces, err = getX() statement.
edit: check my suggestion
Co-authored-by: Dean Sheather <[email protected]>
From an admin POV, this will be helpful for various reasons.