[GitHub] mdeuser commented on a change in pull request #244: Add --web-secure option to action create/update
mdeuser commented on a change in pull request #244: Add --web-secure option to action create/update URL: https://github.com/apache/incubator-openwhisk-cli/pull/244#discussion_r176201197 ## File path: commands/action.go ## @@ -442,14 +457,109 @@ func parseAction(cmd *cobra.Command, args []string, update bool) (*whisk.Action, return nil, noArtifactError() } + whisk.Debug(whisk.DbgInfo, "Parsed action struct: %#v\n", action) + return action, err +} + +func augmentAction(cmd *cobra.Command, args []string, action *whisk.Action, update bool) (*whisk.Action, error) { + var err error + var existingAction *whisk.Action = nil + var augmentedAction *whisk.Action = new(whisk.Action) + *augmentedAction = *action + + if update { + if existingAction, _, err = Client.Actions.Get(action.Name, DO_NOT_FETCH_CODE); err != nil { + whiskErr, isWhiskError := err.(*whisk.WskError) + + if (isWhiskError && whiskErr.ExitCode != whisk.EXIT_CODE_NOT_FOUND) || !isWhiskError { + return nil, actionGetError(action.Name, DO_NOT_FETCH_CODE, err) + } + } + } + + // Augment the action's annotations with the --web related annotations + // FIXME MWD - avoid retrieving existing action TWICE! once above and once in the webAction() below + if augmentedAction, err = augmentWebArg(cmd, args, augmentedAction, existingAction); err != nil { + return nil, err + } + + // Augment the action's annotations with the --web-secure related annotations + if augmentedAction, err = augmentWebSecureArg(cmd, args, action, augmentedAction, existingAction); err != nil { + return nil, err + } + + whisk.Debug(whisk.DbgInfo, "Augmented action struct: %#v\n", augmentedAction) + return augmentedAction, err +} + +func augmentWebArg(cmd *cobra.Command, args []string, action *whisk.Action, existingAction *whisk.Action) (*whisk.Action, error) { + var err error + preserveAnnotations := action.Annotations == nil + var augmentedAction *whisk.Action = new(whisk.Action) + *augmentedAction = *action + if cmd.LocalFlags().Changed(WEB_FLAG) { - preserveAnnotations := action.Annotations == nil - action.Annotations, err = webAction(Flags.action.web, action.Annotations, qualifiedName.GetEntityName(), preserveAnnotations) + if existingAction == nil { + augmentedAction.Annotations, err = webAction(Flags.action.web, action.Annotations, action.Name, preserveAnnotations) + } else { + if augmentedAction.Annotations, err = webAction(Flags.action.web, action.Annotations, action.Name, preserveAnnotations); err == nil { + // Always carry forward any existing --web-secure annotation value + // Although it can be overwritten if --web-secure is set + webSecureAnnotations := getWebSecureAnnotations(existingAction) + if len(webSecureAnnotations) > 0 { + augmentedAction.Annotations = augmentedAction.Annotations.AppendKeyValueArr(webSecureAnnotations) + } + } + } + if err != nil { + return nil, err + } } - whisk.Debug(whisk.DbgInfo, "Parsed action struct: %#v\n", action) + whisk.Debug(whisk.DbgInfo, "augmentWebArg: Augmented action struct: %#v\n", augmentedAction) + return augmentedAction, nil +} - return action, err +/* + * Return a whisk.Action augmented with --web-secure annotation updates + * originalAction: a action constructed from command line argument + * action: an action constructed from command line args + possible other augmentation (i.e. --web annotations) + * existingAction: on an action update, this is the existing action + */ +func augmentWebSecureArg(cmd *cobra.Command, args []string, originalAction *whisk.Action, action *whisk.Action, existingAction *whisk.Action) (*whisk.Action, error) { Review comment: not sure.. i think each flag might be so unique that the operations don't boil down to the same augmentation "type". that said, if at some point we go with the augmentation table approach, if a new sugar flag arrives, it might be able to re-use one of the existing augmentation functions.. the function signature would need enough generalization to allow this though.. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries abou
[GitHub] mdeuser commented on a change in pull request #244: Add --web-secure option to action create/update
mdeuser commented on a change in pull request #244: Add --web-secure option to action create/update URL: https://github.com/apache/incubator-openwhisk-cli/pull/244#discussion_r175903104 ## File path: commands/action.go ## @@ -442,14 +457,109 @@ func parseAction(cmd *cobra.Command, args []string, update bool) (*whisk.Action, return nil, noArtifactError() } + whisk.Debug(whisk.DbgInfo, "Parsed action struct: %#v\n", action) + return action, err +} + +func augmentAction(cmd *cobra.Command, args []string, action *whisk.Action, update bool) (*whisk.Action, error) { + var err error + var existingAction *whisk.Action = nil + var augmentedAction *whisk.Action = new(whisk.Action) + *augmentedAction = *action + + if update { + if existingAction, _, err = Client.Actions.Get(action.Name, DO_NOT_FETCH_CODE); err != nil { + whiskErr, isWhiskError := err.(*whisk.WskError) + + if (isWhiskError && whiskErr.ExitCode != whisk.EXIT_CODE_NOT_FOUND) || !isWhiskError { + return nil, actionGetError(action.Name, DO_NOT_FETCH_CODE, err) + } + } + } + + // Augment the action's annotations with the --web related annotations + // FIXME MWD - avoid retrieving existing action TWICE! once above and once in the webAction() below + if augmentedAction, err = augmentWebArg(cmd, args, augmentedAction, existingAction); err != nil { + return nil, err + } + + // Augment the action's annotations with the --web-secure related annotations + if augmentedAction, err = augmentWebSecureArg(cmd, args, action, augmentedAction, existingAction); err != nil { + return nil, err + } + + whisk.Debug(whisk.DbgInfo, "Augmented action struct: %#v\n", augmentedAction) + return augmentedAction, err +} + +func augmentWebArg(cmd *cobra.Command, args []string, action *whisk.Action, existingAction *whisk.Action) (*whisk.Action, error) { + var err error + preserveAnnotations := action.Annotations == nil + var augmentedAction *whisk.Action = new(whisk.Action) + *augmentedAction = *action Review comment: @dubee good question... not really. in general, there are three separate "action" states that must be kept separate 1. the original cli input that generated an action. this action has the nicely parsed annotations/parameters 2. the in-progress action which may or may not have been manipulated by other command line option processing 3. the existing action (when present) the option augmentation logic is free to update the in-progress state, but not touch the original or existing action states. this way each of the special command option processing will have all of the state to make its action augmentation decisions. i initially retained updating the action, but this became problematic when the --web and --web-secure needed to perform similar annotation preservation logic requiring each to know the original command line state, not the in-progress state. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] mdeuser commented on a change in pull request #244: Add --web-secure option to action create/update
mdeuser commented on a change in pull request #244: Add --web-secure option to action create/update URL: https://github.com/apache/incubator-openwhisk-cli/pull/244#discussion_r175904374 ## File path: commands/action.go ## @@ -442,14 +457,109 @@ func parseAction(cmd *cobra.Command, args []string, update bool) (*whisk.Action, return nil, noArtifactError() } + whisk.Debug(whisk.DbgInfo, "Parsed action struct: %#v\n", action) + return action, err +} + +func augmentAction(cmd *cobra.Command, args []string, action *whisk.Action, update bool) (*whisk.Action, error) { + var err error + var existingAction *whisk.Action = nil + var augmentedAction *whisk.Action = new(whisk.Action) + *augmentedAction = *action + + if update { + if existingAction, _, err = Client.Actions.Get(action.Name, DO_NOT_FETCH_CODE); err != nil { + whiskErr, isWhiskError := err.(*whisk.WskError) + + if (isWhiskError && whiskErr.ExitCode != whisk.EXIT_CODE_NOT_FOUND) || !isWhiskError { + return nil, actionGetError(action.Name, DO_NOT_FETCH_CODE, err) + } + } + } + + // Augment the action's annotations with the --web related annotations + // FIXME MWD - avoid retrieving existing action TWICE! once above and once in the webAction() below + if augmentedAction, err = augmentWebArg(cmd, args, augmentedAction, existingAction); err != nil { + return nil, err + } + + // Augment the action's annotations with the --web-secure related annotations + if augmentedAction, err = augmentWebSecureArg(cmd, args, action, augmentedAction, existingAction); err != nil { + return nil, err + } + + whisk.Debug(whisk.DbgInfo, "Augmented action struct: %#v\n", augmentedAction) + return augmentedAction, err +} + +func augmentWebArg(cmd *cobra.Command, args []string, action *whisk.Action, existingAction *whisk.Action) (*whisk.Action, error) { + var err error + preserveAnnotations := action.Annotations == nil + var augmentedAction *whisk.Action = new(whisk.Action) + *augmentedAction = *action + if cmd.LocalFlags().Changed(WEB_FLAG) { - preserveAnnotations := action.Annotations == nil - action.Annotations, err = webAction(Flags.action.web, action.Annotations, qualifiedName.GetEntityName(), preserveAnnotations) + if existingAction == nil { + augmentedAction.Annotations, err = webAction(Flags.action.web, action.Annotations, action.Name, preserveAnnotations) + } else { + if augmentedAction.Annotations, err = webAction(Flags.action.web, action.Annotations, action.Name, preserveAnnotations); err == nil { + // Always carry forward any existing --web-secure annotation value + // Although it can be overwritten if --web-secure is set + webSecureAnnotations := getWebSecureAnnotations(existingAction) + if len(webSecureAnnotations) > 0 { + augmentedAction.Annotations = augmentedAction.Annotations.AppendKeyValueArr(webSecureAnnotations) + } + } + } + if err != nil { + return nil, err + } } - whisk.Debug(whisk.DbgInfo, "Parsed action struct: %#v\n", action) + whisk.Debug(whisk.DbgInfo, "augmentWebArg: Augmented action struct: %#v\n", augmentedAction) + return augmentedAction, nil +} - return action, err +/* + * Return a whisk.Action augmented with --web-secure annotation updates + * originalAction: a action constructed from command line argument + * action: an action constructed from command line args + possible other augmentation (i.e. --web annotations) + * existingAction: on an action update, this is the existing action + */ +func augmentWebSecureArg(cmd *cobra.Command, args []string, originalAction *whisk.Action, action *whisk.Action, existingAction *whisk.Action) (*whisk.Action, error) { Review comment: right. since that logic for any new sugar flag is unique, each new flag would need to implement it's own "augmentation" function. perhaps in another pr, there might be a table of these augmentation functions which are called in order during action create/update. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Service
[GitHub] mdeuser commented on a change in pull request #244: Add --web-secure option to action create/update
mdeuser commented on a change in pull request #244: Add --web-secure option to action create/update URL: https://github.com/apache/incubator-openwhisk-cli/pull/244#discussion_r175903104 ## File path: commands/action.go ## @@ -442,14 +457,109 @@ func parseAction(cmd *cobra.Command, args []string, update bool) (*whisk.Action, return nil, noArtifactError() } + whisk.Debug(whisk.DbgInfo, "Parsed action struct: %#v\n", action) + return action, err +} + +func augmentAction(cmd *cobra.Command, args []string, action *whisk.Action, update bool) (*whisk.Action, error) { + var err error + var existingAction *whisk.Action = nil + var augmentedAction *whisk.Action = new(whisk.Action) + *augmentedAction = *action + + if update { + if existingAction, _, err = Client.Actions.Get(action.Name, DO_NOT_FETCH_CODE); err != nil { + whiskErr, isWhiskError := err.(*whisk.WskError) + + if (isWhiskError && whiskErr.ExitCode != whisk.EXIT_CODE_NOT_FOUND) || !isWhiskError { + return nil, actionGetError(action.Name, DO_NOT_FETCH_CODE, err) + } + } + } + + // Augment the action's annotations with the --web related annotations + // FIXME MWD - avoid retrieving existing action TWICE! once above and once in the webAction() below + if augmentedAction, err = augmentWebArg(cmd, args, augmentedAction, existingAction); err != nil { + return nil, err + } + + // Augment the action's annotations with the --web-secure related annotations + if augmentedAction, err = augmentWebSecureArg(cmd, args, action, augmentedAction, existingAction); err != nil { + return nil, err + } + + whisk.Debug(whisk.DbgInfo, "Augmented action struct: %#v\n", augmentedAction) + return augmentedAction, err +} + +func augmentWebArg(cmd *cobra.Command, args []string, action *whisk.Action, existingAction *whisk.Action) (*whisk.Action, error) { + var err error + preserveAnnotations := action.Annotations == nil + var augmentedAction *whisk.Action = new(whisk.Action) + *augmentedAction = *action Review comment: @dubee good question... not really. in general, there are three separate "action" states that must be kept separate 1. the original cli input that generated an action. this action has the nicely parsed annotations/parameters 2. the in-progress action which may or may not have been manipulated by other command line option processing 3. the existing action (when present) the option augmentation logic is free to update the in-progress state, but not touch the original or existing action states. this way each of the special command option processing will have all of the state to make its action augmentation decisions. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services