Remove session dep from CommandParameters

CommandParameters is moving (but is not quite there) towards being
only concerned with command parameter validation and restructuring.
It's should not be concerned with also making S3 requests, _especially_
recursively deleting customer objects.  This is better suited for
the CommandArchitecture or the RbCommand objects.  While it would
take more work to get it there, the S3TransferCommand base class seems
to be a better fit and an incremental improvement.  It makes the
CommandParameters class simpler.

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
 

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,44 +529,35 @@ 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'])
         self.assertFalse(cmd_params.parameters['dir_op'])
 
     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'])
         self.assertFalse(cmd_params.parameters['dir_op'])
 
     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()