Skip to content

Remove 'kinds' from CLI; defer to backend.#2053

Merged
csantanapr merged 5 commits into
apache:masterfrom
rabbah:exec-manifest-cli
Mar 24, 2017
Merged

Remove 'kinds' from CLI; defer to backend.#2053
csantanapr merged 5 commits into
apache:masterfrom
rabbah:exec-manifest-cli

Conversation

@rabbah

@rabbah rabbah commented Mar 21, 2017

Copy link
Copy Markdown
Member

This PR is to permit progress on adding python:3 support.
Further CLI enhancements related to kind-awareness planned for/after #1980; in particular wsk property get to list supported kinds for deployment.

FYI @cclauss.

@rabbah rabbah requested a review from dubee March 21, 2017 01:07
@rabbah rabbah added cli review Review for this PR has been requested and yet needs to be done. labels Mar 21, 2017
@rabbah rabbah force-pushed the exec-manifest-cli branch from eab40f1 to 950e318 Compare March 21, 2017 01:47
@rabbah

rabbah commented Mar 21, 2017

Copy link
Copy Markdown
Member Author

pg2/1265 🔵

@rabbah rabbah added the pgapproved Pipeline has approved this change. label Mar 21, 2017
@csantanapr

csantanapr commented Mar 21, 2017

Copy link
Copy Markdown
Member

@dubeejw could put some 👀 on this when you have a chance

actionUpdateCmd.Flags().BoolVar(&flags.action.copy, "copy", false, wski18n.T("treat ACTION as the name of an existing action"))
actionUpdateCmd.Flags().BoolVar(&flags.action.sequence, "sequence", false, wski18n.T("treat ACTION as comma separated sequence of actions to invoke"))
actionUpdateCmd.Flags().StringVar(&flags.action.kind, "kind", "", wski18n.T("the `KIND` of the action runtime (example: swift:3, nodejs:6)"))
actionUpdateCmd.Flags().StringVar(&flags.action.kind, "kind", "", wski18n.T("the `KIND` of the action runtime (example: swift:default, nodejs:default)"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You will need to update the file containing externalized strings for this change. See tools/cli/go-whisk-cli/wski18n/resources/en_US.all.json.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done.

actionCreateCmd.Flags().BoolVar(&flags.action.copy, "copy", false, wski18n.T("treat ACTION as the name of an existing action"))
actionCreateCmd.Flags().BoolVar(&flags.action.sequence, "sequence", false, wski18n.T("treat ACTION as comma separated sequence of actions to invoke"))
actionCreateCmd.Flags().StringVar(&flags.action.kind, "kind", "", wski18n.T("the `KIND` of the action runtime (example: swift:3, nodejs:6)"))
actionCreateCmd.Flags().StringVar(&flags.action.kind, "kind", "", wski18n.T("the `KIND` of the action runtime (example: swift:default, nodejs:default)"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See comment about externalized strings.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done.

action.Exec.Kind = "nodejs"
} else if flags.action.kind == "python" {
action.Exec.Kind = "python"
if len(flags.action.kind) > 0 {

@dubee dubee Mar 21, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If a user enters a --kind value that is not supported, will the backend return an error when the request is sent for processing? If so, is there a test?

It would be fantastic if you can create a function that handles all the --kind processing and reference that in parseAction().

@rabbah rabbah Mar 21, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes it will:

> wsk action create foo t.js --kind foobar
error: Unable to create action: The request content was malformed:
kind 'foobar' not in Set(swift:3, nodejs, blackbox, java, sequence, nodejs:6, python, swift) (code 1206)
Run 'wsk --help' for usage.

I don't understand the second part of the comment re parseAction.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rabbah, opened a PR to move kind handling to its own function: rabbah#9.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Essentially I wanted this action.Exec, err = getExec(args[1]) to reduce the amount of code in parseAction.

@dubee dubee Mar 22, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just to be safe, you may also want to pass flags.action.kind, and flags.action.docker to the new method since those are globals and could be accidentally be overwritten. Ex: getExec(args[1], flags.action.kind, flags.action.docker)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

thanks for the explanation and pr.

@rabbah rabbah Mar 22, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

OK to stage the refactoring to a separate pr so that we can enable python 3?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It can be done later if necessary.

@rabbah rabbah force-pushed the exec-manifest-cli branch from 176299e to 02d18b4 Compare March 21, 2017 14:43
@dubee

dubee commented Mar 23, 2017

Copy link
Copy Markdown
Member

@csantanapr, LGTM!

@rabbah

rabbah commented Mar 23, 2017

Copy link
Copy Markdown
Member Author

I'll re pg.

@rabbah rabbah force-pushed the exec-manifest-cli branch from 31b6f40 to 69b6b5f Compare March 23, 2017 19:33
@rabbah rabbah force-pushed the exec-manifest-cli branch from 60a278d to d240ce3 Compare March 23, 2017 19:52
@rabbah

rabbah commented Mar 24, 2017

Copy link
Copy Markdown
Member Author

pg4/93 good to merge.

@csantanapr

Copy link
Copy Markdown
Member

@dubeejw can you approve the review record if it still Looks good to you?

@dubee

dubee commented Mar 24, 2017

Copy link
Copy Markdown
Member

I think it's good to merge. I don't have write access to officially approve :).

@csantanapr

Copy link
Copy Markdown
Member

@dubeejw You don't need write access to officially approve, you see a green button on this page

@csantanapr csantanapr merged commit 93f6740 into apache:master Mar 24, 2017
@rabbah rabbah deleted the exec-manifest-cli branch March 25, 2017 18:50
houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Jun 23, 2017
* Remove 'kinds' from CLI; defer to backend.

* Fix messages.

* Add cli test for unknown kind.

* Move Kind Process to its Own Method to Declutter ParseAction (apache#9)

* pass flags to getExec.
houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Jun 23, 2017
* Remove 'kinds' from CLI; defer to backend.

* Fix messages.

* Add cli test for unknown kind.

* Move Kind Process to its Own Method to Declutter ParseAction (apache#9)

* pass flags to getExec.
houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Jun 23, 2017
* Remove 'kinds' from CLI; defer to backend.

* Fix messages.

* Add cli test for unknown kind.

* Move Kind Process to its Own Method to Declutter ParseAction (apache#9)

* pass flags to getExec.
houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Jul 17, 2017
* Remove 'kinds' from CLI; defer to backend.

* Fix messages.

* Add cli test for unknown kind.

* Move Kind Process to its Own Method to Declutter ParseAction (apache#9)

* pass flags to getExec.
houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Jul 21, 2017
* Remove 'kinds' from CLI; defer to backend.

* Fix messages.

* Add cli test for unknown kind.

* Move Kind Process to its Own Method to Declutter ParseAction (apache#9)

* pass flags to getExec.
houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Jul 21, 2017
* Remove 'kinds' from CLI; defer to backend.

* Fix messages.

* Add cli test for unknown kind.

* Move Kind Process to its Own Method to Declutter ParseAction (apache#9)

* pass flags to getExec.
houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 13, 2017
* Remove 'kinds' from CLI; defer to backend.

* Fix messages.

* Add cli test for unknown kind.

* Move Kind Process to its Own Method to Declutter ParseAction (apache#9)

* pass flags to getExec.
houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 13, 2017
* Remove 'kinds' from CLI; defer to backend.

* Fix messages.

* Add cli test for unknown kind.

* Move Kind Process to its Own Method to Declutter ParseAction (apache#9)

* pass flags to getExec.
houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 13, 2017
* Remove 'kinds' from CLI; defer to backend.

* Fix messages.

* Add cli test for unknown kind.

* Move Kind Process to its Own Method to Declutter ParseAction (apache#9)

* pass flags to getExec.
houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 13, 2017
* Remove 'kinds' from CLI; defer to backend.

* Fix messages.

* Add cli test for unknown kind.

* Move Kind Process to its Own Method to Declutter ParseAction (apache#9)

* pass flags to getExec.
houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 14, 2017
* Remove 'kinds' from CLI; defer to backend.

* Fix messages.

* Add cli test for unknown kind.

* Move Kind Process to its Own Method to Declutter ParseAction (apache#9)

* pass flags to getExec.
houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 14, 2017
* Remove 'kinds' from CLI; defer to backend.

* Fix messages.

* Add cli test for unknown kind.

* Move Kind Process to its Own Method to Declutter ParseAction (apache#9)

* pass flags to getExec.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli pgapproved Pipeline has approved this change. review Review for this PR has been requested and yet needs to be done.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants