From 28a3c398a375e14c483a36c693e575864bb7a0fe Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 15 Jul 2021 21:20:15 +0530 Subject: [PATCH 01/54] Adding code to remove action files on disk --- st2api/st2api/controllers/v1/actions.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index c78667076d..a03ba30311 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -18,6 +18,7 @@ import stat import errno +from oslo_config import cfg import six from mongoengine import ValidationError @@ -235,9 +236,20 @@ def delete(self, ref_or_id, requester_user): ref_or_id, action_db, ) + + BASE_PATH = cfg.CONF.system.base_path + pack_name = action_db["pack"] + entry_point = action_db["entry_point"] + metadate_file = action_db["metadata_file"] + ACTION_PYTHON_FILE_PATH = os.path.join(BASE_PATH, "packs", pack_name, "actions", entry_point) + ACTION_METADATA_FILE_PATH = os.path.join(BASE_PATH, "packs", pack_name, metadate_file) try: Action.delete(action_db) + if os.path.exists(ACTION_PYTHON_FILE_PATH): + os.remove(ACTION_PYTHON_FILE_PATH) + if os.path.exists(ACTION_METADATA_FILE_PATH): + os.remove(ACTION_METADATA_FILE_PATH) except Exception as e: LOG.error( 'Database delete encountered exception during delete of id="%s". ' From addd29172580f2da238ff8efa5f1609dd6b2d045 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 16 Jul 2021 10:22:25 +0530 Subject: [PATCH 02/54] Update st2api/st2api/controllers/v1/actions.py Co-authored-by: blag --- st2api/st2api/controllers/v1/actions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index a03ba30311..c0702e8406 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -241,8 +241,8 @@ def delete(self, ref_or_id, requester_user): pack_name = action_db["pack"] entry_point = action_db["entry_point"] metadate_file = action_db["metadata_file"] - ACTION_PYTHON_FILE_PATH = os.path.join(BASE_PATH, "packs", pack_name, "actions", entry_point) - ACTION_METADATA_FILE_PATH = os.path.join(BASE_PATH, "packs", pack_name, metadate_file) + action_entrypoint_file_path = os.path.join(BASE_PATH, "packs", pack_name, "actions", entry_point) + action_metadata_file_path = os.path.join(BASE_PATH, "packs", pack_name, metadate_file) try: Action.delete(action_db) From b70b4a35910626d01baf9117d85ac7f10e175be9 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 16 Jul 2021 10:22:34 +0530 Subject: [PATCH 03/54] Update st2api/st2api/controllers/v1/actions.py Co-authored-by: blag --- st2api/st2api/controllers/v1/actions.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index c0702e8406..346c9191c1 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -246,10 +246,10 @@ def delete(self, ref_or_id, requester_user): try: Action.delete(action_db) - if os.path.exists(ACTION_PYTHON_FILE_PATH): - os.remove(ACTION_PYTHON_FILE_PATH) - if os.path.exists(ACTION_METADATA_FILE_PATH): - os.remove(ACTION_METADATA_FILE_PATH) + if os.path.exists(action_entrypoint_file_path): + os.remove(action_entrypoint_file_path) + if os.path.exists(action_metadata_file_path): + os.remove(action_metadata_file_path) except Exception as e: LOG.error( 'Database delete encountered exception during delete of id="%s". ' From f6b91b9048efd2e4175166a0eeef7a7f5e532f19 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 16 Jul 2021 10:29:49 +0530 Subject: [PATCH 04/54] Updating actions.py --- st2api/st2api/controllers/v1/actions.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index 346c9191c1..37e916a53c 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -241,8 +241,12 @@ def delete(self, ref_or_id, requester_user): pack_name = action_db["pack"] entry_point = action_db["entry_point"] metadate_file = action_db["metadata_file"] - action_entrypoint_file_path = os.path.join(BASE_PATH, "packs", pack_name, "actions", entry_point) - action_metadata_file_path = os.path.join(BASE_PATH, "packs", pack_name, metadate_file) + action_entrypoint_file_path = os.path.join( + BASE_PATH, "packs", pack_name, "actions", entry_point + ) + action_metadata_file_path = os.path.join( + BASE_PATH, "packs", pack_name, metadate_file + ) try: Action.delete(action_db) From ca4d4537c7ff309b7cb740f72d7d488796edb7c7 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Mon, 19 Jul 2021 16:32:54 +0530 Subject: [PATCH 05/54] Updating st2api/st2api/controllers/v1/actions.py Added try except block for handling PermissionError while removing action files from disk. Also, code is added for logging errors. --- st2api/st2api/controllers/v1/actions.py | 31 +++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index 37e916a53c..d0c06a4862 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -250,10 +250,37 @@ def delete(self, ref_or_id, requester_user): try: Action.delete(action_db) + if os.path.exists(action_entrypoint_file_path): - os.remove(action_entrypoint_file_path) + try: + os.remove(action_entrypoint_file_path) + except PermissionError: + LOG.error( + 'No permission to delete the "%s" file', + action_entrypoint_file_path, + ) + except Exception as e: + LOG.error( + 'Unable to delete "%s" file. Exception was "%s"', + action_entrypoint_file_path, + e, + ) + if os.path.exists(action_metadata_file_path): - os.remove(action_metadata_file_path) + try: + os.remove(action_metadata_file_path) + except PermissionError: + LOG.error( + 'No permission to delete the "%s" file', + action_metadata_file_path, + ) + except Exception as e: + LOG.error( + 'Unable to delete "%s" file. Exception was "%s"', + action_metadata_file_path, + e, + ) + except Exception as e: LOG.error( 'Database delete encountered exception during delete of id="%s". ' From e7b32b00296dc323ad29b89430a22013fe57e9ae Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Mon, 19 Jul 2021 16:41:27 +0530 Subject: [PATCH 06/54] Updating st2client/commands/resource.py Added code to add a confirmation prompt in CLI to let user know that the operation will delete files on disk as well. If user says OK then to proceed otherwise to back out the operation. --- st2client/st2client/commands/resource.py | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/st2client/st2client/commands/resource.py b/st2client/st2client/commands/resource.py index d66e990a93..e3606cf53e 100644 --- a/st2client/st2client/commands/resource.py +++ b/st2client/st2client/commands/resource.py @@ -35,6 +35,7 @@ from st2client.exceptions.operations import OperationFailureException from st2client.formatters import table from st2client.utils.types import OrderedSet +import st2client ALLOWED_EXTS = [".json", ".yaml", ".yml"] PARSER_FUNCS = {".json": json.load, ".yml": yaml.safe_load, ".yaml": yaml.safe_load} @@ -735,16 +736,29 @@ def __init__(self, resource, *args, **kwargs): def run(self, args, **kwargs): resource_id = getattr(args, self.pk_argument_name, None) instance = self.get_resource(resource_id, **kwargs) - self.manager.delete(instance, **kwargs) + if isinstance(instance, st2client.models.action.Action): + user_input = input( + "It will delete action files on disk as well. Do you want to continue? (y/n): " + ) + if user_input.lower() == "y" or user_input.lower() == "yes": + self.manager.delete(instance, **kwargs) + print( + 'Resource with id "%s" has been successfully deleted from database and disk.' + % (resource_id) + ) + else: + print("Action is not deleted.") + else: + self.manager.delete(instance, **kwargs) + print( + 'Resource with id "%s" has been successfully deleted.' % (resource_id) + ) def run_and_print(self, args, **kwargs): resource_id = getattr(args, self.pk_argument_name, None) try: self.run(args, **kwargs) - print( - 'Resource with id "%s" has been successfully deleted.' % (resource_id) - ) except ResourceNotFoundError: self.print_not_found(resource_id) raise OperationFailureException("Resource %s not found." % resource_id) From 7ca0f6c78f86179306a69c7ff904fc0f6213e87d Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 20 Jul 2021 12:17:47 +0530 Subject: [PATCH 07/54] Removing whitespaces --- st2api/st2api/controllers/v1/actions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index d0c06a4862..f43921cadc 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -18,9 +18,9 @@ import stat import errno +from mongoengine import ValidationError from oslo_config import cfg import six -from mongoengine import ValidationError # TODO: Encapsulate mongoengine errors in our persistence layer. Exceptions # that bubble up to this layer should be core Python exceptions or @@ -236,7 +236,7 @@ def delete(self, ref_or_id, requester_user): ref_or_id, action_db, ) - + BASE_PATH = cfg.CONF.system.base_path pack_name = action_db["pack"] entry_point = action_db["entry_point"] From 0639640498d948314a524feeeed600f474187216 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 22 Jul 2021 20:09:36 +0530 Subject: [PATCH 08/54] Updating st2api/controllers/v1/actions.py Moving path calculations and action file remove related code to st2common/services/packs.py --- st2api/st2api/controllers/v1/actions.py | 47 ++++--------------------- 1 file changed, 7 insertions(+), 40 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index f43921cadc..0815374cc3 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -19,7 +19,6 @@ import errno from mongoengine import ValidationError -from oslo_config import cfg import six # TODO: Encapsulate mongoengine errors in our persistence layer. Exceptions @@ -43,6 +42,7 @@ from st2common.content.utils import get_pack_base_path from st2common.content.utils import get_pack_resource_file_abs_path from st2common.content.utils import get_relative_path_to_pack_file +from st2common.services.packs import delete_action_files_from_pack from st2common.transport.reactor import TriggerDispatcher from st2common.util.system_info import get_host_info import st2common.validators.api.action as action_validator @@ -237,50 +237,17 @@ def delete(self, ref_or_id, requester_user): action_db, ) - BASE_PATH = cfg.CONF.system.base_path pack_name = action_db["pack"] entry_point = action_db["entry_point"] - metadate_file = action_db["metadata_file"] - action_entrypoint_file_path = os.path.join( - BASE_PATH, "packs", pack_name, "actions", entry_point - ) - action_metadata_file_path = os.path.join( - BASE_PATH, "packs", pack_name, metadate_file - ) + metadata_file = action_db["metadata_file"] try: Action.delete(action_db) - - if os.path.exists(action_entrypoint_file_path): - try: - os.remove(action_entrypoint_file_path) - except PermissionError: - LOG.error( - 'No permission to delete the "%s" file', - action_entrypoint_file_path, - ) - except Exception as e: - LOG.error( - 'Unable to delete "%s" file. Exception was "%s"', - action_entrypoint_file_path, - e, - ) - - if os.path.exists(action_metadata_file_path): - try: - os.remove(action_metadata_file_path) - except PermissionError: - LOG.error( - 'No permission to delete the "%s" file', - action_metadata_file_path, - ) - except Exception as e: - LOG.error( - 'Unable to delete "%s" file. Exception was "%s"', - action_metadata_file_path, - e, - ) - + delete_action_files_from_pack( + pack_name=pack_name, + entry_point=entry_point, + metadata_file=metadata_file, + ) except Exception as e: LOG.error( 'Database delete encountered exception during delete of id="%s". ' From c7354c71288d640390b28822f8338a38d520bf6e Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 22 Jul 2021 20:13:16 +0530 Subject: [PATCH 09/54] Updating /st2common/services/packs.py --- st2common/st2common/services/packs.py | 42 +++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 1d8bd69399..d63a0c3a67 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -17,12 +17,14 @@ import itertools +import os import requests import six from six.moves import range from oslo_config import cfg from st2common import log as logging +from st2common.content.utils import get_pack_base_path from st2common.persistence.pack import Pack from st2common.util.misc import lowercase_value from st2common.util.jsonify import json_encode @@ -32,6 +34,7 @@ "fetch_pack_index", "get_pack_from_index", "search_pack_index", + "delete_action_files_from_pack", ] EXCLUDE_FIELDS = ["repo_url", "email"] @@ -215,3 +218,42 @@ def search_pack_index( break return list(itertools.chain.from_iterable(matches)) + +def delete_action_files_from_pack(pack_name, entry_point, metadata_file): + """ + Prepares the path for entry_point file and metadata file of action and + deletes them from disk. + """ + pack_base_path = get_pack_base_path(pack_name=pack_name) + action_entrypoint_file_path = os.path.join(pack_base_path, "actions", entry_point) + action_metadata_file_path = os.path.join(pack_base_path, metadata_file) + + if os.path.exists(action_entrypoint_file_path): + try: + os.remove(action_entrypoint_file_path) + except PermissionError: + LOG.error( + 'No permission to delete the "%s" file', + action_entrypoint_file_path, + ) + except Exception as e: + LOG.error( + 'Unable to delete "%s" file. Exception was "%s"', + action_entrypoint_file_path, + e, + ) + + if os.path.exists(action_metadata_file_path): + try: + os.remove(action_metadata_file_path) + except PermissionError: + LOG.error( + 'No permission to delete the "%s" file', + action_metadata_file_path, + ) + except Exception as e: + LOG.error( + 'Unable to delete "%s" file. Exception was "%s"', + action_metadata_file_path, + e, + ) From b6ed390d95592736b352fe583428f7607e7ccc05 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 22 Jul 2021 20:15:07 +0530 Subject: [PATCH 10/54] Updating /st2common/services/packs.py --- st2common/st2common/services/packs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index d63a0c3a67..5368ec43cf 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -219,6 +219,7 @@ def search_pack_index( return list(itertools.chain.from_iterable(matches)) + def delete_action_files_from_pack(pack_name, entry_point, metadata_file): """ Prepares the path for entry_point file and metadata file of action and From 7395008b4c818b609e9cd3dd1659efc91b731a15 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 22 Jul 2021 20:21:53 +0530 Subject: [PATCH 11/54] Updating /st2client/commands/resource.py Added `--y` option for action delete command at CLI to make command scriptable --- st2client/st2client/commands/resource.py | 38 ++++++++++++++++-------- 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/st2client/st2client/commands/resource.py b/st2client/st2client/commands/resource.py index e3606cf53e..bc75ed20a0 100644 --- a/st2client/st2client/commands/resource.py +++ b/st2client/st2client/commands/resource.py @@ -732,27 +732,41 @@ def __init__(self, resource, *args, **kwargs): self.parser.add_argument(argument, metavar=metavar, help=help) + self.parser.add_argument( + "--yes", + action="store_true", + help="Auto yes flag to delete action files from disk.", + ) + @add_auth_token_to_kwargs_from_cli def run(self, args, **kwargs): resource_id = getattr(args, self.pk_argument_name, None) instance = self.get_resource(resource_id, **kwargs) - if isinstance(instance, st2client.models.action.Action): - user_input = input( - "It will delete action files on disk as well. Do you want to continue? (y/n): " + if args.yes: + self.manager.delete(instance, **kwargs) + print( + 'Resource with id "%s" has been successfully deleted from database and disk.' + % (resource_id) ) - if user_input.lower() == "y" or user_input.lower() == "yes": + else: + if isinstance(instance, st2client.models.action.Action): + user_input = input( + "It will delete action files on disk as well. Do you want to continue? (y/n): " + ) + if user_input.lower() == "y" or user_input.lower() == "yes": + self.manager.delete(instance, **kwargs) + print( + 'Resource with id "%s" has been successfully deleted from database and disk.' + % (resource_id) + ) + else: + print("Action is not deleted.") + else: self.manager.delete(instance, **kwargs) print( - 'Resource with id "%s" has been successfully deleted from database and disk.' + 'Resource with id "%s" has been successfully deleted.' % (resource_id) ) - else: - print("Action is not deleted.") - else: - self.manager.delete(instance, **kwargs) - print( - 'Resource with id "%s" has been successfully deleted.' % (resource_id) - ) def run_and_print(self, args, **kwargs): resource_id = getattr(args, self.pk_argument_name, None) From 284e130395faba42c94465104a94ba6268d87b3e Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 22 Jul 2021 20:25:26 +0530 Subject: [PATCH 12/54] Adding file test_packs.py Unit test for newly added code in /st2common/services/packs.py --- st2common/tests/unit/services/test_packs.py | 57 +++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 st2common/tests/unit/services/test_packs.py diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py new file mode 100644 index 0000000000..78c7ec32ef --- /dev/null +++ b/st2common/tests/unit/services/test_packs.py @@ -0,0 +1,57 @@ +# -*- coding: utf-8 -*- + +# Copyright 2020 The StackStorm Authors. +# Copyright 2019 Extreme Networks, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import absolute_import + +import os +import os.path +import unittest2 + +import st2tests + +from st2common.services.packs import delete_action_files_from_pack + +TEST_PACK = "dummy_pack_1" +TEST_PACK_PATH = ( + st2tests.fixturesloader.get_fixtures_packs_base_path() + "/" + TEST_PACK +) + + +class DeleteActionFilesTest(unittest2.TestCase): + + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + # creating test entry_point file in pack + with open(entry_point, "w") as f1: + f1.write("# Entry point file to be removed") + + # creating test metadata file in pack + with open(metadata_file, "w") as f1: + f1.write("# Metadata file to be removed") + + def test_delete_action_files_from_pack(self): + """ + Test that the action files present in the pack and removed + on the call of delete_action_files_from_pack function. + """ + + self.assertEqual(os.path.exists(self.entry_point), True) + self.assertEqual(os.path.exists(self.metadata_file), True) + delete_action_files_from_pack(TEST_PACK, self.entry_point, self.metadata_file) + self.assertEqual(os.path.exists(self.entry_point), False) + self.assertEqual(os.path.exists(self.metadata_file), False) From f929dc508c61da097f2ebb5452a7482b75ba4745 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 22 Jul 2021 21:31:05 +0530 Subject: [PATCH 13/54] Updating /st2client/commands/resource.py --- st2client/st2client/commands/resource.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2client/st2client/commands/resource.py b/st2client/st2client/commands/resource.py index bc75ed20a0..d42b67d054 100644 --- a/st2client/st2client/commands/resource.py +++ b/st2client/st2client/commands/resource.py @@ -756,7 +756,7 @@ def run(self, args, **kwargs): if user_input.lower() == "y" or user_input.lower() == "yes": self.manager.delete(instance, **kwargs) print( - 'Resource with id "%s" has been successfully deleted from database and disk.' + 'Resource with id "%s" has been successfully deleted from db and disk.' % (resource_id) ) else: From ad6c868a9b804e1ba079cd0640b3c7bec03f7c91 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 22 Jul 2021 21:48:05 +0530 Subject: [PATCH 14/54] Removing trailing whitespaces --- st2client/st2client/commands/resource.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2client/st2client/commands/resource.py b/st2client/st2client/commands/resource.py index d42b67d054..78184a4936 100644 --- a/st2client/st2client/commands/resource.py +++ b/st2client/st2client/commands/resource.py @@ -736,7 +736,7 @@ def __init__(self, resource, *args, **kwargs): "--yes", action="store_true", help="Auto yes flag to delete action files from disk.", - ) + ) @add_auth_token_to_kwargs_from_cli def run(self, args, **kwargs): From bee06cc2a1e10c46e99cf3c7c884f4d642f29610 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 23 Jul 2021 17:46:11 +0530 Subject: [PATCH 15/54] Updating test_packs.py --- st2common/tests/unit/services/test_packs.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py index 78c7ec32ef..9a6a71994a 100644 --- a/st2common/tests/unit/services/test_packs.py +++ b/st2common/tests/unit/services/test_packs.py @@ -50,8 +50,8 @@ def test_delete_action_files_from_pack(self): on the call of delete_action_files_from_pack function. """ - self.assertEqual(os.path.exists(self.entry_point), True) - self.assertEqual(os.path.exists(self.metadata_file), True) + self.assertTrue(os.path.exists(self.entry_point)) + self.assertTrue(os.path.exists(self.metadata_file)) delete_action_files_from_pack(TEST_PACK, self.entry_point, self.metadata_file) - self.assertEqual(os.path.exists(self.entry_point), False) - self.assertEqual(os.path.exists(self.metadata_file), False) + self.assertFalse(os.path.exists(self.entry_point)) + self.assertFalse(os.path.exists(self.metadata_file)) From 54a70e671c679bce9c663d748cded41c972a0105 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Mon, 26 Jul 2021 16:17:34 +0530 Subject: [PATCH 16/54] Updating /st2common/services/packs.py Raising INTERNAL_SERVER_ERROR if there is problem in removal of action files. It means API will fail. --- st2common/st2common/services/packs.py | 29 +++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 5368ec43cf..221190c6ca 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -26,6 +26,7 @@ from st2common import log as logging from st2common.content.utils import get_pack_base_path from st2common.persistence.pack import Pack +from st2common.router import abort from st2common.util.misc import lowercase_value from st2common.util.jsonify import json_encode @@ -43,6 +44,8 @@ LOG = logging.getLogger(__name__) +http_client = six.moves.http_client + def _build_index_list(index_url): if not index_url: @@ -237,12 +240,25 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): 'No permission to delete the "%s" file', action_entrypoint_file_path, ) + msg = 'No permission to delete "{0}" file from disk'.format( + action_entrypoint_file_path + ) + abort( + http_client.INTERNAL_SERVER_ERROR, + six.text_type(msg), + ) except Exception as e: LOG.error( 'Unable to delete "%s" file. Exception was "%s"', action_entrypoint_file_path, e, ) + msg = ( + 'Delete operation unsuccessful. "{0}" file still exists on disk'.format( + action_entrypoint_file_path + ) + ) + abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(msg)) if os.path.exists(action_metadata_file_path): try: @@ -252,9 +268,22 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): 'No permission to delete the "%s" file', action_metadata_file_path, ) + msg = 'No permission to delete "{0}" file from disk'.format( + action_metadata_file_path + ) + abort( + http_client.INTERNAL_SERVER_ERROR, + six.text_type(msg), + ) except Exception as e: LOG.error( 'Unable to delete "%s" file. Exception was "%s"', action_metadata_file_path, e, ) + msg = ( + 'Delete operation unsuccessful. "{0}" file still exists on disk'.format( + action_metadata_file_path + ) + ) + abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(msg)) From 8c37c5e850ee2e1982766fad20e6a8b0aa6f0ad6 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Mon, 26 Jul 2021 16:59:02 +0530 Subject: [PATCH 17/54] Updating /st2common/services/packs.py --- st2common/st2common/services/packs.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 221190c6ca..db9bbb2b0f 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -281,9 +281,10 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): action_metadata_file_path, e, ) - msg = ( - 'Delete operation unsuccessful. "{0}" file still exists on disk'.format( - action_metadata_file_path + if os.path.isfile(action_metadata_file_path): + msg = ( + 'Delete operation unsuccessful. "{0}" file still exists on disk'.format( + action_metadata_file_path + ) ) - ) - abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(msg)) + abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(msg)) From 0769eb125f0752ca207742142e1b528e3d00d9e6 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Mon, 26 Jul 2021 17:38:54 +0530 Subject: [PATCH 18/54] Updating /st2common/services/packs.py --- st2common/st2common/services/packs.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index db9bbb2b0f..eb32a1813b 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -282,9 +282,7 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): e, ) if os.path.isfile(action_metadata_file_path): - msg = ( - 'Delete operation unsuccessful. "{0}" file still exists on disk'.format( - action_metadata_file_path - ) + msg = 'Delete operation unsuccessful. "{0}" file still exists on disk'.format( + action_metadata_file_path ) abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(msg)) From 9b9b9d08c31c053af08be80de50b0d438fcc095a Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Mon, 26 Jul 2021 19:37:19 +0530 Subject: [PATCH 19/54] Updating test_packs.py --- st2common/tests/unit/services/test_packs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py index 9a6a71994a..d450d113b5 100644 --- a/st2common/tests/unit/services/test_packs.py +++ b/st2common/tests/unit/services/test_packs.py @@ -37,11 +37,11 @@ class DeleteActionFilesTest(unittest2.TestCase): metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") # creating test entry_point file in pack - with open(entry_point, "w") as f1: + with open(entry_point, "w") as f: f1.write("# Entry point file to be removed") # creating test metadata file in pack - with open(metadata_file, "w") as f1: + with open(metadata_file, "w") as f: f1.write("# Metadata file to be removed") def test_delete_action_files_from_pack(self): From 45a7fb939da52f5c2eb6134c9f15f6140b8d33ef Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Mon, 26 Jul 2021 19:42:33 +0530 Subject: [PATCH 20/54] Updating test_packs.py --- st2common/tests/unit/services/test_packs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py index d450d113b5..02f9e86903 100644 --- a/st2common/tests/unit/services/test_packs.py +++ b/st2common/tests/unit/services/test_packs.py @@ -38,11 +38,11 @@ class DeleteActionFilesTest(unittest2.TestCase): # creating test entry_point file in pack with open(entry_point, "w") as f: - f1.write("# Entry point file to be removed") + f.write("# Entry point file to be removed") # creating test metadata file in pack with open(metadata_file, "w") as f: - f1.write("# Metadata file to be removed") + f.write("# Metadata file to be removed") def test_delete_action_files_from_pack(self): """ From bf538d07c6e17c3741bba7de8e7f450f25f7bc6e Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 27 Jul 2021 19:34:51 +0530 Subject: [PATCH 21/54] Updating /st2client/commands/resource.py --- st2client/st2client/commands/resource.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2client/st2client/commands/resource.py b/st2client/st2client/commands/resource.py index 78184a4936..c215af2bb6 100644 --- a/st2client/st2client/commands/resource.py +++ b/st2client/st2client/commands/resource.py @@ -751,7 +751,7 @@ def run(self, args, **kwargs): else: if isinstance(instance, st2client.models.action.Action): user_input = input( - "It will delete action files on disk as well. Do you want to continue? (y/n): " + "The resource files on disk will be deleted. Do you want to continue? (y/n): " ) if user_input.lower() == "y" or user_input.lower() == "yes": self.manager.delete(instance, **kwargs) From ea3dac32fccf397422ea8bd2037c3baad457cab5 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 27 Jul 2021 19:37:25 +0530 Subject: [PATCH 22/54] Creating file /st2common/exceptions/packs.py --- st2common/st2common/exceptions/packs.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 st2common/st2common/exceptions/packs.py diff --git a/st2common/st2common/exceptions/packs.py b/st2common/st2common/exceptions/packs.py new file mode 100644 index 0000000000..b02dcc851b --- /dev/null +++ b/st2common/st2common/exceptions/packs.py @@ -0,0 +1,25 @@ +# Copyright 2020 The StackStorm Authors. +# Copyright 2019 Extreme Networks, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import absolute_import +from st2common.exceptions import StackStormBaseException + +__all__ = [ + "ResourceDiskFilesRemovalError", +] + + +class ResourceDiskFilesRemovalError(StackStormBaseException): + pass From 947a16cd32296e31b6f6e5d26d19415b803a7776 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 27 Jul 2021 19:44:09 +0530 Subject: [PATCH 23/54] Updating st2common/services/packs.py --- st2common/st2common/services/packs.py | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index eb32a1813b..750dbb81b0 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -25,8 +25,8 @@ from st2common import log as logging from st2common.content.utils import get_pack_base_path +from st2common.exceptions.packs import ResourceDiskFilesRemovalError from st2common.persistence.pack import Pack -from st2common.router import abort from st2common.util.misc import lowercase_value from st2common.util.jsonify import json_encode @@ -44,8 +44,6 @@ LOG = logging.getLogger(__name__) -http_client = six.moves.http_client - def _build_index_list(index_url): if not index_url: @@ -243,10 +241,7 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): msg = 'No permission to delete "{0}" file from disk'.format( action_entrypoint_file_path ) - abort( - http_client.INTERNAL_SERVER_ERROR, - six.text_type(msg), - ) + raise PermissionError(msg) except Exception as e: LOG.error( 'Unable to delete "%s" file. Exception was "%s"', @@ -258,7 +253,7 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): action_entrypoint_file_path ) ) - abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(msg)) + raise ResourceDiskFilesRemovalError(msg) if os.path.exists(action_metadata_file_path): try: @@ -271,10 +266,7 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): msg = 'No permission to delete "{0}" file from disk'.format( action_metadata_file_path ) - abort( - http_client.INTERNAL_SERVER_ERROR, - six.text_type(msg), - ) + raise PermissionError(msg) except Exception as e: LOG.error( 'Unable to delete "%s" file. Exception was "%s"', @@ -282,7 +274,9 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): e, ) if os.path.isfile(action_metadata_file_path): - msg = 'Delete operation unsuccessful. "{0}" file still exists on disk'.format( - action_metadata_file_path + msg = ( + 'Delete operation unsuccessful. "{0}" file still exists on disk'.format( + action_metadata_file_path + ) ) - abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(msg)) + raise ResourceDiskFilesRemovalError(msg) From 4995144c96f47827960c6f4e852faba0808e341b Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 27 Jul 2021 19:47:35 +0530 Subject: [PATCH 24/54] Adding unit tests in test_packs.py --- st2common/tests/unit/services/test_packs.py | 168 ++++++++++++++++++-- 1 file changed, 152 insertions(+), 16 deletions(-) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py index 02f9e86903..cbbd3a67eb 100644 --- a/st2common/tests/unit/services/test_packs.py +++ b/st2common/tests/unit/services/test_packs.py @@ -19,6 +19,7 @@ import os import os.path +import mock import unittest2 import st2tests @@ -32,26 +33,161 @@ class DeleteActionFilesTest(unittest2.TestCase): + def test_delete_action_files_from_pack(self): + """ + Test that the action files present in the pack and removed + on the call of delete_action_files_from_pack function. + """ - entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") - metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") - # creating test entry_point file in pack - with open(entry_point, "w") as f: - f.write("# Entry point file to be removed") + # creating test entry_point file in dummy pack + with open(entry_point, "w") as f: + f.write("# entry point file to be removed") - # creating test metadata file in pack - with open(metadata_file, "w") as f: - f.write("# Metadata file to be removed") + # creating test metadata file in dummy pack + with open(metadata_file, "w") as f: + f.write("# metadata file to be removed") - def test_delete_action_files_from_pack(self): + # asserting both entry_point and metadata files exists + self.assertTrue(os.path.exists(entry_point)) + self.assertTrue(os.path.exists(metadata_file)) + + delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) + + # asserting both entry_point and metadata files removed and they doesn't exists + self.assertFalse(os.path.exists(entry_point)) + self.assertFalse(os.path.exists(metadata_file)) + + def test_entry_point_file_does_not_exists(self): """ - Test that the action files present in the pack and removed - on the call of delete_action_files_from_pack function. + Tests that entry_point file doesn't exists at the path and if action delete + api calls delete_action_files_from_pack function, it doesn't affect + """ + + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + # creating only test metadata file in dummy pack + with open(metadata_file, "w") as f: + f.write("# metadata file to be removed") + + # asserting entry_point file doesn't exists + self.assertFalse(os.path.exists(entry_point)) + + # asserting metadata files exists + self.assertTrue(os.path.exists(metadata_file)) + + delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) + + # asserting both entry_point and metadata files doesn't exists + self.assertFalse(os.path.exists(entry_point)) + self.assertFalse(os.path.exists(metadata_file)) + + def test_metadata_file_does_not_exists(self): + """ + Tests that metadata file doesn't exists at the path and if action delete + api calls delete_action_files_from_pack function, it doesn't affect """ - self.assertTrue(os.path.exists(self.entry_point)) - self.assertTrue(os.path.exists(self.metadata_file)) - delete_action_files_from_pack(TEST_PACK, self.entry_point, self.metadata_file) - self.assertFalse(os.path.exists(self.entry_point)) - self.assertFalse(os.path.exists(self.metadata_file)) + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + # creating only test entry_point file in dummy pack + with open(entry_point, "w") as f: + f.write("# entry point file to be removed") + + # asserting metadata file doesn't exists + self.assertFalse(os.path.exists(metadata_file)) + + # asserting entry_point files exists + self.assertTrue(os.path.exists(entry_point)) + + delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) + + # asserting both entry_point and metadata files doesn't exists + self.assertFalse(os.path.exists(entry_point)) + self.assertFalse(os.path.exists(metadata_file)) + + +class DeleteActionFilesErrorTest(unittest2.TestCase): + def setUp(self): + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + # creating test entry_point file in dummy pack + with open(entry_point, "w") as f: + f.write("# entry point file to be removed") + + # creating test metadata file in dummy pack + with open(metadata_file, "w") as f: + f.write("# metadata file to be removed") + + def tearDown(self): + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + # removing test entry_point and metadata file in dummy pack + os.remove(entry_point) + os.remove(metadata_file) + + @mock.patch.object(os, "remove") + def test_permission_error_to_remove_resource_entry_point_file(self, remove): + + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + remove.side_effect = PermissionError("No permission to delete file from disk") + + # asserting metadata files exists + self.assertTrue(os.path.exists(entry_point)) + + # asserting delete_action_files_from_pack function raises PermissionError + with self.assertRaises(PermissionError): + delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) + + @mock.patch.object(os, "remove") + def test_permission_error_to_remove_resource_metadata_file(self, remove): + + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + remove.side_effect = PermissionError("No permission to delete file from disk") + + # asserting metadata files exists + self.assertTrue(os.path.exists(metadata_file)) + + # asserting delete_action_files_from_pack function raises PermissionError + with self.assertRaises(PermissionError): + delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) + + @mock.patch.object(os, "remove") + def test_exception_to_remove_resource_entry_point_file(self, remove): + + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + remove.side_effect = Exception("Another exception occured") + + # asserting metadata files exists + self.assertTrue(os.path.exists(entry_point)) + + # asserting delete_action_files_from_pack function raises exception + with self.assertRaises(Exception): + delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) + + @mock.patch.object(os, "remove") + def test_exception_to_remove_resource_metadata_file(self, remove): + + entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + remove.side_effect = Exception("Another exception occured") + + # asserting metadata files exists + self.assertTrue(os.path.exists(metadata_file)) + + # asserting delete_action_files_from_pack function raises exception + with self.assertRaises(Exception): + delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) From e974022734198fc40eabcd37ef8399660099af2b Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 27 Jul 2021 20:08:54 +0530 Subject: [PATCH 25/54] Updating /st2common/services/packs.py --- st2common/st2common/services/packs.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 750dbb81b0..a5a3a31c0b 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -274,9 +274,7 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): e, ) if os.path.isfile(action_metadata_file_path): - msg = ( - 'Delete operation unsuccessful. "{0}" file still exists on disk'.format( - action_metadata_file_path - ) + msg = 'Delete operation unsuccessful. "{0}" file still exists on disk'.format( + action_metadata_file_path ) raise ResourceDiskFilesRemovalError(msg) From d95d8b7b231a18f42c87131dbc62e61d176dcdf9 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 30 Jul 2021 13:03:01 +0530 Subject: [PATCH 26/54] Updating resource.py --- st2client/st2client/commands/resource.py | 30 ++++-------------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/st2client/st2client/commands/resource.py b/st2client/st2client/commands/resource.py index c215af2bb6..7cb149cb10 100644 --- a/st2client/st2client/commands/resource.py +++ b/st2client/st2client/commands/resource.py @@ -35,7 +35,6 @@ from st2client.exceptions.operations import OperationFailureException from st2client.formatters import table from st2client.utils.types import OrderedSet -import st2client ALLOWED_EXTS = [".json", ".yaml", ".yml"] PARSER_FUNCS = {".json": json.load, ".yml": yaml.safe_load, ".yaml": yaml.safe_load} @@ -742,37 +741,16 @@ def __init__(self, resource, *args, **kwargs): def run(self, args, **kwargs): resource_id = getattr(args, self.pk_argument_name, None) instance = self.get_resource(resource_id, **kwargs) - if args.yes: - self.manager.delete(instance, **kwargs) - print( - 'Resource with id "%s" has been successfully deleted from database and disk.' - % (resource_id) - ) - else: - if isinstance(instance, st2client.models.action.Action): - user_input = input( - "The resource files on disk will be deleted. Do you want to continue? (y/n): " - ) - if user_input.lower() == "y" or user_input.lower() == "yes": - self.manager.delete(instance, **kwargs) - print( - 'Resource with id "%s" has been successfully deleted from db and disk.' - % (resource_id) - ) - else: - print("Action is not deleted.") - else: - self.manager.delete(instance, **kwargs) - print( - 'Resource with id "%s" has been successfully deleted.' - % (resource_id) - ) + self.manager.delete(instance, **kwargs) def run_and_print(self, args, **kwargs): resource_id = getattr(args, self.pk_argument_name, None) try: self.run(args, **kwargs) + print( + 'Resource with id "%s" has been successfully deleted.' % (resource_id) + ) except ResourceNotFoundError: self.print_not_found(resource_id) raise OperationFailureException("Resource %s not found." % resource_id) From 9e99615e7c9ccff068d0f87693406286ae257518 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 30 Jul 2021 13:12:12 +0530 Subject: [PATCH 27/54] Updating action.py --- st2client/st2client/commands/action.py | 33 +++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index 17a0627871..488d4e648e 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -35,6 +35,7 @@ from st2client.commands.resource import ResourceNotFoundError from st2client.commands.resource import ResourceViewCommand from st2client.commands.resource import add_auth_token_to_kwargs_from_cli +from st2client.exceptions.operations import OperationFailureException from st2client.formatters import table from st2client.formatters import execution as execution_formatter from st2client.utils import jsutil @@ -278,7 +279,37 @@ class ActionDisableCommand(resource.ContentPackResourceDisableCommand): class ActionDeleteCommand(resource.ContentPackResourceDeleteCommand): - pass + @add_auth_token_to_kwargs_from_cli + def run(self, args, **kwargs): + resource_id = getattr(args, self.pk_argument_name, None) + instance = self.get_resource(resource_id, **kwargs) + if args.yes: + self.manager.delete(instance, **kwargs) + print( + 'Resource with id "%s" has been successfully deleted from database and disk.' + % (resource_id) + ) + else: + user_input = input( + "The resource files on disk will be deleted. Do you want to continue? (y/n): " + ) + if user_input.lower() == "y" or user_input.lower() == "yes": + self.manager.delete(instance, **kwargs) + print( + 'Resource with id "%s" has been successfully deleted from db and disk.' + % (resource_id) + ) + else: + print("Action is not deleted.") + + def run_and_print(self, args, **kwargs): + resource_id = getattr(args, self.pk_argument_name, None) + + try: + self.run(args, **kwargs) + except ResourceNotFoundError: + self.print_not_found(resource_id) + raise OperationFailureException("Resource %s not found." % resource_id) class ActionRunCommandMixin(object): From 4916c0409056de05387e743baf8c1c7a5a495ba1 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 30 Jul 2021 13:13:53 +0530 Subject: [PATCH 28/54] Updating test_packs.py --- st2common/tests/unit/services/test_packs.py | 86 +++++++++++++-------- 1 file changed, 53 insertions(+), 33 deletions(-) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py index cbbd3a67eb..55626cfd93 100644 --- a/st2common/tests/unit/services/test_packs.py +++ b/st2common/tests/unit/services/test_packs.py @@ -42,11 +42,11 @@ def test_delete_action_files_from_pack(self): entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") - # creating test entry_point file in dummy pack + # creating entry_point file in dummy pack with open(entry_point, "w") as f: f.write("# entry point file to be removed") - # creating test metadata file in dummy pack + # creating metadata file in dummy pack with open(metadata_file, "w") as f: f.write("# metadata file to be removed") @@ -56,7 +56,7 @@ def test_delete_action_files_from_pack(self): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) - # asserting both entry_point and metadata files removed and they doesn't exists + # asserting both entry_point and metadata files removed and they don't exist self.assertFalse(os.path.exists(entry_point)) self.assertFalse(os.path.exists(metadata_file)) @@ -69,11 +69,11 @@ def test_entry_point_file_does_not_exists(self): entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") - # creating only test metadata file in dummy pack + # creating only metadata file in dummy pack with open(metadata_file, "w") as f: f.write("# metadata file to be removed") - # asserting entry_point file doesn't exists + # asserting entry_point file doesn't exist self.assertFalse(os.path.exists(entry_point)) # asserting metadata files exists @@ -81,7 +81,7 @@ def test_entry_point_file_does_not_exists(self): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) - # asserting both entry_point and metadata files doesn't exists + # asserting both entry_point and metadata files don't exist self.assertFalse(os.path.exists(entry_point)) self.assertFalse(os.path.exists(metadata_file)) @@ -94,11 +94,11 @@ def test_metadata_file_does_not_exists(self): entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") - # creating only test entry_point file in dummy pack + # creating only entry_point file in dummy pack with open(entry_point, "w") as f: f.write("# entry point file to be removed") - # asserting metadata file doesn't exists + # asserting metadata file doesn't exist self.assertFalse(os.path.exists(metadata_file)) # asserting entry_point files exists @@ -106,31 +106,24 @@ def test_metadata_file_does_not_exists(self): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) - # asserting both entry_point and metadata files doesn't exists + # asserting both entry_point and metadata files don't exist self.assertFalse(os.path.exists(entry_point)) self.assertFalse(os.path.exists(metadata_file)) -class DeleteActionFilesErrorTest(unittest2.TestCase): +class DeleteActionEntryPointFilesErrorTest(unittest2.TestCase): def setUp(self): entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") - metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") - # creating test entry_point file in dummy pack + # creating entry_point file in dummy pack with open(entry_point, "w") as f: f.write("# entry point file to be removed") - # creating test metadata file in dummy pack - with open(metadata_file, "w") as f: - f.write("# metadata file to be removed") - def tearDown(self): entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") - metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") - # removing test entry_point and metadata file in dummy pack + # removing entry_point file from dummy pack os.remove(entry_point) - os.remove(metadata_file) @mock.patch.object(os, "remove") def test_permission_error_to_remove_resource_entry_point_file(self, remove): @@ -140,41 +133,65 @@ def test_permission_error_to_remove_resource_entry_point_file(self, remove): remove.side_effect = PermissionError("No permission to delete file from disk") - # asserting metadata files exists + # asserting entry_point files exists self.assertTrue(os.path.exists(entry_point)) - # asserting delete_action_files_from_pack function raises PermissionError + # asserting metadata file doesn't exist + self.assertFalse(os.path.exists(metadata_file)) + + # asserting delete_action_files_from_pack function raises PermissionError for entry_point file with self.assertRaises(PermissionError): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) @mock.patch.object(os, "remove") - def test_permission_error_to_remove_resource_metadata_file(self, remove): + def test_exception_to_remove_resource_entry_point_file(self, remove): entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") - remove.side_effect = PermissionError("No permission to delete file from disk") + remove.side_effect = Exception("Another exception occured") - # asserting metadata files exists - self.assertTrue(os.path.exists(metadata_file)) + # asserting entry_point files exists + self.assertTrue(os.path.exists(entry_point)) - # asserting delete_action_files_from_pack function raises PermissionError - with self.assertRaises(PermissionError): + # asserting metadata file doesn't exist + self.assertFalse(os.path.exists(metadata_file)) + + # asserting delete_action_files_from_pack function raises exception for entry_point file + with self.assertRaises(Exception): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) + +class DeleteActionMetadataFilesErrorTest(unittest2.TestCase): + def setUp(self): + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + # creating metadata file in dummy pack + with open(metadata_file, "w") as f: + f.write("# metadata file to be removed") + + def tearDown(self): + metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") + + # removing metadata file from dummy pack + os.remove(metadata_file) + @mock.patch.object(os, "remove") - def test_exception_to_remove_resource_entry_point_file(self, remove): + def test_permission_error_to_remove_resource_metadata_file(self, remove): entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") - remove.side_effect = Exception("Another exception occured") + remove.side_effect = PermissionError("No permission to delete file from disk") # asserting metadata files exists - self.assertTrue(os.path.exists(entry_point)) + self.assertTrue(os.path.exists(metadata_file)) - # asserting delete_action_files_from_pack function raises exception - with self.assertRaises(Exception): + # asserting entry_point doesn't exist + self.assertFalse(os.path.exists(entry_point)) + + # asserting delete_action_files_from_pack function raises PermissionError for metadata file + with self.assertRaises(PermissionError): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) @mock.patch.object(os, "remove") @@ -188,6 +205,9 @@ def test_exception_to_remove_resource_metadata_file(self, remove): # asserting metadata files exists self.assertTrue(os.path.exists(metadata_file)) - # asserting delete_action_files_from_pack function raises exception + # asserting entry_point doesn't exist + self.assertFalse(os.path.exists(entry_point)) + + # asserting delete_action_files_from_pack function raises exception for metadata file with self.assertRaises(Exception): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) From 5c0eb6f38e870ba1a4d9773982e869dfabcb7536 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 30 Jul 2021 13:21:42 +0530 Subject: [PATCH 29/54] Updating content.py --- st2common/st2common/exceptions/content.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/st2common/st2common/exceptions/content.py b/st2common/st2common/exceptions/content.py index 3b3449c45b..2336e92d89 100644 --- a/st2common/st2common/exceptions/content.py +++ b/st2common/st2common/exceptions/content.py @@ -23,3 +23,7 @@ class UnsupportedMetaException(StackStormBaseException): class ParseException(ValueError): pass + + +class ResourceDiskFilesRemovalError(StackStormBaseException): + pass From edce1d812b5aa9f18af86a468314d7525e8df0f7 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 30 Jul 2021 13:24:30 +0530 Subject: [PATCH 30/54] Updating packs.py --- st2common/st2common/services/packs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index a5a3a31c0b..4f5ba9c021 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -25,7 +25,7 @@ from st2common import log as logging from st2common.content.utils import get_pack_base_path -from st2common.exceptions.packs import ResourceDiskFilesRemovalError +from st2common.exceptions.content import ResourceDiskFilesRemovalError from st2common.persistence.pack import Pack from st2common.util.misc import lowercase_value from st2common.util.jsonify import json_encode From 4da0f48f74a4e2a9a4b7e154598803a80246f07f Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 30 Jul 2021 13:26:37 +0530 Subject: [PATCH 31/54] Deleting packs.py --- st2common/st2common/exceptions/packs.py | 25 ------------------------- 1 file changed, 25 deletions(-) delete mode 100644 st2common/st2common/exceptions/packs.py diff --git a/st2common/st2common/exceptions/packs.py b/st2common/st2common/exceptions/packs.py deleted file mode 100644 index b02dcc851b..0000000000 --- a/st2common/st2common/exceptions/packs.py +++ /dev/null @@ -1,25 +0,0 @@ -# Copyright 2020 The StackStorm Authors. -# Copyright 2019 Extreme Networks, Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from __future__ import absolute_import -from st2common.exceptions import StackStormBaseException - -__all__ = [ - "ResourceDiskFilesRemovalError", -] - - -class ResourceDiskFilesRemovalError(StackStormBaseException): - pass From 2654528e8653ff3c4239759c0566a3825a9529ea Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 3 Aug 2021 18:17:34 +0530 Subject: [PATCH 32/54] Updating /st2client/commands/resource.py adding `-f` and `--force` flags instead of `--yes` flag for action delete command to auto delete resource files. --- st2client/st2client/commands/resource.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/st2client/st2client/commands/resource.py b/st2client/st2client/commands/resource.py index 7cb149cb10..dc15b43ca6 100644 --- a/st2client/st2client/commands/resource.py +++ b/st2client/st2client/commands/resource.py @@ -732,7 +732,13 @@ def __init__(self, resource, *args, **kwargs): self.parser.add_argument(argument, metavar=metavar, help=help) self.parser.add_argument( - "--yes", + "-f", + action="store_true", + help="Auto yes flag to delete action files from disk.", + ) + + self.parser.add_argument( + "--force", action="store_true", help="Auto yes flag to delete action files from disk.", ) From 6179ca61d2db97a190809a24ad9b73dfc29b9d25 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 3 Aug 2021 18:20:56 +0530 Subject: [PATCH 33/54] Updating /st2client/commands/action.py --- st2client/st2client/commands/action.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index 488d4e648e..269f26ae48 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -35,7 +35,6 @@ from st2client.commands.resource import ResourceNotFoundError from st2client.commands.resource import ResourceViewCommand from st2client.commands.resource import add_auth_token_to_kwargs_from_cli -from st2client.exceptions.operations import OperationFailureException from st2client.formatters import table from st2client.formatters import execution as execution_formatter from st2client.utils import jsutil @@ -283,22 +282,19 @@ class ActionDeleteCommand(resource.ContentPackResourceDeleteCommand): def run(self, args, **kwargs): resource_id = getattr(args, self.pk_argument_name, None) instance = self.get_resource(resource_id, **kwargs) - if args.yes: + resource_delete_msg = 'Resource with id "{0}" has been successfully deleted from database and disk.'.format( + resource_id + ) + if args.f or args.force: self.manager.delete(instance, **kwargs) - print( - 'Resource with id "%s" has been successfully deleted from database and disk.' - % (resource_id) - ) + print(resource_delete_msg) else: user_input = input( "The resource files on disk will be deleted. Do you want to continue? (y/n): " ) if user_input.lower() == "y" or user_input.lower() == "yes": self.manager.delete(instance, **kwargs) - print( - 'Resource with id "%s" has been successfully deleted from db and disk.' - % (resource_id) - ) + print(resource_delete_msg) else: print("Action is not deleted.") @@ -309,7 +305,6 @@ def run_and_print(self, args, **kwargs): self.run(args, **kwargs) except ResourceNotFoundError: self.print_not_found(resource_id) - raise OperationFailureException("Resource %s not found." % resource_id) class ActionRunCommandMixin(object): From 8743f82834eecf112397af1ef89fb5759d0ea277 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 3 Aug 2021 23:32:52 +0530 Subject: [PATCH 34/54] Updating /st2common/services/packs.py --- st2common/st2common/services/packs.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 4f5ba9c021..3130d1d7f8 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -235,7 +235,7 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): os.remove(action_entrypoint_file_path) except PermissionError: LOG.error( - 'No permission to delete the "%s" file', + 'No permission to delete "%s" file', action_entrypoint_file_path, ) msg = 'No permission to delete "{0}" file from disk'.format( @@ -260,7 +260,7 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): os.remove(action_metadata_file_path) except PermissionError: LOG.error( - 'No permission to delete the "%s" file', + 'No permission to delete "%s" file', action_metadata_file_path, ) msg = 'No permission to delete "{0}" file from disk'.format( From d35a59e25e2c19504cb2448a08b15f21f389f1fb Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 4 Aug 2021 20:19:56 +0530 Subject: [PATCH 35/54] Moving `-f` and `--force` arguments to ActionDeleteCommand --- st2client/st2client/commands/resource.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/st2client/st2client/commands/resource.py b/st2client/st2client/commands/resource.py index dc15b43ca6..d66e990a93 100644 --- a/st2client/st2client/commands/resource.py +++ b/st2client/st2client/commands/resource.py @@ -731,18 +731,6 @@ def __init__(self, resource, *args, **kwargs): self.parser.add_argument(argument, metavar=metavar, help=help) - self.parser.add_argument( - "-f", - action="store_true", - help="Auto yes flag to delete action files from disk.", - ) - - self.parser.add_argument( - "--force", - action="store_true", - help="Auto yes flag to delete action files from disk.", - ) - @add_auth_token_to_kwargs_from_cli def run(self, args, **kwargs): resource_id = getattr(args, self.pk_argument_name, None) From bee5a3ff47e8822fb215ec9002872ad614898fff Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 4 Aug 2021 20:22:56 +0530 Subject: [PATCH 36/54] Added `-f` and `--force` args under ActionDeleteCommand --- st2client/st2client/commands/action.py | 33 +++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index 269f26ae48..751e6253d3 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -277,7 +277,38 @@ class ActionDisableCommand(resource.ContentPackResourceDisableCommand): ] -class ActionDeleteCommand(resource.ContentPackResourceDeleteCommand): +class ActionDeleteCommand(resource.ResourceCommand): + pk_argument_name = "ref_or_id" + + def __init__(self, resource, *args, **kwargs): + super(ActionDeleteCommand, self).__init__( + resource, + "delete", + "Delete an existing %s." % resource.get_display_name().lower(), + *args, + **kwargs, + ) + + argument = self.pk_argument_name + metavar = self._get_metavar_for_argument(argument=self.pk_argument_name) + help = self._get_help_for_argument( + resource=resource, argument=self.pk_argument_name + ) + + self.parser.add_argument(argument, metavar=metavar, help=help) + + self.parser.add_argument( + "-f", + action="store_true", + help="Auto yes flag to delete action files from disk.", + ) + + self.parser.add_argument( + "--force", + action="store_true", + help="Auto yes flag to delete action files from disk.", + ) + @add_auth_token_to_kwargs_from_cli def run(self, args, **kwargs): resource_id = getattr(args, self.pk_argument_name, None) From 858b3b11498f1b8213339d15b142e16d46944c1f Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 5 Aug 2021 11:32:01 +0530 Subject: [PATCH 37/54] Updating /st2client/commands/action.py --- st2client/st2client/commands/action.py | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index 751e6253d3..ca018f8e25 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -277,25 +277,9 @@ class ActionDisableCommand(resource.ContentPackResourceDisableCommand): ] -class ActionDeleteCommand(resource.ResourceCommand): - pk_argument_name = "ref_or_id" - +class ActionDeleteCommand(resource.ContentPackResourceDeleteCommand): def __init__(self, resource, *args, **kwargs): - super(ActionDeleteCommand, self).__init__( - resource, - "delete", - "Delete an existing %s." % resource.get_display_name().lower(), - *args, - **kwargs, - ) - - argument = self.pk_argument_name - metavar = self._get_metavar_for_argument(argument=self.pk_argument_name) - help = self._get_help_for_argument( - resource=resource, argument=self.pk_argument_name - ) - - self.parser.add_argument(argument, metavar=metavar, help=help) + super(ActionDeleteCommand, self).__init__(resource, *args, **kwargs) self.parser.add_argument( "-f", From 5439154db7977c19d7459e57ab8f0cf6e3222416 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 5 Aug 2021 12:09:04 +0530 Subject: [PATCH 38/54] Updating /st2client/commands/action.py --- st2client/st2client/commands/action.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index ca018f8e25..ef85003662 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -277,7 +277,7 @@ class ActionDisableCommand(resource.ContentPackResourceDisableCommand): ] -class ActionDeleteCommand(resource.ContentPackResourceDeleteCommand): +class ActionDeleteCommand(resource.ResourceDeleteCommand): def __init__(self, resource, *args, **kwargs): super(ActionDeleteCommand, self).__init__(resource, *args, **kwargs) From f003988b59385f66b55a7649b1d02a4cf3964403 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 5 Aug 2021 13:12:25 +0530 Subject: [PATCH 39/54] Updating /st2client/commands/action.py --- st2client/st2client/commands/action.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index ef85003662..ca018f8e25 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -277,7 +277,7 @@ class ActionDisableCommand(resource.ContentPackResourceDisableCommand): ] -class ActionDeleteCommand(resource.ResourceDeleteCommand): +class ActionDeleteCommand(resource.ContentPackResourceDeleteCommand): def __init__(self, resource, *args, **kwargs): super(ActionDeleteCommand, self).__init__(resource, *args, **kwargs) From d8db49e76340656456ac51cbf5f7283a4e250b33 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Thu, 5 Aug 2021 17:39:30 +0530 Subject: [PATCH 40/54] Updating /st2common/services/packs.py --- st2common/st2common/services/packs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 3130d1d7f8..71d1c577a1 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -226,6 +226,7 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): Prepares the path for entry_point file and metadata file of action and deletes them from disk. """ + pack_base_path = get_pack_base_path(pack_name=pack_name) action_entrypoint_file_path = os.path.join(pack_base_path, "actions", entry_point) action_metadata_file_path = os.path.join(pack_base_path, metadata_file) From 04bc3b2ac7e686554a7acb5c48e399b649ae64aa Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Fri, 6 Aug 2021 09:43:43 +0530 Subject: [PATCH 41/54] Combining `-f` and `--force` args for action delete command --- st2client/st2client/commands/action.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index ca018f8e25..235347cfc2 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -283,13 +283,9 @@ def __init__(self, resource, *args, **kwargs): self.parser.add_argument( "-f", - action="store_true", - help="Auto yes flag to delete action files from disk.", - ) - - self.parser.add_argument( "--force", action="store_true", + dest="force", help="Auto yes flag to delete action files from disk.", ) @@ -300,7 +296,7 @@ def run(self, args, **kwargs): resource_delete_msg = 'Resource with id "{0}" has been successfully deleted from database and disk.'.format( resource_id ) - if args.f or args.force: + if args.force: self.manager.delete(instance, **kwargs) print(resource_delete_msg) else: From 6bd83fedba9619beb15fd61a3f6495b52a761ff1 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 10 Aug 2021 15:59:42 +0530 Subject: [PATCH 42/54] Adding extra try/except block for disk file delete --- st2api/st2api/controllers/v1/actions.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index 0815374cc3..984f1a48de 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -243,11 +243,20 @@ def delete(self, ref_or_id, requester_user): try: Action.delete(action_db) - delete_action_files_from_pack( - pack_name=pack_name, - entry_point=entry_point, - metadata_file=metadata_file, - ) + try: + delete_action_files_from_pack( + pack_name=pack_name, + entry_point=entry_point, + metadata_file=metadata_file, + ) + except Exception as e: + LOG.error( + "Exception encountered during deleting resource files from disk." + "Exception was %s", + e, + ) + abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) + return except Exception as e: LOG.error( 'Database delete encountered exception during delete of id="%s". ' From 37a36de4d08dbb4d10e34d5b33eefaf68dd5fb6d Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 10 Aug 2021 20:07:41 +0530 Subject: [PATCH 43/54] Updating CHANGELOG.rst --- CHANGELOG.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index c5421460ef..197b8edef3 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -282,6 +282,16 @@ Changed * Update orquesta to v1.4.0. +* Modified action delete api. Action delete api removes related action/workflow files on disk + along with de-registering them from database. Prompts on CLI for user permission before + removing disk files. + + ``-f`` and ``--force`` arguments added for action delete CLI command as auto yes flag and + will delete related files on disk without prompting for user permission. #5304 + + Contributed by @mahesh-orch. + + Improvements ~~~~~~~~~~~~ From dcbf567cf2e75c7a90d08395548baaaa719825b4 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 10 Aug 2021 20:13:57 +0530 Subject: [PATCH 44/54] Updating CHANGELOG.rst --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 197b8edef3..b470087bb1 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -286,7 +286,7 @@ Changed along with de-registering them from database. Prompts on CLI for user permission before removing disk files. - ``-f`` and ``--force`` arguments added for action delete CLI command as auto yes flag and + ``-f`` and ``--force`` arguments added for action delete CLI command as auto yes flag and will delete related files on disk without prompting for user permission. #5304 Contributed by @mahesh-orch. From 89cbc60fb2cecbac97256f9913747eaf7472310d Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 10 Aug 2021 20:26:59 +0530 Subject: [PATCH 45/54] Updating CHANGELOG.rst --- CHANGELOG.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b470087bb1..9648a1acc7 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -282,8 +282,8 @@ Changed * Update orquesta to v1.4.0. -* Modified action delete api. Action delete api removes related action/workflow files on disk - along with de-registering them from database. Prompts on CLI for user permission before +* Modified action delete api. Action delete api removes related action/workflow files on disk + along with de-registering them from database. Prompts on CLI for user permission before removing disk files. ``-f`` and ``--force`` arguments added for action delete CLI command as auto yes flag and From b581fc72b29ec660049f309ee2513641ab018907 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 11 Aug 2021 20:04:08 +0530 Subject: [PATCH 46/54] CHANGELOG for modification in action delete API shifted to 'in developement-changed' section --- CHANGELOG.rst | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 9648a1acc7..17421827a5 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,6 +7,15 @@ in development Changed ~~~~~~~ +* Modified action delete api. Action delete api removes related action/workflow files on disk + along with de-registering them from database. Prompts on CLI for user permission before + removing disk files. + + ``-f`` and ``--force`` arguments added for action delete CLI command as auto yes flag and + will delete related files on disk without prompting for user permission. #5304 + + Contributed by @mahesh-orch. + * Removed --python3 deprecated flag from st2client. #5305 Contributed by Amanda McGuinness (@amanda11 Ammeon Solutions) @@ -282,16 +291,6 @@ Changed * Update orquesta to v1.4.0. -* Modified action delete api. Action delete api removes related action/workflow files on disk - along with de-registering them from database. Prompts on CLI for user permission before - removing disk files. - - ``-f`` and ``--force`` arguments added for action delete CLI command as auto yes flag and - will delete related files on disk without prompting for user permission. #5304 - - Contributed by @mahesh-orch. - - Improvements ~~~~~~~~~~~~ From 24cbc0af833d253916256e658eff02542cd1f3ce Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 11 Aug 2021 20:23:58 +0530 Subject: [PATCH 47/54] Adding log warning in case of file to be removed doesn't exists and updating message to return in case of failure in deleting action files --- st2common/st2common/services/packs.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 71d1c577a1..37ee815655 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -236,7 +236,7 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): os.remove(action_entrypoint_file_path) except PermissionError: LOG.error( - 'No permission to delete "%s" file', + 'No permission to delete the "%s" file', action_entrypoint_file_path, ) msg = 'No permission to delete "{0}" file from disk'.format( @@ -249,19 +249,22 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): action_entrypoint_file_path, e, ) - msg = ( - 'Delete operation unsuccessful. "{0}" file still exists on disk'.format( - action_entrypoint_file_path - ) + msg = 'The action file "{0}" could not be removed from disk, please check the logs or ask your StackStorm administrator to check and delete the actions files manually'.format( + action_entrypoint_file_path ) raise ResourceDiskFilesRemovalError(msg) + else: + LOG.warning( + 'The action entry point file "%s" does not exists on disk.', + action_entrypoint_file_path, + ) if os.path.exists(action_metadata_file_path): try: os.remove(action_metadata_file_path) except PermissionError: LOG.error( - 'No permission to delete "%s" file', + 'No permission to delete the "%s" file', action_metadata_file_path, ) msg = 'No permission to delete "{0}" file from disk'.format( @@ -270,12 +273,17 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): raise PermissionError(msg) except Exception as e: LOG.error( - 'Unable to delete "%s" file. Exception was "%s"', + 'Could not delete "%s" file. Exception was "%s"', action_metadata_file_path, e, ) if os.path.isfile(action_metadata_file_path): - msg = 'Delete operation unsuccessful. "{0}" file still exists on disk'.format( + msg = 'The action file "{0}" could not be removed from disk, please check the logs or ask your StackStorm administrator to check and delete the actions files manually'.format( action_metadata_file_path ) raise ResourceDiskFilesRemovalError(msg) + else: + LOG.warning( + 'The action metadata file "%s" does not exists on disk.', + action_metadata_file_path, + ) From 80788e8e16fc014a57abc4138464458332846d6f Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 11 Aug 2021 20:30:53 +0530 Subject: [PATCH 48/54] Adding assertions for error messages. Using `os.path.join` to prepare path instead of string concatenation. Adding docstring for test classes to describe specificness. --- st2common/tests/unit/services/test_packs.py | 40 +++++++++++++++------ 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py index 55626cfd93..636751e104 100644 --- a/st2common/tests/unit/services/test_packs.py +++ b/st2common/tests/unit/services/test_packs.py @@ -18,7 +18,6 @@ from __future__ import absolute_import import os -import os.path import mock import unittest2 @@ -27,8 +26,8 @@ from st2common.services.packs import delete_action_files_from_pack TEST_PACK = "dummy_pack_1" -TEST_PACK_PATH = ( - st2tests.fixturesloader.get_fixtures_packs_base_path() + "/" + TEST_PACK +TEST_PACK_PATH = os.path.join( + st2tests.fixturesloader.get_fixtures_packs_base_path(), TEST_PACK ) @@ -112,6 +111,12 @@ def test_metadata_file_does_not_exists(self): class DeleteActionEntryPointFilesErrorTest(unittest2.TestCase): + """ + Testing that exceptions are thrown by delete_action_files_from_pack function for + entry point file. Here only entry point file is created and metadata file doesn't + exist + """ + def setUp(self): entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") @@ -139,8 +144,10 @@ def test_permission_error_to_remove_resource_entry_point_file(self, remove): # asserting metadata file doesn't exist self.assertFalse(os.path.exists(metadata_file)) - # asserting delete_action_files_from_pack function raises PermissionError for entry_point file - with self.assertRaises(PermissionError): + expected_msg = 'No permission to delete "/home/ashwini/stackstormst2-repo/st2/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/test_entry_point.py" file from disk' + + # asserting PermissionError with message on call of delete_action_files_from_pack to delete entry_point file + with self.assertRaisesRegex(PermissionError, expected_msg): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) @mock.patch.object(os, "remove") @@ -157,12 +164,19 @@ def test_exception_to_remove_resource_entry_point_file(self, remove): # asserting metadata file doesn't exist self.assertFalse(os.path.exists(metadata_file)) - # asserting delete_action_files_from_pack function raises exception for entry_point file - with self.assertRaises(Exception): + expected_msg = 'The action file "/home/ashwini/stackstormst2-repo/st2/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/test_entry_point.py" could not be removed from disk, please check the logs or ask your StackStorm administrator to check and delete the actions files manually' + + # asserting exception with message on call of delete_action_files_from_pack to delete entry_point file + with self.assertRaisesRegex(Exception, expected_msg): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) class DeleteActionMetadataFilesErrorTest(unittest2.TestCase): + """ + Testing that exceptions are thrown by delete_action_files_from_pack function for + metadata file. Here only metadata file is created and metadata file doesn't exist + """ + def setUp(self): metadata_file = os.path.join(TEST_PACK_PATH, "actions", "test_metadata.yaml") @@ -190,8 +204,10 @@ def test_permission_error_to_remove_resource_metadata_file(self, remove): # asserting entry_point doesn't exist self.assertFalse(os.path.exists(entry_point)) - # asserting delete_action_files_from_pack function raises PermissionError for metadata file - with self.assertRaises(PermissionError): + expected_msg = 'No permission to delete "/home/ashwini/stackstormst2-repo/st2/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/test_metadata.yaml" file from disk' + + # asserting PermissionError with message on call of delete_action_files_from_pack to delete metadata file + with self.assertRaisesRegex(PermissionError, expected_msg): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) @mock.patch.object(os, "remove") @@ -208,6 +224,8 @@ def test_exception_to_remove_resource_metadata_file(self, remove): # asserting entry_point doesn't exist self.assertFalse(os.path.exists(entry_point)) - # asserting delete_action_files_from_pack function raises exception for metadata file - with self.assertRaises(Exception): + expected_msg = 'The action file "/home/ashwini/stackstormst2-repo/st2/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/test_metadata.yaml" could not be removed from disk, please check the logs or ask your StackStorm administrator to check and delete the actions files manually' + + # asserting exception with message on call of delete_action_files_from_pack to delete metadata file + with self.assertRaisesRegex(Exception, expected_msg): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) From 749c1872908c10e20ec4466daa90914f8fa33a36 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 11 Aug 2021 20:39:45 +0530 Subject: [PATCH 49/54] Removing an unnecessary argument from getattr() and cleaning up logic to remove duplication. --- st2client/st2client/commands/action.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index 235347cfc2..59315b12f4 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -296,21 +296,19 @@ def run(self, args, **kwargs): resource_delete_msg = 'Resource with id "{0}" has been successfully deleted from database and disk.'.format( resource_id ) - if args.force: - self.manager.delete(instance, **kwargs) - print(resource_delete_msg) - else: + user_input = "" + if not args.force: user_input = input( "The resource files on disk will be deleted. Do you want to continue? (y/n): " ) - if user_input.lower() == "y" or user_input.lower() == "yes": - self.manager.delete(instance, **kwargs) - print(resource_delete_msg) - else: - print("Action is not deleted.") + if args.force or user_input.lower() == "y" or user_input.lower() == "yes": + self.manager.delete(instance, **kwargs) + print(resource_delete_msg) + else: + print("Action is not deleted.") def run_and_print(self, args, **kwargs): - resource_id = getattr(args, self.pk_argument_name, None) + resource_id = getattr(args, self.pk_argument_name) try: self.run(args, **kwargs) From ec59efd52678099dda2fce38afcb2a16dcc1672d Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 11 Aug 2021 20:46:55 +0530 Subject: [PATCH 50/54] Putting try/except blocks one after another instead of nesting --- st2api/st2api/controllers/v1/actions.py | 30 ++++++++++++------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index 984f1a48de..1e8bf7c7b5 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -18,8 +18,8 @@ import stat import errno -from mongoengine import ValidationError import six +from mongoengine import ValidationError # TODO: Encapsulate mongoengine errors in our persistence layer. Exceptions # that bubble up to this layer should be core Python exceptions or @@ -243,20 +243,6 @@ def delete(self, ref_or_id, requester_user): try: Action.delete(action_db) - try: - delete_action_files_from_pack( - pack_name=pack_name, - entry_point=entry_point, - metadata_file=metadata_file, - ) - except Exception as e: - LOG.error( - "Exception encountered during deleting resource files from disk." - "Exception was %s", - e, - ) - abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) - return except Exception as e: LOG.error( 'Database delete encountered exception during delete of id="%s". ' @@ -266,6 +252,20 @@ def delete(self, ref_or_id, requester_user): ) abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) return + try: + delete_action_files_from_pack( + pack_name=pack_name, + entry_point=entry_point, + metadata_file=metadata_file, + ) + except Exception as e: + LOG.error( + "Exception encountered during deleting resource files from disk." + "Exception was %s", + e, + ) + abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) + return extra = {"action_db": action_db} LOG.audit("Action deleted. Action.id=%s" % (action_db.id), extra=extra) From 13eae5465c4216efb073ba5025e18b4ea49f454e Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Wed, 11 Aug 2021 20:58:09 +0530 Subject: [PATCH 51/54] Updating expected_msg variable to take generic path for entry point and metadata file --- st2common/tests/unit/services/test_packs.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py index 636751e104..1cbd0ad7f5 100644 --- a/st2common/tests/unit/services/test_packs.py +++ b/st2common/tests/unit/services/test_packs.py @@ -144,10 +144,12 @@ def test_permission_error_to_remove_resource_entry_point_file(self, remove): # asserting metadata file doesn't exist self.assertFalse(os.path.exists(metadata_file)) - expected_msg = 'No permission to delete "/home/ashwini/stackstormst2-repo/st2/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/test_entry_point.py" file from disk' + expected_msg = 'No permission to delete "{0}" file from disk'.format( + entry_point + ) # asserting PermissionError with message on call of delete_action_files_from_pack to delete entry_point file - with self.assertRaisesRegex(PermissionError, expected_msg): + with self.assertRaisesRegexp(PermissionError, expected_msg): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) @mock.patch.object(os, "remove") @@ -164,10 +166,12 @@ def test_exception_to_remove_resource_entry_point_file(self, remove): # asserting metadata file doesn't exist self.assertFalse(os.path.exists(metadata_file)) - expected_msg = 'The action file "/home/ashwini/stackstormst2-repo/st2/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/test_entry_point.py" could not be removed from disk, please check the logs or ask your StackStorm administrator to check and delete the actions files manually' + expected_msg = 'The action file "{0}" could not be removed from disk, please check the logs or ask your StackStorm administrator to check and delete the actions files manually'.format( + entry_point + ) # asserting exception with message on call of delete_action_files_from_pack to delete entry_point file - with self.assertRaisesRegex(Exception, expected_msg): + with self.assertRaisesRegexp(Exception, expected_msg): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) @@ -204,7 +208,9 @@ def test_permission_error_to_remove_resource_metadata_file(self, remove): # asserting entry_point doesn't exist self.assertFalse(os.path.exists(entry_point)) - expected_msg = 'No permission to delete "/home/ashwini/stackstormst2-repo/st2/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/test_metadata.yaml" file from disk' + expected_msg = 'No permission to delete "{0}" file from disk'.format( + metadata_file + ) # asserting PermissionError with message on call of delete_action_files_from_pack to delete metadata file with self.assertRaisesRegex(PermissionError, expected_msg): @@ -224,7 +230,9 @@ def test_exception_to_remove_resource_metadata_file(self, remove): # asserting entry_point doesn't exist self.assertFalse(os.path.exists(entry_point)) - expected_msg = 'The action file "/home/ashwini/stackstormst2-repo/st2/st2tests/st2tests/fixtures/packs/dummy_pack_1/actions/test_metadata.yaml" could not be removed from disk, please check the logs or ask your StackStorm administrator to check and delete the actions files manually' + expected_msg = 'The action file "{0}" could not be removed from disk, please check the logs or ask your StackStorm administrator to check and delete the actions files manually'.format( + metadata_file + ) # asserting exception with message on call of delete_action_files_from_pack to delete metadata file with self.assertRaisesRegex(Exception, expected_msg): From 63f3d126369b595eb56338f3e2725e2b419b19a0 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 17 Aug 2021 15:01:56 +0530 Subject: [PATCH 52/54] Removing redundant check, adding an appropriate check for file at path and fixing long line msg --- st2common/st2common/services/packs.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/st2common/st2common/services/packs.py b/st2common/st2common/services/packs.py index 37ee815655..74e97e6424 100644 --- a/st2common/st2common/services/packs.py +++ b/st2common/st2common/services/packs.py @@ -231,7 +231,7 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): action_entrypoint_file_path = os.path.join(pack_base_path, "actions", entry_point) action_metadata_file_path = os.path.join(pack_base_path, metadata_file) - if os.path.exists(action_entrypoint_file_path): + if os.path.isfile(action_entrypoint_file_path): try: os.remove(action_entrypoint_file_path) except PermissionError: @@ -239,7 +239,7 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): 'No permission to delete the "%s" file', action_entrypoint_file_path, ) - msg = 'No permission to delete "{0}" file from disk'.format( + msg = 'No permission to delete "%s" file from disk' % ( action_entrypoint_file_path ) raise PermissionError(msg) @@ -249,8 +249,10 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): action_entrypoint_file_path, e, ) - msg = 'The action file "{0}" could not be removed from disk, please check the logs or ask your StackStorm administrator to check and delete the actions files manually'.format( - action_entrypoint_file_path + msg = ( + 'The action file "%s" could not be removed from disk, please ' + "check the logs or ask your StackStorm administrator to check " + "and delete the actions files manually" % (action_entrypoint_file_path) ) raise ResourceDiskFilesRemovalError(msg) else: @@ -259,7 +261,7 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): action_entrypoint_file_path, ) - if os.path.exists(action_metadata_file_path): + if os.path.isfile(action_metadata_file_path): try: os.remove(action_metadata_file_path) except PermissionError: @@ -267,7 +269,7 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): 'No permission to delete the "%s" file', action_metadata_file_path, ) - msg = 'No permission to delete "{0}" file from disk'.format( + msg = 'No permission to delete "%s" file from disk' % ( action_metadata_file_path ) raise PermissionError(msg) @@ -277,11 +279,12 @@ def delete_action_files_from_pack(pack_name, entry_point, metadata_file): action_metadata_file_path, e, ) - if os.path.isfile(action_metadata_file_path): - msg = 'The action file "{0}" could not be removed from disk, please check the logs or ask your StackStorm administrator to check and delete the actions files manually'.format( - action_metadata_file_path - ) - raise ResourceDiskFilesRemovalError(msg) + msg = ( + 'The action file "%s" could not be removed from disk, please ' + "check the logs or ask your StackStorm administrator to check " + "and delete the actions files manually" % (action_metadata_file_path) + ) + raise ResourceDiskFilesRemovalError(msg) else: LOG.warning( 'The action metadata file "%s" does not exists on disk.', From 936f618114e9b2dfb120a2575e72bab9e32c3a95 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 17 Aug 2021 15:06:52 +0530 Subject: [PATCH 53/54] Minor fixes for long line msgs, comments, typos in comments and method name for error msg assertions --- st2common/tests/unit/services/test_packs.py | 64 +++++++++++---------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/st2common/tests/unit/services/test_packs.py b/st2common/tests/unit/services/test_packs.py index 1cbd0ad7f5..04e68dee51 100644 --- a/st2common/tests/unit/services/test_packs.py +++ b/st2common/tests/unit/services/test_packs.py @@ -55,14 +55,14 @@ def test_delete_action_files_from_pack(self): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) - # asserting both entry_point and metadata files removed and they don't exist + # asserting both entry_point and metadata files removed and they doesn't exist self.assertFalse(os.path.exists(entry_point)) self.assertFalse(os.path.exists(metadata_file)) def test_entry_point_file_does_not_exists(self): """ Tests that entry_point file doesn't exists at the path and if action delete - api calls delete_action_files_from_pack function, it doesn't affect + api calls delete_action_files_from_pack function, it doesn't affect. """ entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") @@ -87,7 +87,7 @@ def test_entry_point_file_does_not_exists(self): def test_metadata_file_does_not_exists(self): """ Tests that metadata file doesn't exists at the path and if action delete - api calls delete_action_files_from_pack function, it doesn't affect + api calls delete_action_files_from_pack function, it doesn't affect. """ entry_point = os.path.join(TEST_PACK_PATH, "actions", "test_entry_point.py") @@ -100,7 +100,7 @@ def test_metadata_file_does_not_exists(self): # asserting metadata file doesn't exist self.assertFalse(os.path.exists(metadata_file)) - # asserting entry_point files exists + # asserting entry_point file exists self.assertTrue(os.path.exists(entry_point)) delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) @@ -112,9 +112,9 @@ def test_metadata_file_does_not_exists(self): class DeleteActionEntryPointFilesErrorTest(unittest2.TestCase): """ - Testing that exceptions are thrown by delete_action_files_from_pack function for - entry point file. Here only entry point file is created and metadata file doesn't - exist + Testing that exceptions are thrown by delete_action_files_from_pack function + for entry point file. Here only entry point file is created and metadata + file doesn't exist. """ def setUp(self): @@ -138,17 +138,16 @@ def test_permission_error_to_remove_resource_entry_point_file(self, remove): remove.side_effect = PermissionError("No permission to delete file from disk") - # asserting entry_point files exists + # asserting entry_point file exists self.assertTrue(os.path.exists(entry_point)) # asserting metadata file doesn't exist self.assertFalse(os.path.exists(metadata_file)) - expected_msg = 'No permission to delete "{0}" file from disk'.format( - entry_point - ) + expected_msg = 'No permission to delete "%s" file from disk' % (entry_point) - # asserting PermissionError with message on call of delete_action_files_from_pack to delete entry_point file + # asserting PermissionError with message on call of delete_action_files_from_pack + # to delete entry_point file with self.assertRaisesRegexp(PermissionError, expected_msg): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) @@ -160,17 +159,20 @@ def test_exception_to_remove_resource_entry_point_file(self, remove): remove.side_effect = Exception("Another exception occured") - # asserting entry_point files exists + # asserting entry_point file exists self.assertTrue(os.path.exists(entry_point)) # asserting metadata file doesn't exist self.assertFalse(os.path.exists(metadata_file)) - expected_msg = 'The action file "{0}" could not be removed from disk, please check the logs or ask your StackStorm administrator to check and delete the actions files manually'.format( - entry_point + expected_msg = ( + 'The action file "%s" could not be removed from disk, please ' + "check the logs or ask your StackStorm administrator to check " + "and delete the actions files manually" % (entry_point) ) - # asserting exception with message on call of delete_action_files_from_pack to delete entry_point file + # asserting exception with message on call of delete_action_files_from_pack + # to delete entry_point file with self.assertRaisesRegexp(Exception, expected_msg): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) @@ -178,7 +180,7 @@ def test_exception_to_remove_resource_entry_point_file(self, remove): class DeleteActionMetadataFilesErrorTest(unittest2.TestCase): """ Testing that exceptions are thrown by delete_action_files_from_pack function for - metadata file. Here only metadata file is created and metadata file doesn't exist + metadata file. Here only metadata file is created and metadata file doesn't exist. """ def setUp(self): @@ -202,18 +204,17 @@ def test_permission_error_to_remove_resource_metadata_file(self, remove): remove.side_effect = PermissionError("No permission to delete file from disk") - # asserting metadata files exists + # asserting metadata file exists self.assertTrue(os.path.exists(metadata_file)) - # asserting entry_point doesn't exist + # asserting entry_point file doesn't exist self.assertFalse(os.path.exists(entry_point)) - expected_msg = 'No permission to delete "{0}" file from disk'.format( - metadata_file - ) + expected_msg = 'No permission to delete "%s" file from disk' % (metadata_file) - # asserting PermissionError with message on call of delete_action_files_from_pack to delete metadata file - with self.assertRaisesRegex(PermissionError, expected_msg): + # asserting PermissionError with message on call of delete_action_files_from_pack + # to delete metadata file + with self.assertRaisesRegexp(PermissionError, expected_msg): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) @mock.patch.object(os, "remove") @@ -224,16 +225,19 @@ def test_exception_to_remove_resource_metadata_file(self, remove): remove.side_effect = Exception("Another exception occured") - # asserting metadata files exists + # asserting metadata file exists self.assertTrue(os.path.exists(metadata_file)) - # asserting entry_point doesn't exist + # asserting entry_point file doesn't exist self.assertFalse(os.path.exists(entry_point)) - expected_msg = 'The action file "{0}" could not be removed from disk, please check the logs or ask your StackStorm administrator to check and delete the actions files manually'.format( - metadata_file + expected_msg = ( + 'The action file "%s" could not be removed from disk, please ' + "check the logs or ask your StackStorm administrator to check " + "and delete the actions files manually" % (metadata_file) ) - # asserting exception with message on call of delete_action_files_from_pack to delete metadata file - with self.assertRaisesRegex(Exception, expected_msg): + # asserting exception with message on call of delete_action_files_from_pack + # to delete metadata file + with self.assertRaisesRegexp(Exception, expected_msg): delete_action_files_from_pack(TEST_PACK, entry_point, metadata_file) From 96029a09e311b30c173d3b081e14c0941a19f425 Mon Sep 17 00:00:00 2001 From: mahesh-orch <81608498+mahesh-orch@users.noreply.github.com> Date: Tue, 17 Aug 2021 15:11:39 +0530 Subject: [PATCH 54/54] Minor fixes for variable name for msg and length of the msg line --- st2client/st2client/commands/action.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index 59315b12f4..29963fd2de 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -293,8 +293,9 @@ def __init__(self, resource, *args, **kwargs): def run(self, args, **kwargs): resource_id = getattr(args, self.pk_argument_name, None) instance = self.get_resource(resource_id, **kwargs) - resource_delete_msg = 'Resource with id "{0}" has been successfully deleted from database and disk.'.format( - resource_id + msg = ( + 'Resource with id "%s" has been successfully deleted from database and disk.' + % (resource_id) ) user_input = "" if not args.force: @@ -303,7 +304,7 @@ def run(self, args, **kwargs): ) if args.force or user_input.lower() == "y" or user_input.lower() == "yes": self.manager.delete(instance, **kwargs) - print(resource_delete_msg) + print(msg) else: print("Action is not deleted.")