From 62c6f19d63635ed3400baebdb6405f4d2f943741 Mon Sep 17 00:00:00 2001 From: Oluwatosin Adewale Date: Mon, 22 Oct 2018 19:37:40 -0700 Subject: [PATCH 1/3] Fixed bug where if update --remove is called with --ids, the correct params aren't passed in subsequent iterations of the update command. --- .../azure/cli/core/commands/__init__.py | 17 ++++++- .../resource/tests/latest/test_resource.py | 46 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/azure-cli-core/azure/cli/core/commands/__init__.py b/src/azure-cli-core/azure/cli/core/commands/__init__.py index 605c0b13a3c..1377b0ec72b 100644 --- a/src/azure-cli-core/azure/cli/core/commands/__init__.py +++ b/src/azure-cli-core/azure/cli/core/commands/__init__.py @@ -343,7 +343,7 @@ def execute(self, args): logger.warning(d.message) try: - result = cmd(params) + result = cmd(self._copy_params(params)) if cmd.supports_no_wait and getattr(expanded_arg, 'no_wait', False): result = None elif cmd.no_wait_param and getattr(expanded_arg, cmd.no_wait_param, False): @@ -402,6 +402,21 @@ def remove_additional_prop_layer(obj, converted_dic): converted_dic.update(converted_dic.pop('additionalProperties')) return converted_dic + @staticmethod + # copy params so that original params remain unmodified when. + # params need to be unmodified in the event that a command is executed multiple times against different --ids + def _copy_params(params): + import copy + new_params = {} + for key, value in params.items(): + # TODO: is try except more robust here??? + if isinstance(value, AzCliCommand): + new_params[key] = value + else: + new_params[key] = copy.deepcopy(value) + return new_params + + def _rudimentary_get_command(self, args): # pylint: disable=no-self-use """ Rudimentary parsing to get the command """ nouns = [] diff --git a/src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/tests/latest/test_resource.py b/src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/tests/latest/test_resource.py index 815d7207a05..82953c882c6 100644 --- a/src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/tests/latest/test_resource.py +++ b/src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/tests/latest/test_resource.py @@ -154,6 +154,52 @@ def test_resource_id_scenario(self, resource_group): self.cmd('resource delete --id {subnet_id}', checks=self.is_empty()) self.cmd('resource delete --id {vnet_id}', checks=self.is_empty()) +class ResourceGenericUpdate(LiveScenarioTest): + @ResourceGroupPreparer(name_prefix='cli_test_resource_id') + def test_resource_id_scenario(self, resource_group): + self.kwargs.update({ + 'stor_1': self.create_random_name(prefix='stor1', length=10), + 'stor_2': self.create_random_name(prefix='stor2', length=10) + }) + + # create storage accounts + self.cmd('az storage account create -g {rg} -n {stor_1}') + self.cmd('az storage account create -g {rg} -n {stor_2}') + + # get ids + self.kwargs['stor_ids'] = " ".join(self.cmd('az storage account list -g {rg} --query "[].id"').get_output_in_json()) + + # update tags + self.cmd('az storage account update --ids {stor_ids} --set tags.isTag=True tags.isNotTag=False') + + self.cmd('az storage account show --name {stor_1} -g {rg}', checks=[ + self.check('tags.isTag', 'True'), + self.check('tags.isNotTag', 'False') + ]) + self.cmd('az storage account show --name {stor_2} -g {rg}', checks=[ + self.check('tags.isTag', 'True'), + self.check('tags.isNotTag', 'False') + ]) + + # delete tags.isTag + self.cmd('az storage account update --ids {stor_ids} --remove tags.isTag') + + self.cmd('az storage account show --name {stor_1} -g {rg} --query "tags"', checks=[ + self.check('isNotTag', 'False'), + self.check('isTag', None) + ]) + self.cmd('az storage account show --name {stor_2} -g {rg} --query "tags"', checks=[ + self.check('isNotTag', 'False'), + self.check('isTag', None) + ]) + + # delete tags.isNotTag + self.cmd('az storage account update --ids {stor_ids} --remove tags.isNotTag') + + # check tags is empty. + self.cmd('az storage account show --name {stor_1} -g {rg} --query "tags"', checks=self.is_empty()) + self.cmd('az storage account show --name {stor_2} -g {rg} --query "tags"', checks=self.is_empty()) + class ResourceCreateAndShowScenarioTest(ScenarioTest): From edba8f34e6c484e05446ff656e8e9961281cf556 Mon Sep 17 00:00:00 2001 From: Oluwatosin Adewale Date: Tue, 23 Oct 2018 13:45:17 -0700 Subject: [PATCH 2/3] Fixed Pep8 issues --- src/azure-cli-core/azure/cli/core/commands/__init__.py | 1 - .../cli/command_modules/resource/tests/latest/test_resource.py | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/azure-cli-core/azure/cli/core/commands/__init__.py b/src/azure-cli-core/azure/cli/core/commands/__init__.py index 1377b0ec72b..2966e1fe2d6 100644 --- a/src/azure-cli-core/azure/cli/core/commands/__init__.py +++ b/src/azure-cli-core/azure/cli/core/commands/__init__.py @@ -416,7 +416,6 @@ def _copy_params(params): new_params[key] = copy.deepcopy(value) return new_params - def _rudimentary_get_command(self, args): # pylint: disable=no-self-use """ Rudimentary parsing to get the command """ nouns = [] diff --git a/src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/tests/latest/test_resource.py b/src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/tests/latest/test_resource.py index 82953c882c6..5023e6bf9c6 100644 --- a/src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/tests/latest/test_resource.py +++ b/src/command_modules/azure-cli-resource/azure/cli/command_modules/resource/tests/latest/test_resource.py @@ -154,6 +154,7 @@ def test_resource_id_scenario(self, resource_group): self.cmd('resource delete --id {subnet_id}', checks=self.is_empty()) self.cmd('resource delete --id {vnet_id}', checks=self.is_empty()) + class ResourceGenericUpdate(LiveScenarioTest): @ResourceGroupPreparer(name_prefix='cli_test_resource_id') def test_resource_id_scenario(self, resource_group): From 349ccb7f6141be013e8068f53bf499d0967257a0 Mon Sep 17 00:00:00 2001 From: Oluwatosin Adewale Date: Wed, 24 Oct 2018 10:34:20 -0700 Subject: [PATCH 3/3] Fixed bug by changing generic update logic instead of copying CLI params. --- src/azure-cli-core/HISTORY.rst | 2 +- .../azure/cli/core/commands/__init__.py | 16 +--------------- .../azure/cli/core/commands/arm.py | 5 +++-- 3 files changed, 5 insertions(+), 18 deletions(-) diff --git a/src/azure-cli-core/HISTORY.rst b/src/azure-cli-core/HISTORY.rst index 71cd59c95ff..da1a7f3821d 100644 --- a/src/azure-cli-core/HISTORY.rst +++ b/src/azure-cli-core/HISTORY.rst @@ -5,7 +5,7 @@ Release History 2.0.50 ++++++ -* Minor fixes. +* Fix issue where update commands using `--remove` and `--ids` fail after first update is applied to first resource in ids list. 2.0.49 ++++++ diff --git a/src/azure-cli-core/azure/cli/core/commands/__init__.py b/src/azure-cli-core/azure/cli/core/commands/__init__.py index 2966e1fe2d6..605c0b13a3c 100644 --- a/src/azure-cli-core/azure/cli/core/commands/__init__.py +++ b/src/azure-cli-core/azure/cli/core/commands/__init__.py @@ -343,7 +343,7 @@ def execute(self, args): logger.warning(d.message) try: - result = cmd(self._copy_params(params)) + result = cmd(params) if cmd.supports_no_wait and getattr(expanded_arg, 'no_wait', False): result = None elif cmd.no_wait_param and getattr(expanded_arg, cmd.no_wait_param, False): @@ -402,20 +402,6 @@ def remove_additional_prop_layer(obj, converted_dic): converted_dic.update(converted_dic.pop('additionalProperties')) return converted_dic - @staticmethod - # copy params so that original params remain unmodified when. - # params need to be unmodified in the event that a command is executed multiple times against different --ids - def _copy_params(params): - import copy - new_params = {} - for key, value in params.items(): - # TODO: is try except more robust here??? - if isinstance(value, AzCliCommand): - new_params[key] = value - else: - new_params[key] = copy.deepcopy(value) - return new_params - def _rudimentary_get_command(self, args): # pylint: disable=no-self-use """ Rudimentary parsing to get the command """ nouns = [] diff --git a/src/azure-cli-core/azure/cli/core/commands/arm.py b/src/azure-cli-core/azure/cli/core/commands/arm.py index a2c6a86caa7..8ae445e8a89 100644 --- a/src/azure-cli-core/azure/cli/core/commands/arm.py +++ b/src/azure-cli-core/azure/cli/core/commands/arm.py @@ -794,6 +794,7 @@ def set_properties(instance, expression, force_string): def add_properties(instance, argument_values, force_string): # The first argument indicates the path to the collection to add to. + argument_values = list(argument_values) list_attribute_path = _get_internal_path(argument_values.pop(0)) list_to_add_to = _find_property(instance, list_attribute_path) @@ -833,8 +834,8 @@ def add_properties(instance, argument_values, force_string): def remove_properties(instance, argument_values): - # The first argument indicates the path to the collection to add to. - argument_values = argument_values if isinstance(argument_values, list) else [argument_values] + # The first argument indicates the path to the collection to remove from. + argument_values = list(argument_values) if isinstance(argument_values, list) else [argument_values] list_attribute_path = _get_internal_path(argument_values.pop(0)) list_index = None