-
Notifications
You must be signed in to change notification settings - Fork 928
feat!: add table format to 'coder license ls' #8421
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor nits, but otherwise looks good!
enterprise/cli/licenses.go
Outdated
} else { | ||
var strs []string | ||
if lic.AllFeaturesClaim() { | ||
strs = append(strs, "all") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to add all zero features if we add all?
UUID UPLOADED AT FEATURES EXPIRES AT
1d6fab30-b9fe-40f1-8510-f054bbe252ab 2022-11-05T16:45:44Z audit_log=1, user_limit=5 2023-08-26T22:19:20Z
6a6fa845-b022-4980-953f-982617bdf74e 2023-01-19T22:01:43Z all, workspace_quota=0, audit_log=0, browser_only=0, rbac=0, scim=0, user_limit=0 2023-10-10T21:01:34Z
In this case, I think simply omitting those would make sense: all, workspace_quota=0, audit_log=0, browser_only=0, rbac=0, scim=0, user_limit=0
-> all
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was honestly curious why we even include the claim if it's 0.
You are right though, I'll omit 0s.
codersdk/licenses.go
Outdated
return time.Time{}, xerrors.Errorf("license_expires claim has unexpected type %T", expClaim) | ||
} | ||
|
||
func (l *License) Trail() bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/Trail/Trial/g
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦
for i := range list { | ||
humanExp, err := list[i].ExpiresAt() | ||
if err == nil { | ||
list[i].Claims[codersdk.LicenseExpiryClaim+"_human"] = humanExp.Format(time.RFC3339) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say both this and the change in default output would warrant a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair 👍
What this does
Closes #7697
If anyone has an idea on a better way to format the claims, lmk.
We might want to just move the License struct to the
codersdk
so we can json unmarshal the claims easier.Behavior changes
license_expires
claim in the json payload from a unix timestamp to a RFC3339 string in the-o json
output. I kept this, but moved it tolicense_expires_human
. I did this to maintain the original rational, but also preserve the original payload. The original reasoning was probably because the table view did not exist.coder licenses ls -o json
for the json output.