-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[Draft] Support --what-if argument v2 #32477
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
base: dev
Are you sure you want to change the base?
Changes from all commits
abb2f17
46cf88d
7bc0289
df108e2
f126b42
08cc6c1
6a1b839
6a52aac
b4aae27
b764741
c3caf10
2b61732
3f86e44
499e477
e11f6aa
8d394ba
6393d35
2d75315
31dab53
4207bca
8402c51
6c3d3bc
9c44270
7d89ee3
6bdefb9
6fd0a35
45f3dc9
0cf60fd
7cf3c0a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -506,6 +506,7 @@ class AzCliCommandInvoker(CommandInvoker): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # pylint: disable=too-many-statements,too-many-locals,too-many-branches | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def execute(self, args): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| args_copy = args[:] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from knack.events import (EVENT_INVOKER_PRE_CMD_TBL_CREATE, EVENT_INVOKER_POST_CMD_TBL_CREATE, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| EVENT_INVOKER_CMD_TBL_LOADED, EVENT_INVOKER_PRE_PARSE_ARGS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| EVENT_INVOKER_POST_PARSE_ARGS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -586,7 +587,12 @@ def execute(self, args): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| args[0] = '--help' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.parser.enable_autocomplete() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if '--what-if' in (args_copy): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return self._what_if(args_copy) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| elif '--export-bicep' in (args_copy): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| elif '--export-bicep' in (args_copy): | |
| elif '--export-bicep' in args_copy: |
Copilot
AI
Dec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message format is inconsistent. Using f-string formatting with .format() style placeholders mixed in other parts of the code. This error should follow the same pattern as others:
error_msg = "\"--what-if\" argument is not supported for this command."Or use consistent formatting throughout.
Copilot
AI
Dec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded list of supported commands should be maintained in a centralized configuration file or constant at the module level rather than buried in this method. This makes it easier to maintain and update the list of supported commands.
Consider moving this to a module-level constant:
# At module level
WHAT_IF_SUPPORTED_COMMANDS = {
'vm create',
'vm update',
# ... rest of commands
}| # Define supported commands for what-if functionality | |
| WHAT_IF_SUPPORTED_COMMANDS = { | |
| 'vm create', | |
| 'vm update', | |
| 'storage account create', | |
| 'storage container create', | |
| 'storage share create', | |
| 'network vnet create', | |
| 'network vnet update', | |
| 'storage account network-rule add', | |
| 'vm disk attach', | |
| 'vm disk detach', | |
| 'vm nic remove', | |
| 'sql server create', | |
| 'sql server update', | |
| } | |
| # Supported commands for what-if functionality (module-level constant) | |
| WHAT_IF_SUPPORTED_COMMANDS = { | |
| 'vm create', | |
| 'vm update', | |
| 'storage account create', | |
| 'storage container create', | |
| 'storage share create', | |
| 'network vnet create', | |
| 'network vnet update', | |
| 'storage account network-rule add', | |
| 'vm disk attach', | |
| 'vm disk detach', | |
| 'vm nic remove', | |
| 'sql server create', | |
| 'sql server update', | |
| } | |
| # Define supported commands for what-if functionality | |
| # Use module-level WHAT_IF_SUPPORTED_COMMANDS |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -268,6 +268,26 @@ def get_location_type(cli_ctx): | |||||||
| return location_type | ||||||||
|
|
||||||||
|
|
||||||||
| def get_what_if_type(): | ||||||||
| what_if_type = CLIArgumentType( | ||||||||
| options_list=['--what-if'], | ||||||||
| help="Preview the changes that will be made without actually executing the command. " | ||||||||
| "This will call the what-if service to compare the current state with the expected state after execution.", | ||||||||
|
||||||||
| "This will call the what-if service to compare the current state with the expected state after execution.", | |
| "This will call the what-if service to compare the current state with the expected state after execution. " | |
| "Type: boolean. Default: False.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conditional check is redundant. The check
if '--what-if' in (args_copy)doesn't need the extra parentheses aroundargs_copy, and this check is performed again inside the_what_ifmethod at line 702. Consider simplifying: