diff --git a/awscli/customizations/s3/subcommands.py b/awscli/customizations/s3/subcommands.py index 10aec665efa9..eadc29d23af2 100644 --- a/awscli/customizations/s3/subcommands.py +++ b/awscli/customizations/s3/subcommands.py @@ -460,14 +460,14 @@ def _run_main(self, parsed_args, parsed_globals): super(S3TransferCommand, self)._run_main(parsed_args, parsed_globals) self._convert_path_args(parsed_args) params = self._build_call_parameters(parsed_args, {}) - cmd_params = CommandParameters(self._session, self.NAME, params, + cmd_params = CommandParameters(self.NAME, params, self.USAGE) cmd_params.add_region(parsed_globals) cmd_params.add_endpoint_url(parsed_globals) cmd_params.add_verify_ssl(parsed_globals) cmd_params.add_page_size(parsed_args) cmd_params.add_paths(parsed_args.paths) - cmd_params.check_force(parsed_globals) + self._handle_rm_force(parsed_globals, cmd_params.parameters) cmd = CommandArchitecture(self._session, self.NAME, cmd_params.parameters) cmd.set_endpoints() @@ -494,6 +494,26 @@ def _convert_path_args(self, parsed_args): new_path = enc_path.decode('utf-8') parsed_args.paths[i] = new_path + def _handle_rm_force(self, parsed_globals, parameters): + """ + This function recursive deletes objects in a bucket if the force + parameters was thrown when using the remove bucket command. + """ + # XXX: This shouldn't really be here. This was originally moved from + # the CommandParameters class to here, but this is still not the ideal + # place for this code. This should be moved + # to either the CommandArchitecture class, or the RbCommand class where + # the actual operations against S3 are performed. This may require + # some refactoring though to move this to either of those classes. + # For now, moving this out of CommandParameters allows for that class + # to be kept simple. + if 'force' in parameters: + if parameters['force']: + bucket = find_bucket_key(parameters['src'][5:])[0] + path = 's3://' + bucket + del_objects = RmCommand(self._session) + del_objects([path, '--recursive'], parsed_globals) + class CpCommand(S3TransferCommand): NAME = 'cp' @@ -789,12 +809,16 @@ class CommandParameters(object): This class is used to do some initial error based on the parameters and arguments passed to the command line. """ - def __init__(self, session, cmd, parameters, usage): + def __init__(self, cmd, parameters, usage): """ Stores command name and parameters. Ensures that the ``dir_op`` flag is true if a certain command is being used. + + :param cmd: The name of the command, e.g. "rm". + :param parameters: A dictionary of parameters. + :param usage: A usage string + """ - self.session = session self.cmd = cmd self.parameters = parameters self.usage = usage @@ -914,21 +938,6 @@ def check_src_path(self, paths): else: raise Exception("Error: Local path does not exist") - def check_force(self, parsed_globals): - """ - This function recursive deletes objects in a bucket if the force - parameters was thrown when using the remove bucket command. - """ - if 'force' in self.parameters: - if self.parameters['force']: - bucket = find_bucket_key(self.parameters['src'][5:])[0] - path = 's3://' + bucket - try: - del_objects = RmCommand(self.session) - del_objects([path, '--recursive'], parsed_globals) - except: - pass - def add_region(self, parsed_globals): self.parameters['region'] = parsed_globals.region diff --git a/tests/unit/customizations/s3/test_subcommands.py b/tests/unit/customizations/s3/test_subcommands.py index f99ab2504e07..a23682c050b0 100644 --- a/tests/unit/customizations/s3/test_subcommands.py +++ b/tests/unit/customizations/s3/test_subcommands.py @@ -447,16 +447,14 @@ def setUp(self): self.environ = {} self.environ_patch = patch('os.environ', self.environ) self.environ_patch.start() - self.session = FakeSession() self.mock = MagicMock() self.mock.get_config = MagicMock(return_value={'region': None}) self.loc_files = make_loc_files() - self.bucket = make_s3_files(self.session) + self.bucket = 's3testbucket' def tearDown(self): self.environ_patch.stop() clean_loc_files(self.loc_files) - s3_cleanup(self.bucket, self.session) def test_check_path_type_pass(self): # This tests the class's ability to determine whether the correct @@ -477,7 +475,7 @@ def test_check_path_type_pass(self): 'locallocal': [local_file, local_file]} for cmd in cmds.keys(): - cmd_param = CommandParameters(self.session, cmd, {}, '') + cmd_param = CommandParameters(cmd, {}, '') cmd_param.add_region(mock.Mock()) correct_paths = cmds[cmd] for path_args in correct_paths: @@ -505,7 +503,7 @@ def test_check_path_type_fail(self): 'locallocal': [local_file, local_file]} for cmd in cmds.keys(): - cmd_param = CommandParameters(self.session, cmd, {}, '') + cmd_param = CommandParameters(cmd, {}, '') cmd_param.add_region(mock.Mock()) wrong_paths = cmds[cmd] for path_args in wrong_paths: @@ -531,22 +529,13 @@ def test_check_src_path_pass(self): parameters = {} for filename in files: parameters['dir_op'] = filename[1] - cmd_parameter = CommandParameters(self.session, 'put', - parameters, '') + cmd_parameter = CommandParameters('put', parameters, '') cmd_parameter.add_region(mock.Mock()) cmd_parameter.check_src_path(filename[0]) - def test_check_force(self): - # This checks to make sure that the force parameter is run. If - # successful. The delete command will fail as the bucket is empty - # and be caught by the exception. - cmd_params = CommandParameters(self.session, 'rb', {'force': True},'') - cmd_params.parameters['src'] = 's3://mybucket' - cmd_params.check_force(None) - def test_validate_streaming_paths_upload(self): parameters = {'src': '-', 'dest': 's3://bucket'} - cmd_params = CommandParameters(self.session, 'cp', parameters, '') + cmd_params = CommandParameters('cp', parameters, '') cmd_params._validate_streaming_paths() self.assertTrue(cmd_params.parameters['is_stream']) self.assertTrue(cmd_params.parameters['only_show_errors']) @@ -554,7 +543,7 @@ def test_validate_streaming_paths_upload(self): def test_validate_streaming_paths_download(self): parameters = {'src': 'localfile', 'dest': '-'} - cmd_params = CommandParameters(self.session, 'cp', parameters, '') + cmd_params = CommandParameters('cp', parameters, '') cmd_params._validate_streaming_paths() self.assertTrue(cmd_params.parameters['is_stream']) self.assertTrue(cmd_params.parameters['only_show_errors']) @@ -562,13 +551,13 @@ def test_validate_streaming_paths_download(self): def test_validate_no_streaming_paths(self): parameters = {'src': 'localfile', 'dest': 's3://bucket'} - cmd_params = CommandParameters(self.session, 'cp', parameters, '') + cmd_params = CommandParameters('cp', parameters, '') cmd_params._validate_streaming_paths() self.assertFalse(cmd_params.parameters['is_stream']) def test_validate_streaming_paths_error(self): parameters = {'src': '-', 'dest': 's3://bucket'} - cmd_params = CommandParameters(self.session, 'sync', parameters, '') + cmd_params = CommandParameters('sync', parameters, '') with self.assertRaises(ValueError): cmd_params._validate_streaming_paths()