Sort packages, actions, triggers, rules by name alphabetically. #1729

lzbj · 2017-01-17T07:03:04Z

Address issue #1569.

rabbah · 2017-01-17T11:51:46Z

I would make this a feature toggled by a flag: sort by date or alphabetelically.

csantanapr · 2017-01-17T20:41:59Z

@rabbah 😕 date? I'm not aware that entities like rules, packages, etc.. have a date associated to it.

rabbah · 2017-01-17T20:45:11Z

The current order is by date (last updated eg). All entities when stored in the database have a generated date property (it's hidden from the user) if the entity doesn't have an obvious property to use as date (which is the case for activations for example).

csantanapr · 2017-01-17T22:36:56Z

@rabbah ah so your saying that the REST API already support a sort parameter that CLI can pass to it to sort by date or alphabetically?

csantanapr · 2017-01-17T22:38:06Z

I tried wsk list and wsk rule get with -v and was not able to see a date property in the return data. I was just trying to understand where is this sorting that your proposing happens client or server side.

rabbah · 2017-01-17T22:51:23Z

No I'm saying records in the database have a date field. It is not visible to you as the end user.

csantanapr · 2017-01-17T23:02:59Z

OK How are you proposing we implement this sorting featured flag?

It looks like it can't be done ONLY with CLI changes.

Something along the lines to add support in the backend to provide a parameter that get's used to return the sorted list, or return the list with the date information for the client to do the sorting.

rabbah · 2017-01-17T23:04:50Z

We already have sort by date. This is the current ordering. I'm simply proposing that you choose a default (name vs date) and guard the sort in the cli by such a flag.

csantanapr · 2017-01-17T23:12:34Z

Ah see, make sense now. I think it's being a long day for me 🤣 going home now.

@lzbj I assume you understood the propose change, very simple to handle.

I propose flag name sort possible values date or name

My OCD tells me that default when flag not specify should be name
But practical person says date

I'm OK flipping a coin, or if someone has a strong opinion

rabbah · 2017-01-17T23:53:02Z

Thought: sort by name by default and add -t to sort by time (matching ls -t for example).
For activations however should we support sorting lexically? One thought is to sort lexically by the corresponding action/trigger/rule name and within each group sort by time. And the flag -t could then flatten the grouping... Seems confusing though.

lzbj · 2017-01-18T02:14:53Z

@rabbah ,@csantanapr, I'll try to figure out and and a flag. Thanks for review.

rabbah · 2017-01-24T11:59:04Z

@lzbj I added wip flag - remove it when ready and apply review label.

rabbah · 2017-01-31T05:09:11Z

Looks good. A test should be added. I suggest WskBasicUsageTests class.

lzbj · 2017-02-06T02:10:44Z

Thanks, I'll add a basic test cases for it. @rabbah ,pls help review, thank you.

dubee · 2017-02-21T16:48:28Z

Needs a PG.

lzbj · 2017-02-22T02:46:36Z

@dubeejw , could you please help run a PG? thank you.

rabbah

@dubeejw is all the code duplication really necessary?
does the CLI have some common base types where the comparators can be added?

Would be also nice to inherit common flags.

rabbah · 2017-02-22T16:04:29Z

tests/src/whisk/core/cli/test/WskBasicUsageTests.scala

+            try {
+                val namespace = wsk.namespace.list().stdout.trim.split("\n").last
+                val env = Map("WSK_CONFIG_FILE" -> tmpwskprops.getAbsolutePath())
+                wsk.cli(Seq("property", "set", "-i", "--apihost", wskprops.apihost, "--auth", wskprops.authKey,


using a config file seems like overkill - this is to avoid adding sort to the Wsk.list method?

Correct, I'll remove it.

rabbah · 2017-02-22T16:05:17Z

tests/src/whisk/core/cli/test/WskBasicUsageTests.scala

+                        action.create(name2, file2, annotations = getValidJSONTestArgInput,
+                            parameters = getValidJSONTestArgInput)
+                }
+                val stdout = wsk.cli(Seq("list", "-i", "--apihost", wskprops.apihost, "--auth", wskprops.authKey), env = env).stdout


I think a better approach is to change list to accept sort.

ok, add it.

rabbah

see comments.

dubee · 2017-02-22T16:35:14Z

tools/cli/go-whisk-cli/commands/namespace.go

@@ -119,8 +119,10 @@ var listCmd = &cobra.Command{
 }

 func init() {
+    listCmd.Flags().BoolVarP(&flags.namespace.date, "sort", "t", false, wski18n.T("sort by date"))


Looks like this flag will work for wsk list, but not for individual list commands like wsk action list. Also, I don't think the flag name is very descriptive.

Yes, just to address issue #1569, ok, I'll try to change a better one for your review.

dubee · 2017-02-22T16:41:40Z

tools/cli/go-whisk/whisk/trigger.go

@@ -46,6 +46,20 @@ type TriggerListOptions struct {
    Docs            bool            `url:"docs,omitempty"`
 }

+type TriggerArray []Trigger


I think you might be able to create functions that expect an generic interface {} type. Then cast to the appropriate type for Name comparison. This way there isn't specific sort functions needed for triggers, actions, rules, and packages. See https://golang.org/doc/effective_go.html#type_switch.

Ok, I'll try this approach, my initial though was to let entities like action, trigger implements some interface which is convenient for container manipulates etc, but anyway, if you think it's not necessary, let's change it.

csantanapr · 2017-02-22T18:38:46Z

@rabbah assigning to you since you are already reviewing this PR

lzbj · 2017-02-23T02:26:39Z

@rabbah @dubeejw @csantanapr ,thank you, I'll try to update accordingly.

cfjedimaster · 2017-03-02T16:33:45Z

As just an FYI - and sorry if I missed this in the thread - but I'd argue for sorting by alpha to be the default. I can see supporting date sorting too as an option, but I'd imagine it being more normal to have an alpha sort.

rabbah · 2017-03-16T03:04:25Z

@lzbj are you still working on this? It would be nice to get this in.

lzbj · 2017-03-16T03:15:06Z

@rabbah , not recently, I'll work on this when available.

rabbah · 2017-04-10T20:02:11Z

@RSulzmann can you take this over? it's fairly close and I think @lzbj isn't actively working on this. David, can you provide an update if you are still working on this?

lzbj · 2017-04-11T01:19:49Z

@rabbah , yes, I'll provide an update.

lzbj · 2017-04-11T08:44:47Z

tools/cli/go-whisk-cli/commands/util.go

@@ -285,8 +285,26 @@ func printSummary(collection interface{}) {
    }
 }

+func Len(collection interface{}) func() int {


@dubeejw , is your comments means this kind of function? I thought perhaps the action array should implement the sort interfaces, right?

By default will sort by entity name alphabetically. Address issue apache#1569 for enhancement request.

lzbj · 2017-04-11T08:47:38Z

@rabbah , I just added a simple function to discuss with dubeejw about his comments, anyhow, feel free to reassign other people if this high priority.

rabbah · 2017-06-13T15:29:41Z

Closing in favor of #2326.

lzbj force-pushed the issue#1569 branch 2 times, most recently from a9aa8eb to d10ed7a Compare January 19, 2017 00:55

rabbah added cli wip labels Jan 24, 2017

lzbj added review Review for this PR has been requested and yet needs to be done. and removed wip labels Jan 25, 2017

rabbah assigned dubee Jan 31, 2017

lzbj force-pushed the issue#1569 branch from d10ed7a to b949d04 Compare February 9, 2017 06:53

csantanapr requested review from dubee and rabbah February 22, 2017 15:53

csantanapr assigned csantanapr and unassigned dubee Feb 22, 2017

rabbah reviewed Feb 22, 2017

View reviewed changes

rabbah requested changes Feb 22, 2017

View reviewed changes

dubee reviewed Feb 22, 2017

View reviewed changes

csantanapr assigned rabbah and unassigned csantanapr Feb 22, 2017

lzbj force-pushed the issue#1569 branch from b949d04 to ac494b4 Compare April 11, 2017 08:41

lzbj commented Apr 11, 2017

View reviewed changes

Add flag to sort listed entities by date.

ac494b4

By default will sort by entity name alphabetically. Address issue apache#1569 for enhancement request.

markusthoemmes assigned markusthoemmes and unassigned rabbah Apr 25, 2017

christ unassigned markusthoemmes May 12, 2017

rabbah mentioned this pull request Jun 1, 2017

Alphabetize listings in CLI #2326

Merged

rabbah closed this Jun 13, 2017

Sort packages, actions, triggers, rules by name alphabetically. #1729

Sort packages, actions, triggers, rules by name alphabetically. #1729

Conversation

lzbj commented Jan 17, 2017

rabbah commented Jan 17, 2017

csantanapr commented Jan 17, 2017

rabbah commented Jan 17, 2017

csantanapr commented Jan 17, 2017 • edited Loading

csantanapr commented Jan 17, 2017

rabbah commented Jan 17, 2017

csantanapr commented Jan 17, 2017

rabbah commented Jan 17, 2017

csantanapr commented Jan 17, 2017

rabbah commented Jan 17, 2017

lzbj commented Jan 18, 2017

rabbah commented Jan 24, 2017

rabbah commented Jan 31, 2017

lzbj commented Feb 6, 2017 • edited Loading

dubee commented Feb 21, 2017

lzbj commented Feb 22, 2017

rabbah left a comment

Choose a reason for hiding this comment

rabbah Feb 22, 2017

Choose a reason for hiding this comment

lzbj Feb 24, 2017

Choose a reason for hiding this comment

rabbah Feb 22, 2017

Choose a reason for hiding this comment

lzbj Feb 24, 2017

Choose a reason for hiding this comment

rabbah left a comment

Choose a reason for hiding this comment

dubee Feb 22, 2017 • edited Loading

Choose a reason for hiding this comment

lzbj Feb 24, 2017

Choose a reason for hiding this comment

dubee Feb 22, 2017 • edited Loading

Choose a reason for hiding this comment

lzbj Feb 24, 2017

Choose a reason for hiding this comment

csantanapr commented Feb 22, 2017

lzbj commented Feb 23, 2017

cfjedimaster commented Mar 2, 2017

rabbah commented Mar 16, 2017

lzbj commented Mar 16, 2017

rabbah commented Apr 10, 2017

lzbj commented Apr 11, 2017

lzbj Apr 11, 2017

Choose a reason for hiding this comment

lzbj commented Apr 11, 2017

rabbah commented Jun 13, 2017

csantanapr commented Jan 17, 2017 •

edited

Loading

lzbj commented Feb 6, 2017 •

edited

Loading

dubee Feb 22, 2017 •

edited

Loading

dubee Feb 22, 2017 •

edited

Loading