-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[Config] az config: Add new config command module
#14436
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
Conversation
|
Config refactor |
| @@ -1,5 +1,6 @@ | |||
| { | |||
| "az": "src/azure-cli/azure/cli/command_modules/profile/_help.py", | |||
| "confige": "src/azure-cli/azure/cli/command_modules/config/_help.py", | |||
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.
extra e?
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.
Oh! Nice catch. Perhaps our CI should check it? @haroldrandom @myronfanqiu
| try: | ||
| return next(x for x in items if x['name'] == name) | ||
| except StopIteration: | ||
| raise CLIError("Configuration '{}' is not set.".format(key)) |
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.
Another option for this case is to return empty string. Personally, I prefer empty string for this case.
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.
Empty string means the config is actually set to '', which is different from the concept of "not set".
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.
Make sense to me. Maybe we should also confirm this with PM.
| name = parts[1] | ||
|
|
||
| with ScopedConfig(cmd.cli_ctx.config, local): | ||
| cmd.cli_ctx.config.remove_option(section, name) |
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.
What will be returned if key is not set?
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.
It will ignore the operation and return False. See remove_option:
remove_option(section, option)
Remove the specified option from the specified section. If the section does not exist, raise NoSectionError. If the option existed to be removed, return True; otherwise return 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.
However, configparser.NoSectionError is ignored by Knack.
try:
existed = self.config_parser.remove_option(section, option)
self.set(self.config_parser)
except configparser.NoSectionError:
passWe'd better confirm with PM about the behavior later.
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.
Agree with your suggestion.
mmyyrroonn
left a comment
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.
Approved for linter_exclusions.yml file
|
An excerpt from the design spec https://github.com/jiasli/azure-cli-spec/blob/master/az-config.md: Additional work - migrate to settings.json
Visual Studio CodeThe config file is at {
"terminal.integrated.shell.windows": "C:\\WINDOWS\\System32\\WindowsPowerShell\\v1.0\\powershell.exe",
"terminal.integrated.scrollback": 99999,
"editor.minimap.enabled": false,
...
}Windows TerminalThe config file is at {
"$schema": "https://aka.ms/terminal-profiles-schema",
"defaultProfile": "{574e775e-4f2a-5b96-ac1e-a2962a402336}",
"profiles":
[
{
// Make changes here to the powershell.exe profile
"guid": "{61c54bbd-c2c6-5271-96e7-009a87ff44bf}",
"name": "Windows PowerShell",
"commandline": "powershell.exe",
"hidden": false
},
...
],
...
}Using |

Description
Resolve #13981
Add new
configcommand module. This PR relies on Knack change microsoft/knack#217.Testing Guide
Set
Disable color with
core.no_color.Hide warnings and only show errors with
core.only_show_errors.Turn on client-side telemetry.
Turn on file logging and set its location.
Set the default resource group to
myRG.Set the default resource group to
myRGon a local scope.Get
Get all configurations.
Get configurations in
coresection.Get configuration of key
core.no_color.Unset
Unset configurations of key
core.no_color.