Allow optional leading slash in CLI commands #2424

underwoodb-sd-ibm · 2017-06-26T20:23:58Z

This update is intended to allow the user to input a 3 part fully qualified name (FQN) with or without a leading '/' whenever entering a command.

Closes #2328

underwoodb-sd-ibm · 2017-06-29T20:32:43Z

PG2 1762
Failed

dubee

Might want to test for triggers, rules, etc. Need to update the docs as well.

underwoodb-sd-ibm · 2017-06-30T14:37:02Z

PG4 477 - Passed

underwoodb-sd-ibm · 2017-07-06T19:48:47Z

@mdeuser - in case you hadn't seen this, for when/if you have time for further review.

mdeuser · 2017-07-06T21:43:25Z

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

+// replaceMissingLeadSlash(name) returns a (possibly fully qualified) resource name,
+//      inserting a leading '/' if it is of 3 parts (namespace/packageName/actionName)
+//      and lacking the leading '/'.
+func replaceMissingLeadSlash(name string) string {


Would it be more clear if this were a method on the QualifedName struct? Perhaps rename it as well to a name that implies it only really works on a fully qualified entity name

I suppose it might be time for the QualifiedName struct and associated methods, constructors, etc to reside in its own module.

mdeuser · 2017-07-06T21:46:30Z

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

    if  strings.HasPrefix(name, "/")  {
-        parts := strings.Split(name, "/")
+        //parts = strings.Split(name, "/")


Remove commented out code

mdeuser · 2017-07-06T21:46:36Z

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

@@ -112,7 +126,7 @@ func parseQualifiedName(name string) (QualifiedName, error) {
            return qualifiedName, err
        }

-        parts := strings.Split(name, "/")
+        //parts = strings.Split(name, "/")


Remove commented out code

mdeuser · 2017-07-06T22:02:37Z

tests/src/test/scala/system/basic/WskBasicTests.scala

@@ -95,6 +97,41 @@ class WskBasicTests
            stderr should include(errormsg)
    }

+    it should "accept a 3 part Fully Qualified Name without a leading '/'" in {
+        val packageName = "packageName"


I recommend using labels that can uniquely identify this particular test. If there's a need to traverse the backend logs, having unique labels is helpful

underwoodb-sd-ibm · 2017-07-11T22:31:03Z

PG4 511 Passed

dubee · 2017-07-14T14:21:20Z

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

@@ -0,0 +1,207 @@
+/*


I think the standard naming conventions for go files are to separate words using an underscore. Ie, quailifed_name.go.

dubee · 2017-07-14T14:22:54Z

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

+
+//  GetNamespaceFromQN() returns the name of the namespace in qualifiedName without
+//      a leading '/'
+func (qualifiedName *QualifiedName) GetNamespaceFromQN() string {


I think you can drop FromQN since it isn't in the other function names. Since the type is a QualifiedName we can assume we can assume the from aspect.

dubee · 2017-07-14T14:26:27Z

tests/src/test/scala/system/basic/WskBasicTests.scala

+        try {
+            validArgs foreach {
+                cmd =>
+                    val stderr = wsk.cli(cmd ++ wskprops.overrides ++ auth, expectedExitCode = SUCCESS_EXIT).stderr


cmd => wsk.cli(cmd ++ wskprops.overrides ++ auth, expectedExitCode = SUCCESS_EXIT)

Don't need to check standard error here since we are expecting a SUCCESS_EXIT code.

dubee · 2017-07-14T14:26:42Z

tests/src/test/scala/system/basic/WskBasicTests.scala

+            }
+        } finally {
+            cleanUp foreach {
+                cmd =>


Drop the new line after =>.

dubee

@underwoodb-sd-ibm, please put the link to the issue in your PR description and prepend with "Closes ".

dubee · 2017-07-14T14:42:14Z

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

+    "../wski18n"
+)
+
+type QualifiedName struct {


I like that all the qualified name functions are now part of the QualifiedName type. Having all the functionality for a type consolidated in one place make the code easier to follow. 👍

dubee · 2017-07-14T16:24:33Z

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

+        qualifiedName.namespace = parts[1]
+
+        if len(parts) < 2 || len(parts) > 4 {
+            whisk.Debug(whisk.DbgError, "A valid qualified name was not detected\n")


This code is duplicated below. Can move the body of the if block to a function and just call that where needed.

dubee · 2017-07-14T16:26:12Z

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

@@ -34,8 +34,8 @@ func entityNameError(entityName string) (error) {
    return whisk.MakeWskError(errors.New(errMsg), whisk.EXITCODE_ERR_GENERAL, whisk.DISPLAY_MSG, whisk.DISPLAY_USAGE)
 }

-func parseQualifiedNameError(entityName string, err error) (error) {
-    whisk.Debug(whisk.DbgError, "parseQualifiedName(%s) failed: %s\n", entityName, err)
+func NewQualifiedNameError(entityName string, err error) (error) {


Can this be moved into qualified_name.go?

rabbah · 2017-08-08T18:39:51Z

need to rebase.

underwoodb-sd-ibm · 2017-08-08T19:14:03Z

PG2 1922

underwoodb-sd-ibm · 2017-08-09T00:37:27Z

PG2 1922 - Passed

rabbah · 2017-08-09T19:33:15Z

@underwoodb-sd-ibm can you move the tests out of the system package? sorry just noticed that.
don't need to re-pg.

rabbah

see comment

…thout leading slash '/' - Updated docs, fixed bug when creating Rule, improved and moved tests to WskBasicTests - Created new module for QualifiedName struct and functions * Built constructor and getter methods for QualifiedName, modified code to recognize QualifiedName pointers * Changed qualifiedname.go to qualified_name.go * Updated code to use getter functions for QualifiedName

underwoodb-sd-ibm · 2017-08-10T12:06:59Z

@rabbah, tests have been moved to WskBasicUsageTests. Please confirm where I put it in the test is satisfactory, and if so, I'll tag Carlos for merging. Thanks!

dubee · 2017-08-10T14:47:00Z

@underwoodb-sd-ibm, test is in the correct suite.

underwoodb-sd-ibm · 2017-08-10T14:54:15Z

@csantanapr - I'm assuming we need to wait for @rabbah to give the ok, but is it possible to merge this guy when/if you have time? Thanks!

csantanapr · 2017-08-10T15:08:21Z

@rabbah if you are OK with the recent changes then adjust your review and merge away.

rabbah

thanks for moving the tests.

…thout leading slash in CLI (apache#2424)

underwoodb-sd-ibm closed this Jun 27, 2017

underwoodb-sd-ibm reopened this Jun 27, 2017

underwoodb-sd-ibm changed the title ~~(WIP) Leading Slash issue #2328~~ (WIP) Leading Slash (issue #2328) Jun 27, 2017

underwoodb-sd-ibm changed the title ~~(WIP) Leading Slash (issue #2328)~~ (WIP) Leading Slash (CLI issue #2328) Jun 27, 2017

underwoodb-sd-ibm force-pushed the leadSlash2328 branch from 7085175 to 02691a5 Compare June 29, 2017 16:43

dubee requested changes Jun 29, 2017

View reviewed changes

rabbah added cli enhancement review Review for this PR has been requested and yet needs to be done. labels Jun 29, 2017

underwoodb-sd-ibm force-pushed the leadSlash2328 branch from 02691a5 to 4848b15 Compare June 30, 2017 14:03

underwoodb-sd-ibm changed the title ~~(WIP) Leading Slash (CLI issue #2328)~~ (Review) Leading Slash (CLI issue #2328) Jun 30, 2017

mdeuser requested changes Jul 6, 2017

View reviewed changes

underwoodb-sd-ibm force-pushed the leadSlash2328 branch 3 times, most recently from 0d6b9aa to 475de08 Compare July 11, 2017 18:10

underwoodb-sd-ibm changed the title ~~(Review) Leading Slash (CLI issue #2328)~~ (Review) Leading Slash Closes https://github.com/apache/incubator-openwhisk/issues/2328 Jul 11, 2017

underwoodb-sd-ibm changed the title ~~(Review) Leading Slash Closes https://github.com/apache/incubator-openwhisk/issues/2328~~ (Review) Leading Slash Closes $https://github.com/apache/incubator-openwhisk/issues/2328 Jul 11, 2017

underwoodb-sd-ibm changed the title ~~(Review) Leading Slash Closes $https://github.com/apache/incubator-openwhisk/issues/2328~~ (Review) Leading Slash (CLI issue #2328) Jul 11, 2017

underwoodb-sd-ibm force-pushed the leadSlash2328 branch from 475de08 to 1f36484 Compare July 11, 2017 19:25

underwoodb-sd-ibm force-pushed the leadSlash2328 branch from 1f36484 to 4cefccf Compare July 12, 2017 15:05

dubee requested changes Jul 14, 2017

View reviewed changes

dubee reviewed Jul 14, 2017

View reviewed changes

underwoodb-sd-ibm force-pushed the leadSlash2328 branch from 4cefccf to 740926f Compare July 14, 2017 15:56

dubee requested changes Jul 14, 2017

View reviewed changes

underwoodb-sd-ibm force-pushed the leadSlash2328 branch from 740926f to 050cc80 Compare July 14, 2017 17:42

underwoodb-sd-ibm force-pushed the leadSlash2328 branch 2 times, most recently from a972611 to 31b3cd0 Compare August 8, 2017 19:11

underwoodb-sd-ibm force-pushed the leadSlash2328 branch 3 times, most recently from d2b8312 to 11fb9f0 Compare August 9, 2017 18:12

rabbah requested changes Aug 9, 2017

View reviewed changes

underwoodb-sd-ibm force-pushed the leadSlash2328 branch from 11fb9f0 to ff7e59a Compare August 9, 2017 20:49

csantanapr assigned rabbah Aug 10, 2017

rabbah approved these changes Aug 11, 2017

View reviewed changes

rabbah merged commit c2c44c3 into apache:master Aug 11, 2017

houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Aug 17, 2017

Allow FQN with 3 parts (namespace/package/action) to be input with/wi…

b2de585

…thout leading slash in CLI (apache#2424)

houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Aug 17, 2017

Allow FQN with 3 parts (namespace/package/action) to be input with/wi…

8e6254d

…thout leading slash in CLI (apache#2424)

houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Sep 18, 2017

Allow FQN with 3 parts (namespace/package/action) to be input with/wi…

9b26034

…thout leading slash in CLI (apache#2424)

houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Sep 18, 2017

Allow FQN with 3 parts (namespace/package/action) to be input with/wi…

eea71f2

…thout leading slash in CLI (apache#2424)

houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 13, 2017

Allow FQN with 3 parts (namespace/package/action) to be input with/wi…

5de0820

…thout leading slash in CLI (apache#2424)

houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 13, 2017

Allow FQN with 3 parts (namespace/package/action) to be input with/wi…

0dcfaf3

…thout leading slash in CLI (apache#2424)

houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 13, 2017

Allow FQN with 3 parts (namespace/package/action) to be input with/wi…

dd410b9

…thout leading slash in CLI (apache#2424)

houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 13, 2017

Allow FQN with 3 parts (namespace/package/action) to be input with/wi…

2cbe6aa

…thout leading slash in CLI (apache#2424)

houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 14, 2017

Allow FQN with 3 parts (namespace/package/action) to be input with/wi…

e6f6567

…thout leading slash in CLI (apache#2424)

houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 14, 2017

Allow FQN with 3 parts (namespace/package/action) to be input with/wi…

5e07a4e

…thout leading slash in CLI (apache#2424)

BillZong pushed a commit to BillZong/openwhisk that referenced this pull request Nov 18, 2019

Allow FQN with 3 parts (namespace/package/action) to be input with/wi…

bb6d7c5

…thout leading slash in CLI (apache#2424)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow optional leading slash in CLI commands #2424

Allow optional leading slash in CLI commands #2424

underwoodb-sd-ibm commented Jun 26, 2017 •

edited

Loading

underwoodb-sd-ibm commented Jun 29, 2017 •

edited

Loading

dubee left a comment

underwoodb-sd-ibm commented Jun 30, 2017 •

edited

Loading

underwoodb-sd-ibm commented Jul 6, 2017

mdeuser Jul 6, 2017

mdeuser Jul 6, 2017

mdeuser Jul 6, 2017

mdeuser Jul 6, 2017

mdeuser Jul 6, 2017

underwoodb-sd-ibm commented Jul 11, 2017

dubee Jul 14, 2017

dubee Jul 14, 2017

dubee Jul 14, 2017

dubee Jul 14, 2017

dubee Jul 14, 2017

dubee left a comment

dubee Jul 14, 2017

dubee Jul 14, 2017

dubee Jul 14, 2017

rabbah commented Aug 8, 2017

underwoodb-sd-ibm commented Aug 8, 2017

underwoodb-sd-ibm commented Aug 9, 2017

rabbah commented Aug 9, 2017

rabbah left a comment

underwoodb-sd-ibm commented Aug 10, 2017

dubee commented Aug 10, 2017

underwoodb-sd-ibm commented Aug 10, 2017

csantanapr commented Aug 10, 2017

rabbah left a comment

Allow optional leading slash in CLI commands #2424

Allow optional leading slash in CLI commands #2424

Conversation

underwoodb-sd-ibm commented Jun 26, 2017 • edited Loading

underwoodb-sd-ibm commented Jun 29, 2017 • edited Loading

dubee left a comment

Choose a reason for hiding this comment

underwoodb-sd-ibm commented Jun 30, 2017 • edited Loading

underwoodb-sd-ibm commented Jul 6, 2017

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

underwoodb-sd-ibm commented Jul 11, 2017

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

dubee 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

rabbah commented Aug 8, 2017

underwoodb-sd-ibm commented Aug 8, 2017

underwoodb-sd-ibm commented Aug 9, 2017

rabbah commented Aug 9, 2017

rabbah left a comment

Choose a reason for hiding this comment

underwoodb-sd-ibm commented Aug 10, 2017

dubee commented Aug 10, 2017

underwoodb-sd-ibm commented Aug 10, 2017

csantanapr commented Aug 10, 2017

rabbah left a comment

Choose a reason for hiding this comment

underwoodb-sd-ibm commented Jun 26, 2017 •

edited

Loading

underwoodb-sd-ibm commented Jun 29, 2017 •

edited

Loading

underwoodb-sd-ibm commented Jun 30, 2017 •

edited

Loading