[Config] Rename local-context to config param-persist #15068
Conversation
arrownj
requested review from
fengzhou-msft,
jiasli,
jsntcy,
Juliehzl and
mmyyrroonn
as code owners
|
Config |
| @@ -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.', | |||
There was a problem hiding this comment.
How about we change the file name local_context.py to avoid inconsistency?
There was a problem hiding this comment.
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.
linter_exclusions.yml
Outdated
| @@ -1188,6 +1188,16 @@ config unset: | |||
| key: | |||
| rule_exclusions: | |||
| - no_positional_parameters | |||
| config parampersist show: | |||
There was a problem hiding this comment.
why don't we use param-persist? all names of your functions is param_persist
There was a problem hiding this comment.
The name is provided by PM after a long time survey.
There was a problem hiding this comment.
hard to understand the command name. prefer param-persist
There was a problem hiding this comment.
Yeah, I will confirm with PM whether we can update it to param-persist.
| 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.') |
There was a problem hiding this comment.
should we use + to prevent no value added for the parameter?
There was a problem hiding this comment.
Support no value here.
| 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') |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
arrownj
changed the title
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Description
We have a long story in email to ask rename original
az local-contexttoaz 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 onaz config parampersist offaz config parampersist showaz config parampersist delete resource_group_nameaz local-context offHistory 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.