From 99db44b86c35b0d81bd37c7e9146599ebddade09 Mon Sep 17 00:00:00 2001 From: Mathieu Guillaume Date: Fri, 8 Nov 2013 18:19:00 +0100 Subject: [PATCH 1/3] Add back support for --endpoint-url option to s3 --- awscli/customizations/s3/s3.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/awscli/customizations/s3/s3.py b/awscli/customizations/s3/s3.py index a9c9c77b519a..4d241de6e5c6 100644 --- a/awscli/customizations/s3/s3.py +++ b/awscli/customizations/s3/s3.py @@ -269,6 +269,7 @@ def _do_command(self, parsed_args, parsed_globals): params = self._build_call_parameters(parsed_args, {}) cmd_params = CommandParameters(self._session, self._name, params) cmd_params.check_region(parsed_globals) + cmd_params.check_endpoint_url(parsed_globals) cmd_params.add_paths(parsed_args.paths) cmd_params.check_force(parsed_globals) cmd = CommandArchitecture(self._session, self._name, @@ -434,7 +435,8 @@ def __init__(self, session, cmd, parameters): self.parameters = parameters self.instructions = [] self._service = self.session.get_service('s3') - self._endpoint = self._service.get_endpoint(self.parameters['region']) + self._endpoint = self._service.get_endpoint(region_name=self.parameters['region'], + endpoint_url=self.parameters['endpoint_url']) def create_instructions(self): """ @@ -731,15 +733,31 @@ def check_region(self, parsed_globals): parsed_region = None if 'region' in parsed_globals: parsed_region = getattr(parsed_globals, 'region') - if not region and not parsed_region: + if 'endpoint_url' in parsed_globals: + parsed_endpoint_url = getattr(parsed_globals, 'endpoint_url') + else: + parsed_endpoint_url = None + if not region and not parsed_region and parsed_endpoint_url is None: raise Exception("A region must be specified --region or " "specifying the region\nin a configuration " - "file or as an environment variable\n") + "file or as an environment variable.\n" + "Alternately, an endpoint can be specified " + "with --endpoint-url") if parsed_region: self.parameters['region'] = parsed_region - else: + elif region: self.parameters['region'] = region + else: + self.parameters['region'] = None + def check_endpoint_url(self, parsed_globals): + """ + Adds endpoint_url to the parameters. + """ + if 'endpoint_url' in parsed_globals: + self.parameters['endpoint_url'] = getattr(parsed_globals, 'endpoint_url') + else: + self.parameters['endpoint_url'] = None # This is a dictionary useful for automatically adding the different commands, # the amount of arguments it takes, and the optional parameters that can appear From dc14b66124d87afa113014b09e587fab86498064 Mon Sep 17 00:00:00 2001 From: Mathieu Guillaume Date: Fri, 8 Nov 2013 18:32:06 +0100 Subject: [PATCH 2/3] Update S3 unit tests for endpoint_url param --- awscli/customizations/s3/s3.py | 2 +- tests/unit/customizations/s3/fake_session.py | 4 +-- tests/unit/customizations/s3/test_s3.py | 28 ++++++++++++-------- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/awscli/customizations/s3/s3.py b/awscli/customizations/s3/s3.py index 4d241de6e5c6..f045b6faa1c3 100644 --- a/awscli/customizations/s3/s3.py +++ b/awscli/customizations/s3/s3.py @@ -750,7 +750,7 @@ def check_region(self, parsed_globals): else: self.parameters['region'] = None - def check_endpoint_url(self, parsed_globals): + def check_endpoint_url(self, parsed_globals): """ Adds endpoint_url to the parameters. """ diff --git a/tests/unit/customizations/s3/fake_session.py b/tests/unit/customizations/s3/fake_session.py index f89f5d281553..6fe6691f854a 100644 --- a/tests/unit/customizations/s3/fake_session.py +++ b/tests/unit/customizations/s3/fake_session.py @@ -77,9 +77,9 @@ class FakeService(object): def __init__(self, session): self.session = session - def get_endpoint(self, region): + def get_endpoint(self, region_name, endpoint_url=None): endpoint = Mock() - endpoint.region_name = region + endpoint.region_name = region_name return endpoint def get_operation(self, name): diff --git a/tests/unit/customizations/s3/test_s3.py b/tests/unit/customizations/s3/test_s3.py index af974aa171fb..ee160cac868e 100644 --- a/tests/unit/customizations/s3/test_s3.py +++ b/tests/unit/customizations/s3/test_s3.py @@ -169,9 +169,9 @@ def test_create_instructions(self): 'mb': ['s3_handler'], 'rb': ['s3_handler']} - params = {'filters': True, 'region': 'us-east-1'} + params = {'filters': True, 'region': 'us-east-1', 'endpoint_url': None} for cmd in cmds: - cmd_arc = CommandArchitecture(self.session, cmd, {'region': 'us-east-1'}) + cmd_arc = CommandArchitecture(self.session, cmd, {'region': 'us-east-1', 'endpoint_url': None}) cmd_arc.create_instructions() self.assertEqual(cmd_arc.instructions, instructions[cmd]) @@ -191,7 +191,8 @@ def test_run_cp_put(self): filters = [['--include', '*']] params = {'dir_op': False, 'dryrun': True, 'quiet': False, 'src': local_file, 'dest': s3_file, 'filters': filters, - 'paths_type': 'locals3', 'region': 'us-east-1'} + 'paths_type': 'locals3', 'region': 'us-east-1', + 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'cp', params) cmd_arc.create_instructions() cmd_arc.run() @@ -208,7 +209,8 @@ def test_run_cp_get(self): filters = [['--include', '*']] params = {'dir_op': False, 'dryrun': True, 'quiet': False, 'src': s3_file, 'dest': local_file, 'filters': filters, - 'paths_type': 's3local', 'region': 'us-east-1'} + 'paths_type': 's3local', 'region': 'us-east-1', + 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'cp', params) cmd_arc.create_instructions() cmd_arc.run() @@ -223,7 +225,8 @@ def test_run_cp_copy(self): filters = [['--include', '*']] params = {'dir_op': False, 'dryrun': True, 'quiet': False, 'src': s3_file, 'dest': s3_file, 'filters': filters, - 'paths_type': 's3s3', 'region': 'us-east-1'} + 'paths_type': 's3s3', 'region': 'us-east-1', + 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'cp', params) cmd_arc.create_instructions() cmd_arc.run() @@ -238,7 +241,8 @@ def test_run_mv(self): filters = [['--include', '*']] params = {'dir_op': False, 'dryrun': True, 'quiet': False, 'src': s3_file, 'dest': s3_file, 'filters': filters, - 'paths_type': 's3s3', 'region': 'us-east-1'} + 'paths_type': 's3s3', 'region': 'us-east-1', + 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'mv', params) cmd_arc.create_instructions() cmd_arc.run() @@ -253,7 +257,8 @@ def test_run_remove(self): filters = [['--include', '*']] params = {'dir_op': False, 'dryrun': True, 'quiet': False, 'src': s3_file, 'dest': s3_file, 'filters': filters, - 'paths_type': 's3', 'region': 'us-east-1'} + 'paths_type': 's3', 'region': 'us-east-1', + 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'rm', params) cmd_arc.create_instructions() cmd_arc.run() @@ -272,7 +277,8 @@ def test_run_sync(self): filters = [['--include', '*']] params = {'dir_op': True, 'dryrun': True, 'quiet': False, 'src': local_dir, 'dest': s3_prefix, 'filters': filters, - 'paths_type': 'locals3', 'region': 'us-east-1'} + 'paths_type': 'locals3', 'region': 'us-east-1', + 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'sync', params) cmd_arc.create_instructions() cmd_arc.run() @@ -286,7 +292,7 @@ def test_run_mb(self): s3_prefix = 's3://' + self.bucket + '/' params = {'dir_op': True, 'dryrun': True, 'quiet': False, 'src': s3_prefix, 'dest': s3_prefix, 'paths_type': 's3', - 'region': 'us-east-1'} + 'region': 'us-east-1', 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'mb', params) cmd_arc.create_instructions() cmd_arc.run() @@ -300,7 +306,7 @@ def test_run_rb(self): s3_prefix = 's3://' + self.bucket + '/' params = {'dir_op': True, 'dryrun': True, 'quiet': False, 'src': s3_prefix, 'dest': s3_prefix, 'paths_type': 's3', - 'region': 'us-east-1'} + 'region': 'us-east-1', 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'rb', params) cmd_arc.create_instructions() rc = cmd_arc.run() @@ -315,7 +321,7 @@ def test_run_rb_nonzero_rc(self): s3_prefix = 's3://' + self.bucket + '/' params = {'dir_op': True, 'dryrun': False, 'quiet': False, 'src': s3_prefix, 'dest': s3_prefix, 'paths_type': 's3', - 'region': 'us-east-1'} + 'region': 'us-east-1', 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'rb', params) cmd_arc.create_instructions() rc = cmd_arc.run() From 04d386a887c2d64864b09f1028d04878ca25f666 Mon Sep 17 00:00:00 2001 From: Mathieu Guillaume Date: Fri, 8 Nov 2013 18:51:32 +0100 Subject: [PATCH 3/3] Add unit test to ensure endpoint-url is passed to botocore for s3 --- tests/unit/test_clidriver.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/unit/test_clidriver.py b/tests/unit/test_clidriver.py index 0e0463ef730b..756855fa968a 100644 --- a/tests/unit/test_clidriver.py +++ b/tests/unit/test_clidriver.py @@ -330,6 +330,17 @@ def test_aws_with_region(self): endpoint.assert_called_with(region_name='us-east-1', endpoint_url=None) + def test_s3_with_region_and_endpoint_url(self): + with mock.patch('botocore.service.Service.get_endpoint') as endpoint: + http_response = models.Response() + http_response.status_code = 200 + endpoint.return_value.make_request.return_value = ( + http_response, {}) + self.assert_params_for_cmd( + 's3 ls s3://test --region us-east-1 --endpoint-url https://foobar.com/', + expected_rc=0) + endpoint.assert_called_with(region_name='us-east-1', + endpoint_url='https://foobar.com/') def inject_new_param(self, argument_table, **kwargs): argument = CustomArgument('unknown-arg', {})