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

[Config] Rename local-context to config param-persist #15068

Merged
merged 9 commits into from
Sep 14, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions linter_exclusions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1188,6 +1188,16 @@ config unset:
key:
rule_exclusions:
- no_positional_parameters
config parampersist show:
Copy link
Contributor

Choose a reason for hiding this comment

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

why don't we use param-persist? all names of your functions is param_persist

Copy link
Contributor Author

@arrownj arrownj Sep 11, 2020

Choose a reason for hiding this comment

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

The name is provided by PM after a long time survey.

Copy link
Member

Choose a reason for hiding this comment

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

It's a bit hard to read.

Copy link
Contributor

Choose a reason for hiding this comment

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

hard to understand the command name. prefer param-persist

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I will confirm with PM whether we can update it to param-persist.

parameters:
name:
rule_exclusions:
- no_positional_parameters
config parampersist delete:
parameters:
name:
rule_exclusions:
- no_positional_parameters
consumption budget create:
parameters:
resource_groups:
Expand Down
10 changes: 5 additions & 5 deletions src/azure-cli-core/azure/cli/core/local_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,17 +133,17 @@ def delete_file(self, recursive=False):
parent_dir = os.path.dirname(local_context_file.config_path)
if not os.listdir(parent_dir):
shutil.rmtree(parent_dir)
logger.warning('Local context persistence file in working directory %s is deleted.',
Copy link
Member

Choose a reason for hiding this comment

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

How about we change the file name local_context.py to avoid inconsistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just as I described in the PR description, this PR is only attempted to update the UE part, actually the internal class / persist file / test framework are still called Local Context (az local-context is marked deprecated but not totally removed yet). Let's leave the internal renaming work to totally remove az local-context command.

logger.warning('Param persist file in working directory %s is deleted.',
os.path.dirname(local_context_file.config_dir))
except Exception: # pylint: disable=broad-except
logger.warning('Fail to delete local context persistence file in working directory %s',
logger.warning('Fail to delete param persist file in working directory %s',
os.path.dirname(local_context_file.config_dir))

def clear(self, recursive=False):
local_context_files = self._load_local_context_files(recursive=recursive)
for local_context_file in local_context_files:
local_context_file.clear()
logger.warning('Local context information in working directory %s is cleared.',
logger.warning('Param persist information in working directory %s is cleared.',
os.path.dirname(local_context_file.config_dir))

def delete(self, names=None):
Expand All @@ -152,8 +152,8 @@ def delete(self, names=None):
for scope in local_context_file.sections():
for name in names:
local_context_file.remove_option(scope, name)
logger.warning('Local context value is deleted. You can run `az local-context show` to show all available '
'values.')
logger.warning('Param persist value is deleted. You can run `az config parampersist show` to show all '
'available values.')

def get_value(self, names=None):
result = {}
Expand Down
39 changes: 39 additions & 0 deletions src/azure-cli/azure/cli/command_modules/config/_help.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,42 @@
- name: Unset the configuration of key `core.no_color`.
text: az config unset core.no_color
"""

helps['config parampersist'] = """
type: group
short-summary: Manage parameter persist
arrownj marked this conversation as resolved.
Show resolved Hide resolved
arrownj marked this conversation as resolved.
Show resolved Hide resolved
"""

helps['config parampersist on'] = """
type: command
short-summary: Turn on parameter persist
"""

helps['config parampersist off'] = """
type: command
short-summary: Turn off parameter persist
"""

helps['config parampersist show'] = """
type: command
short-summary: Show parameter persist data
examples:
- name: Show all parameter persist value
text: az config parampersist show
- name: Show resource_group_name parameter persist value
text: az config parampersist show resource_group_name
"""

helps['config parampersist delete'] = """
type: command
short-summary: Delete parameter persist data
examples:
- name: Delete resource_group_name from parameter persist
text: az config parampersist delete resource_group_name
- name: Clear all parameter persist data
text: az config parampersist delete --all
- name: Delete parameter persist file
text: az config parampersist delete --all --purge
- name: Delete parameter persist file recursively
text: az config parampersist delete --all --purge --recursive
"""
10 changes: 10 additions & 0 deletions src/azure-cli/azure/cli/command_modules/config/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,13 @@ def load_arguments(self, _):
c.argument('local', action='store_true',
help='Include local configuration. Scan from the working directory up to the root drive, then the global configuration '
'and unset the first occurrence.')

with self.argument_context('config parampersist show') as c:
c.positional('name', nargs='*', help='Space-separated list of parameter persist names.')

with self.argument_context('config parampersist delete') as c:
c.positional('name', nargs='*', help='Space-separated list of parameter persist names. Either positional name argument or --all can be specified.')
c.argument('all', help='Clear all parameter persist data. Either positional name argument or --all can be specified.', action='store_true')
c.argument('yes', options_list=['--yes', '-y'], help='Do not prompt for confirmation. Only available when --all is specified.', action='store_true')
c.argument('purge', help='Delete parameter persist file from working directory. Only available when --all is specified.', action='store_true')
c.argument('recursive', help='Indicates this is recursive delete of parameter persist . Only available when --all is specified.', action='store_true')
21 changes: 21 additions & 0 deletions src/azure-cli/azure/cli/command_modules/config/_validators.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# --------------------------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------

from knack.util import CLIError


def validate_param_persist(cmd, namespace): # pylint: disable=unused-argument
if not cmd.cli_ctx.local_context.username:
raise CLIError('Can\'t get system user account. Parameter persist is ignored.')
if not cmd.cli_ctx.local_context.current_dir:
raise CLIError('The working directory has been deleted or recreated. You can change to another working '
'directory or reenter current one if it is recreated.')


def validate_param_persist_for_delete(cmd, namespace):
if (namespace.all and namespace.name) or (not namespace.all and not namespace.name):
raise CLIError('Please specify either positional argument name or --all.')

validate_param_persist(cmd, namespace)
8 changes: 7 additions & 1 deletion src/azure-cli/azure/cli/command_modules/config/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,19 @@
# --------------------------------------------------------------------------------------------

from azure.cli.core.commands import CliCommandType
from azure.cli.command_modules.config._validators import validate_param_persist, validate_param_persist_for_delete


def load_command_table(self, _):

config_custom = CliCommandType(operations_tmpl='azure.cli.command_modules.config.custom#{}')

with self.command_group('config', config_custom, is_experimental=True) as g:
g.command('set', 'config_set')
g.command('get', 'config_get')
g.command('unset', 'config_unset')

with self.command_group('config parampersist', config_custom, is_experimental=True) as g:
g.command('on', 'turn_param_persist_on')
g.command('off', 'turn_param_persist_off')
g.show_command('show', 'show_param_persist', validator=validate_param_persist)
g.command('delete', 'delete_param_persist', validator=validate_param_persist_for_delete)
38 changes: 38 additions & 0 deletions src/azure-cli/azure/cli/command_modules/config/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,41 @@ def config_unset(cmd, key=None, local=False):

with ScopedConfig(cmd.cli_ctx.config, local):
cmd.cli_ctx.config.remove_option(section, name)


def turn_param_persist_on(cmd):
if not cmd.cli_ctx.local_context.is_on:
cmd.cli_ctx.local_context.turn_on()
logger.warning('Parameter persist is turned on, you can run `az config parampersist off` to turn it off.')
else:
raise CLIError('Parameter persist is on already.')


def turn_param_persist_off(cmd):
if cmd.cli_ctx.local_context.is_on:
cmd.cli_ctx.local_context.turn_off()
logger.warning('Parameter persist is turned off, you can run `az config parampersist on` to turn it on.')
else:
raise CLIError('Parameter persist is off already.')


def show_param_persist(cmd, name=None):
if not name:
name = None
return cmd.cli_ctx.local_context.get_value(name)


def delete_param_persist(cmd, name=None, all=False, yes=False, purge=False, recursive=False): # pylint: disable=redefined-builtin
if name:
cmd.cli_ctx.local_context.delete(name)

if all:
from azure.cli.core.util import user_confirmation
if purge:
user_confirmation('You are going to delete parameter persist file. '
'Are you sure you want to continue this operation ?', yes)
cmd.cli_ctx.local_context.delete_file(recursive)
else:
user_confirmation('You are going to clear all parameter persist values. '
'Are you sure you want to continue this operation ?', yes)
cmd.cli_ctx.local_context.clear(recursive)
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# --------------------------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------
import unittest

from azure.cli.testsdk import LocalContextScenarioTest


class ParamPersistScenarioTest(LocalContextScenarioTest):

def test_param_persist_commands(self):
self.cmd('config parampersist show')
self.cmd('config parampersist show resource_group_name vnet_name')
self.cmd('config parampersist delete resource_group_name vnet_name')
self.cmd('config parampersist delete --all -y')
self.cmd('config parampersist delete --all --purge -y')
self.cmd('config parampersist delete --all --purge -y --recursive')

from knack.util import CLIError
with self.assertRaises(CLIError):
self.cmd('config parampersist delete resource_group_name --all')


if __name__ == '__main__':
unittest.main()
11 changes: 10 additions & 1 deletion src/azure-cli/azure/cli/command_modules/configure/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,16 @@ def load_command_table(self, _):
g.command('delete', 'delete_cache_contents')
g.command('purge', 'purge_cache_contents')

with self.command_group('local-context', configure_custom, is_experimental=True) as g:
def _local_context_deprecate_message(self):
msg = "This {} has been deprecated and will be removed in future release.".format(self.object_type)
msg += " Use '{}' instead.".format(self.redirect)
# msg += " For more information go to"
# msg += " <Add param persist doc link here when it is ready.>"
return msg

with self.command_group('local-context', configure_custom, is_experimental=True,
deprecate_info=self.deprecate(redirect="config parampersist",
message_func=_local_context_deprecate_message)) as g:
g.command('on', 'turn_local_context_on')
g.command('off', 'turn_local_context_off')
g.command('show', 'show_local_context', validator=validate_local_context) # pylint: disable=show-command
Expand Down