-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Changes from 4 commits
63d02a9
03856d7
4cdebc8
3eba829
86e9ba8
e0db83e
c3ecb91
4398f6b
4b4226f
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 |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about we change the file name There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
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): | ||
|
@@ -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 = {} | ||
|
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) |
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() |
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.
why don't we use param-persist? all names of your functions is
param_persist
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 name is provided by PM after a long time survey.
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's a bit hard to read.
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.
hard to understand the command name. prefer
param-persist
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.
Yeah, I will confirm with PM whether we can update it to
param-persist
.