Skip to content
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

Support passing del annotation #488

Merged
merged 3 commits into from
Aug 25, 2020

Conversation

ningyougang
Copy link
Contributor

@ningyougang ningyougang commented Aug 12, 2020

Currently, if passing another annotations, original previous annotation
will be removed and the passed new annotations will be added.

It may give users some confused that why my previous annotation gone.
So it is better to not delete user's previous annotation when adding new
annotation, but at the same time, need to provide a feature that
support to delete annotation by user via ClI, e.g.

wsk action update hello --del-annotation key1 --del-annotation key2

another brother prs to support del annotation:

@ningyougang
Copy link
Contributor Author

build error
image
It seems need to merge this firstly: apache/openwhisk-client-go#137 ?

url bool
save bool
saveAs string
delAnnotation []string
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add delAnnotation field

@rabbah
Copy link
Member

rabbah commented Aug 13, 2020 via email

@@ -1305,6 +1308,7 @@ func init() {
actionUpdateCmd.Flags().StringVarP(&Flags.common.paramFile, "param-file", "P", "", wski18n.T("`FILE` containing parameter values in JSON format"))
actionUpdateCmd.Flags().StringVar(&Flags.action.web, WEB_FLAG, "", wski18n.T("treat ACTION as a web action, a raw HTTP web action, or as a standard action; yes | true = web action, raw = raw HTTP web action, no | false = standard action"))
actionUpdateCmd.Flags().StringVar(&Flags.action.websecure, WEB_SECURE_FLAG, "", wski18n.T("secure the web action. where `SECRET` is true, false, or any string. Only valid when the ACTION is a web action"))
actionUpdateCmd.Flags().StringArrayVar(&Flags.action.delAnnotation, "del-annotation", []string{}, wski18n.T("del annotation"))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
actionUpdateCmd.Flags().StringArrayVar(&Flags.action.delAnnotation, "del-annotation", []string{}, wski18n.T("del annotation"))
actionUpdateCmd.Flags().StringArrayVar(&Flags.action.delAnnotation, "del-annotation", []string{}, wski18n.T("the list of annotations to be deleted from the action"))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@style95 updated accordingly.

@@ -480,6 +480,9 @@ func parseAction(cmd *cobra.Command, args []string, update bool) (*whisk.Action,
return nil, noArtifactError()
}

if update {
action.DelAnnotations = Flags.action.delAnnotation
Copy link
Member

@style95 style95 Aug 18, 2020

Choose a reason for hiding this comment

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

I am not clear about this, can users pass list of annotations? or they have to use --del-annotation for every annotation?
( --del-annotation key1 --del-annotation key2 vs --del-annotation [key1, key2])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Must pass like this --del-annotation key1 --del-annotation key2, in go backend, it will recevie the value by array:
https://github.com/apache/openwhisk-client-go/pull/137/files#diff-7a2fb893e2fa76731b906c5689a14e95R39

@ningyougang ningyougang force-pushed the support-passing-del-annotation branch from aab03ad to 0c0070d Compare August 18, 2020 09:18
Copy link
Member

@style95 style95 left a comment

Choose a reason for hiding this comment

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

LGTM

@ningyougang ningyougang reopened this Aug 24, 2020
@ningyougang
Copy link
Contributor Author

The reason of the build failed is that
image

When get the openwhisk-client-go codes, it didn't fetch the latest codes.

@ningyougang ningyougang force-pushed the support-passing-del-annotation branch from 72a76a3 to 30e698a Compare August 24, 2020 07:53
@ningyougang ningyougang force-pushed the support-passing-del-annotation branch from 30e698a to 7a47971 Compare August 24, 2020 10:08
@style95 style95 merged commit f201de1 into apache:master Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants