8000 feat!: add table format to 'coder license ls' by Emyrk · Pull Request #8421 · coder/coder · GitHub
[go: up one dir, main page]

Skip to content

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

Merged
merged 8 commits into from
Jul 12, 2023
Merged

Conversation

Emyrk
Copy link
Member
@Emyrk Emyrk commented Jul 11, 2023

What this does

Closes #7697

UUID                                  UPLOADED AT           FEATURES      EXPIRES AT                 
c9d1cbfa-6a8c-49f7-8999-82fc64934aef  2023-02-03T18:24:27Z  all features  2024-02-03T13:21:02-05:00 

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

  • We changed the 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 to license_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.
  • Table view is now the default. coder licenses ls -o json for the json output.

@Emyrk Emyrk requested a review from mafredri July 11, 2023 17:05
Copy link
Member
@mafredri mafredri left a 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!

} else {
var strs []string
if lic.AllFeaturesClaim() {
strs = append(strs, "all")
Copy link
Member

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.

Copy link
Member Author

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.

return time.Time{}, xerrors.Errorf("license_expires claim has unexpected type %T", expClaim)
}

func (l *License) Trail() bool {
Copy link
Member

Choose a reason for hiding this comment

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

s/Trail/Trial/g

Copy link
Member Author

Choose a reason for hiding this comment

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

🤦

8000
for i := range list {
humanExp, err := list[i].ExpiresAt()
if err == nil {
list[i].Claims[codersdk.LicenseExpiryClaim+"_human"] = humanExp.Format(time.RFC3339)
Copy link
Member

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair 👍

@Emyrk Emyrk changed the title feat: add table format to 'coder license ls' feat!: add table format to 'coder license ls' Jul 12, 2023
@github-actions github-actions bot added the release/breaking This label is applied to PRs to detect breaking changes as part of the release process label Jul 12, 2023
@Emyrk Emyrk merged commit 928091a into main Jul 12, 2023
@Emyrk Emyrk deleted the stevenmasley/license_table branch July 12, 2023 12:06
@github-actions github-actions bot locked and limited conversation to collaborators Jul 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release/breaking This label is applied to PRs to detect breaking changes as part of the release process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: add table format for coder license list
2 participants
0