[Config] Rename local-context to config param-persist by arrownj · Pull Request #15068 · Azure/azure-cli

arrownj · 2020-09-07T06:46:06Z

Description
We have a long story in email to ask rename original az local-context to az config parampersist.

This PR is to do this work, and we also add deprecate info for az local-context.

And please note that in this PR, we only change the UE part(command name), the internal class name/ function name were not changed.

Testing Guide

az config parampersist on

Command group 'config parampersist' is experimental and not covered by customer support. Please use with discretion.
Parameter persist is turned on, you can run az config parampersist off to turn it off.

az config parampersist off

Command group 'config parampersist' is experimental and not covered by customer support. Please use with discretion.
Parameter persist is turned off, you can run az config parampersist on to turn it on.

az config parampersist show

Command group 'config parampersist' is experimental and not covered by customer support. Please use with discretion.
{
"all": {
"identity_name": "xiaojxu-msi-3",
"resource_group_name": "img_tmpl_customizers000001"
}
}

az config parampersist delete resource_group_name

Command group 'config parampersist' is experimental and not covered by customer support. Please use with discretion.
Param persist value is deleted. You can run az config parampersist show to show all available values.

az local-context off

This command is implicitly deprecated because command group 'local-context' is deprecated and will be removed in a future release. Use 'config parampersist' instead.
Command group 'local-context' is experimental and not covered by customer support. Please use with discretion.
Local context is turned off, you can run az local-context on to turn it on.

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.

This checklist is used to make sure that common guidelines for a pull request are followed.

The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.

yonzhan · 2020-09-07T08:37:04Z

Config

jiasli · 2020-09-08T08:22:30Z

src/azure-cli-core/azure/cli/core/local_context.py

                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.',


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

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.

src/azure-cli/azure/cli/command_modules/config/_help.py

mmyyrroonn · 2020-09-11T05:35:07Z

linter_exclusions.yml

    key:
      rule_exclusions:
      - no_positional_parameters
+config parampersist show:


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

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

It's a bit hard to read.

hard to understand the command name. prefer param-persist

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

Juliehzl · 2020-09-11T08:02:55Z

src/azure-cli/azure/cli/command_modules/config/_params.py

+        c.positional('name', nargs='*', help='Space-separated list of parameter persistence names.')
+
+    with self.argument_context('config parampersist delete') as c:
+        c.positional('name', nargs='*', help='Space-separated list of parameter persistence names. Either positional name argument or --all can be specified.')


should we use + to prevent no value added for the parameter?

Support no value here.

Juliehzl · 2020-09-11T08:03:53Z

src/azure-cli/azure/cli/command_modules/config/_params.py

+        c.argument('all', help='Clear all parameter persistence 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 persistence file from working directory. Only available when --all is specified.', action='store_true')
+        c.argument('recursive', help='Indicates this is recursive delete of parameter persistence. Only available when --all is specified.', action='store_true')


arrownj · 2020-09-11T09:02:19Z

/azp run

azure-pipelines · 2020-09-11T09:02:32Z

Azure Pipelines successfully started running 2 pipeline(s).

arrownj · 2020-09-14T02:12:52Z

/azp run

azure-pipelines · 2020-09-14T02:13:53Z

Azure Pipelines successfully started running 2 pipeline(s).

[Config] Rename local-context to config parampersist

63d02a9

arrownj requested a review from haroldrandom as a code owner September 7, 2020 06:46

arrownj self-assigned this Sep 7, 2020

update warning message

03856d7

arrownj requested review from Juliehzl, fengzhou-msft, jiasli, jsntcy and mmyyrroonn as code owners September 7, 2020 06:46

fix sytle/linter error

4cdebc8

fix style/linter error

3eba829

jiasli reviewed Sep 8, 2020

View reviewed changes

jiasli reviewed Sep 10, 2020

View reviewed changes

src/azure-cli/azure/cli/command_modules/config/_help.py Outdated Show resolved Hide resolved

jiasli reviewed Sep 10, 2020

View reviewed changes

src/azure-cli/azure/cli/command_modules/config/_help.py Outdated Show resolved Hide resolved

jiasli approved these changes Sep 10, 2020

View reviewed changes

arrownj added 2 commits September 10, 2020 11:26

udpate some message

86e9ba8

update some message

e0db83e

mmyyrroonn approved these changes Sep 11, 2020

View reviewed changes

mmyyrroonn reviewed Sep 11, 2020

View reviewed changes

fengzhou-msft approved these changes Sep 11, 2020

View reviewed changes

Juliehzl reviewed Sep 11, 2020

View reviewed changes

update help message

c3ecb91

haroldrandom approved these changes Sep 11, 2020

View reviewed changes

arrownj added 2 commits September 14, 2020 09:42

update to param-persist

4398f6b

fix test

4b4226f

arrownj changed the title ~~[Config] Rename local-context to config parampersist~~ [Config] Rename local-context to config param-persist Sep 14, 2020

arrownj merged commit 2c52cbc into Azure:dev Sep 14, 2020

This was referenced Nov 24, 2022

{local-context} Remove local-context command group #22471

Merged

Drop az config param-persist and Local Context #24726

Closed

Conversation

arrownj commented Sep 7, 2020

Uh oh!

yonzhan commented Sep 7, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arrownj Sep 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Juliehzl Sep 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arrownj commented Sep 11, 2020

Uh oh!

azure-pipelines bot commented Sep 11, 2020

Uh oh!

arrownj commented Sep 14, 2020

Uh oh!

azure-pipelines bot commented Sep 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

arrownj Sep 11, 2020 •

edited

Loading

Juliehzl Sep 11, 2020 •

edited

Loading