Add porter bundle list command #329

vdice · 2019-05-09T19:14:30Z

Adds porter bundle list to list installed bundles, optionally with json/yaml outputs.

In doing so, I made some updates to table.go from the printer pkg, as it appeared previously we weren't actually utilizing the power of the tabWriter library for table output spacing.

  $ porter bundle list
NAME              CREATED               MODIFIED              LAST ACTION   LAST STATUS
porter-keyvault   Fri Apr 26 11:38:27   Fri Apr 26 11:38:41   install       success

 $ porter bundle list -o json
[
  {
    "Name": "porter-keyvault",
    "Created": "Fri Apr 26 11:38:27",
    "Modified": "Fri Apr 26 11:38:41",
    "Action": "install",
    "Status": "success"
  }
]

 $ porter bundle list -o yaml
- name: porter-keyvault
  created: Fri Apr 26 11:38:27
  modified: Fri Apr 26 11:38:41
  action: install
  status: success

Closes #284

carolynvs

Very close just some small help text changes and optional follow-ups.

carolynvs · 2019-05-10T13:02:17Z

cmd/porter/bundle.go

@@ -77,6 +79,35 @@ func buildBuildCommand(p *porter.Porter) *cobra.Command {
 	return cmd
 }

+func buildBundleListCommand(p *porter.Porter) *cobra.Command {
+	opts := struct {


Heads up that I made a mistake when I first did some of the original commands, defining the structs anonymously here, instead of as a type in pkg/porter like we do for other commands like Install.

It would be really nice at some point to fix these older commands to use the new and improved pattern, instead of adding more commands that use my old mistake. I'm fine with merging this, but if you feel motivated, as a follow up maybe we could fix this one and the other one you used as a pattern for this?

Ah, ok. Definitely, I'll start a follow-up.

carolynvs · 2019-05-10T13:03:02Z

cmd/porter/bundle.go

+
+	cmd := &cobra.Command{
+		Use:   "list",
+		Short: "list bundles",


Let's change this to be a bit more clear what we are listing. How about "List installed bundles"?

I see that used managed bundles down below which I really like. So maybe you have better wording to suggest? "List managed bundles"? Up to you but just "List bundles" I think is too vague, especially since duffle lists bundles in a different way than porter.

Updated....

carolynvs · 2019-05-10T13:05:45Z

cmd/porter/bundle.go

+	cmd := &cobra.Command{
+		Use:   "list",
+		Short: "list bundles",
+		Long:  `List all bundles managed by Porter`,


We can get a bit more verbose here. Maybe let them know that they can see bundles that have been installed? They may not immediately connect the word "managed" with something that they did with "porter install", you know?

Updated; let me know what you think... meanwhile, new ticket to address dissonance between bundles and clams in: #330

carolynvs · 2019-05-10T13:07:08Z

cmd/porter/bundle_test.go

+	}{
+		{"no args", "bundle list", ""},
+		{"output json", "bundle list -o json", ""},
+		{"invalid format", "bundle list -o wingdings", "invalid format: wingdings"},


haha wingdings

carolynvs · 2019-05-10T13:10:40Z

pkg/cnab/provider/list.go

+				if !ok {
+					return nil
+				}
+				return []interface{}{cl.Name, cl.Created, cl.Modified, cl.Action, cl.Status}


It would be great to either in this PR or in a follow-up format the dates in a human friendly format. I'm cool with either finding a go format that looks less verbose or using a package like go humanize.

Updated with a friendlier format; let me know what you think.

carolynvs · 2019-05-10T13:14:29Z

pkg/porter/cnab.go

@@ -18,6 +19,7 @@ type CNABProvider interface {
 	Install(arguments cnabprovider.InstallArguments) error
 	Upgrade(arguments cnabprovider.UpgradeArguments) error
 	Uninstall(arguments cnabprovider.UninstallArguments) error
+	List(opts printer.PrintOptions) error


I'm not sure if this needs to go on the CNAB provider interface. 🤔 The others were there because they were mapping to code that was all duffle implemented stuff, that later we may want to swap out with test stubs.

This is all porter's own code that doesn't need to be put into this interface. I'd be happy moving all the code out of provider/list.go into pkg/porter/list.go and removing this interface declaration.

Ah good call; moving out 👍

carolynvs · 2019-05-10T13:19:42Z

pkg/printer/table.go

+
+// tabify is a helper function which takes a slice and injects tab characters
+// between each element such that tabwriter can work its magic
+func tabify(untabified []interface{}) []interface{} {


D'oh! I always use a better package than the stdlib text/tabwriter and completely forgot to do this.

I think I'll do a follow-up later to switch to a table writer package that I like better but thank you, this fixes the stdlib for now. 👍

* update 'porter bundle list' verbiage * relocate list implementation into porter (remove from cnab/provider) * print times in a condensed, human-readable format

carolynvs

🚀

carolynvs · 2019-05-10T17:11:21Z

cmd/porter/bundle.go

-		rawFormat string
-		format    printer.Format
-	}{}
+	opts := porter.ListOptions{}


Yay! Thanks for fixing my past mistakes for me. 🙇‍♀

carolynvs · 2019-05-10T17:11:29Z

cmd/porter/bundle.go

-		Short: "list bundles",
-		Long:  `List all bundles managed by Porter`,
+		Short: "list installed bundles",
+		Long: `List all bundles installed by Porter.


carolynvs · 2019-05-10T17:12:24Z

pkg/porter/list.go

+
+const (
+	// TimeFormat is used to generate a human-readable representation of a raw time.Time value
+	TimeFormat = "Mon Jan _2 15:04:05"


👍

Seriously, I'm so going to make a prettytime pkg now.

feat(cmd): add bundle list command

2c1314d

ghost assigned vdice May 9, 2019

ghost added the review label May 9, 2019

vdice force-pushed the feat/list-bundles branch from 56882df to 96b2a9a Compare May 9, 2019 19:15

update PrintTable to properly harness tabWriter; add/update tests

766ff28

vdice force-pushed the feat/list-bundles branch from 96b2a9a to 766ff28 Compare May 9, 2019 21:03

carolynvs self-requested a review May 10, 2019 12:59

carolynvs requested changes May 10, 2019

View reviewed changes

updates per PR feedback

81658ec

* update 'porter bundle list' verbiage * relocate list implementation into porter (remove from cnab/provider) * print times in a condensed, human-readable format

vdice mentioned this pull request May 10, 2019

Porter bundles vs claims #330

Closed

carolynvs approved these changes May 10, 2019

View reviewed changes

carolynvs merged commit 23c1960 into getporter:master May 10, 2019

ghost removed the review label May 10, 2019

vdice deleted the feat/list-bundles branch May 10, 2019 17:15

vdice mentioned this pull request May 10, 2019

ref(*): remove use of anonymous options where applicable #331

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add porter bundle list command #329

Add porter bundle list command #329

vdice commented May 9, 2019 •

edited

Loading

carolynvs left a comment

carolynvs May 10, 2019

vdice May 10, 2019

carolynvs May 10, 2019

vdice May 10, 2019

carolynvs May 10, 2019

vdice May 10, 2019

carolynvs May 10, 2019

carolynvs May 10, 2019

vdice May 10, 2019 •

edited

Loading

carolynvs May 10, 2019

vdice May 10, 2019

carolynvs May 10, 2019

carolynvs left a comment

carolynvs May 10, 2019

carolynvs May 10, 2019

carolynvs May 10, 2019

Add porter bundle list command #329

Add porter bundle list command #329

Conversation

vdice commented May 9, 2019 • edited Loading

carolynvs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vdice May 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carolynvs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vdice commented May 9, 2019 •

edited

Loading

vdice May 10, 2019 •

edited

Loading