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 Force & Confirm #1783

Merged
merged 1 commit into from
Jan 20, 2017
Merged

Support Force & Confirm #1783

merged 1 commit into from
Jan 20, 2017

Conversation

derekbekoe
Copy link
Member

@derekbekoe derekbekoe commented Jan 18, 2017

Closes #1355

  • When registering a command, add confirmation=True to enable user confirmation. Also supports a string message or callable.
  • Add a —force flag for commands that support this feature.
  • Integrated with configuration system so it can be enabled/disabled by setting AZURE_CORE_DISABLE_CONFIRM_PROMPT

Added for the following commands:
az group delete
az vm delete
az network dns zone delete


EXAMPLES:

$ az group delete -n debekoe-a3
Are you sure you want to perform this operation? (y/n): n
Operation cancelled.

Only accepts appropriate values.

$ az group delete -n debekoe-a3
Are you sure you want to perform this operation? (y/n): 1
Are you sure you want to perform this operation? (y/n): 2
Are you sure you want to perform this operation? (y/n): 3
Are you sure you want to perform this operation? (y/n): n
Operation cancelled.
$ az group delete -n debekoe-a3 --force
$

No TTY:

$ az group delete -n debekoe-a3
Unable to prompt for confirmation as no tty available. Use --force.
Operation cancelled.

@derekbekoe
Copy link
Member Author

Only added it for these commands:
az group delete
az vm delete
az network dns zone delete

Are there any other critical ones we should add it for @tjprescott & @johanste?

@tjprescott
Copy link
Member

I believe the port does this for all delete operations, so I would lean toward that.

Copy link
Member

@tjprescott tjprescott left a comment

Choose a reason for hiding this comment

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

A question but overall LGTM.

```
You will generally only specify `name`, `operation` and possibly `table_transformer`.
- `module_name` - The name of the module that is registering the command (e.g. `azure.cli.command_modules.vm.commands`). Typically this will be `__name__`.
- `name` - String uniquely naming your command and placing it within the command hierachy. It will be the string that you would type at the command line, omitting `az` (ex: access your command at `az mypackage mycommand` using a name of `mypackage mycommand`).
- `operation` - The handler that will be executed. Format is `<module_to_import>#<attribute_list>`
- For example if `operation='azure.mgmt.compute.operations.virtual_machines_operations#VirtualMachinesOperations.get'`, the CLI will import `azure.mgmt.compute.operations.virtual_machines_operations`, get the `VirtualMachinesOperations` attribute and then the `get` attribute of `VirtualMachinesOperations`.
- `table_transformer` (optional) - Supply a callable that takes, transforms and returns a result for table output.
- `confirmation` (optional) - Supply True to enable default confirmation. Alternatively, supply a callable that takes the command arguments as a dict and returning a boolean. Alternatively, supply a string for the prompt.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not entirely sure I understand this. There are three cases:

  1. True => default prompt enabled
  2. callable => you have access to the command namespace (?) and return true or false? Do we use this?
  3. string => enables the prompt with a custom message.

Does using #2 mean you essentially are using your own handler and throwing away the default one?

Copy link
Member Author

Choose a reason for hiding this comment

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

Your understanding of 1 & 3 is correct.
For 2, you can specify your own confirmation prompt handler. You get the command namespace so you can use it if required. e.g. Print a custom prompt based on the name of one of the parameters.

And no we do not currently use 2 or 3 but there are use-cases where they'll be useful.

Copy link
Member

@tjprescott tjprescott left a comment

Choose a reason for hiding this comment

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

I'd recommend a second set of eyes, but LGTM.

- When registering a command, add `confirmation=True` to enable user confirmation. Also supports a string message or callable.
- Add a —force flag for commands that support this feature.
- Integrated with configuration system so it can be enabled/disabled by setting AZURE_CORE_DISABLE_CONFIRM_PROMPT

Added for the following commands:
az group delete
az vm delete
az network dns zone delete
@derekbekoe derekbekoe merged commit f274679 into Azure:master Jan 20, 2017
@derekbekoe derekbekoe deleted the force-confirm branch January 20, 2017 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants