Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

s3 website ignore --error-document argument #714

Merged
merged 8 commits into from
Apr 1, 2014
Merged

s3 website ignore --error-document argument #714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
57632c2
Update s3 website to honor --error-document argument
quiver Mar 21, 2014
03879d4
Bug fix for integ test on windows wiht py3
jamesls Mar 27, 2014
d158437
Merge branch 'issue-718' into develop
jamesls Mar 27, 2014
8821f67
Add s3 whitespace issues to changelog
jamesls Mar 27, 2014
7c13102
Update changelog with S3 endpoint fix
jamesls Mar 28, 2014
edc2290
Update s3 website to honor --error-document argument
quiver Mar 21, 2014
fbd4ee7
Merge branch 'fix-s3-get-bucket-website' of github.com:quiver/aws-cli…
quiver Mar 29, 2014
1be05d2
test each command in separate test case
quiver Mar 29, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,15 @@ CHANGELOG
Next Release (TBD)
==================

* bugfix:``aws s3``: Fix issue with fips-us-gov-west-1 endpoint
(`issue botocore 265 (https://github.com/boto/botocore/pull/265)`__)
* bugfix:Table Output: Fix issue when displaying unicode
characters in table output
(`issue 721 <https://github.com/aws/aws-cli/pull/721>`__)
* bugfix:``aws s3``: Fix regression when syncing files with
whitespace
(`issue 706 <https://github.com/aws/aws-cli/issues/706>`__,
`issue 718 <https://github.com/aws/aws-cli/issues/718>`__)


1.3.4
Expand Down
2 changes: 1 addition & 1 deletion awscli/customizations/s3/s3.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ def _build_website_configuration(self, parsed_args):
website_config = {}
if parsed_args.index_document is not None:
website_config['IndexDocument'] = {'Suffix': parsed_args.index_document}
elif parsed_args.error_document is not None:
if parsed_args.error_document is not None:
website_config['ErrorDocument'] = {'Key': parsed_args.error_document}
return website_config

Expand Down
2 changes: 1 addition & 1 deletion tests/integration/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def aws(command, collect_memory=False, env_vars=None,
aws_command = 'python %s' % AWS_CMD
full_command = '%s %s' % (aws_command, command)
stdout_encoding = _get_stdout_encoding()
if isinstance(full_command, six.text_type):
if isinstance(full_command, six.text_type) and not six.PY3:
full_command = full_command.encode(stdout_encoding)
LOG.debug("Running command: %s", full_command)
env = os.environ.copy()
Expand Down
22 changes: 21 additions & 1 deletion tests/integration/customizations/s3/test_plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,8 +718,9 @@ def test_transfer_single_large_file(self):


class TestWebsiteConfiguration(BaseS3CLICommand):
def test_create_website_configuration(self):
def test_create_website_index_configuration(self):
bucket_name = self.create_bucket()
# Supply only --index-document argument.
full_command = 's3 website %s --index-document index.html' % (bucket_name)
p = aws(full_command)
self.assertEqual(p.rc, 0)
Expand All @@ -729,6 +730,25 @@ def test_create_website_configuration(self):
parsed = operation.call(
self.endpoint, bucket=bucket_name)[1]
self.assertEqual(parsed['IndexDocument']['Suffix'], 'index.html')
self.assertEqual(parsed['ErrorDocument'], {})
self.assertEqual(parsed['RoutingRules'], [])
self.assertEqual(parsed['RedirectAllRequestsTo'], {})

def test_create_website_index_and_error_configuration(self):
bucket_name = self.create_bucket()
# Supply both --index-document and --error-document arguments.
p = aws('s3 website %s --index-document index.html '
'--error-document error.html' % bucket_name)
self.assertEqual(p.rc, 0)
self.assert_no_errors(p)
# Verify we have a bucket website configured.
operation = self.service.get_operation('GetBucketWebsite')
parsed = operation.call(
self.endpoint, bucket=bucket_name)[1]
self.assertEqual(parsed['IndexDocument']['Suffix'], 'index.html')
self.assertEqual(parsed['ErrorDocument']['Key'], 'error.html')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing an error on this line. Do you see this as well?

======================================================================
ERROR: test_create_website_configuration (tests.integration.customizations.s3.test_plugin.TestWebsiteConfiguration)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "aws-cli/tests/integration/customizations/s3/test_plugin.py", line 746, in test_create_website_configuration
    self.assertEqual(parsed['ErrorDocument']['Key'], 'error.html')
KeyError: 'Key'
-------------------- >> begin captured logging << --------------------

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got the same error in a newly created environment and I think I found the culprit.

The problem stems from how integration test is executed.

As you can see in tests/integration/__init__.py line #21-#24, integration tests are executed by running CWD/bin/aws command, and when they are run, module awscli is imported not from CWD/awscli but from system default module search path.

That's why new test case(added to demonstrate the current problem) fails.

Following hackish code forces integration tests to be executed using CWD/awscli module.

$ diff --git a/awscli/clidriver.py b/awscli/clidriver.py
index 6a7da73..3e20f96 100644
--- a/awscli/clidriver.py
+++ b/awscli/clidriver.py
@@ -535,3 +535,6 @@ class CLIOperationCaller(object):
             output = self._session.get_variable('output')
         formatter = get_formatter(output, args)
         formatter(operation, response)
+
+if __name__ == '__main__':
+    main()
diff --git a/tests/integration/__init__.py b/tests/integration/__init__.py
index fdde492..d8cca93 100644
--- a/tests/integration/__init__.py
+++ b/tests/integration/__init__.py
@@ -82,6 +82,7 @@ def aws(command, collect_memory=False, env_vars=None,
         aws_command = os.environ['AWS_TEST_COMMAND']
     else:
         aws_command = 'python %s' % AWS_CMD
+        aws_command = 'python -m awscli.clidriver'
     full_command = '%s %s' % (aws_command, command)
     stdout_encoding = _get_stdout_encoding()
     if isinstance(full_command, six.text_type) and not six.PY3:

With this patch, I can run tests without error:

$ nosetests tests/integration/customizations/s3/test_plugin.py:TestWebsiteConfiguration
.
----------------------------------------------------------------------
Ran 1 test in 4.730s

OK

self.assertEqual(parsed['RoutingRules'], [])
self.assertEqual(parsed['RedirectAllRequestsTo'], {})


class TestIncludeExcludeFilters(BaseS3CLICommand):
Expand Down