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.

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

Conversation

quiver
Copy link
Contributor

@quiver quiver commented Mar 21, 2014

When both --index-document and --error-document options are supplied,
--error-document argument was ignored.

How to reproduce

$ aws s3 mb s3://b0e1.example.com
make_bucket: s3://b0e1.example.com/
$ aws s3 website s3://b0e1.example.com --index-document  test.html --error-document error.html
$ aws s3api get-bucket-website --bucket b0e1.example.com
{
    "RedirectAllRequestsTo": {},
    "IndexDocument": {
        "Suffix": "test.html"
    },
    "ErrorDocument": {}, # <- ignored!
    "RoutingRules": []
}

When both --index-document and --error-document options are supplied,
--error-document argument was ignored.
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

Subprocess expectes str() not bytes() and will try to process
the arg as a list if it receives bytes().
* issue-718:
  Add integ tests for aws#718
  Use unicode escape sequence for file type
@jamesls
Copy link
Member

jamesls commented Mar 28, 2014

Upon further debugging it looks like this is because we're using the same bucket for this test, and the second command that adds the error document takes a while to propogate. If I had a time.sleep(2) between the first and second command, the test always passes for me.

Another option is to just split this into two separate tests.

@quiver
Copy link
Contributor Author

quiver commented Mar 29, 2014

I had a time.sleep(2) between the first and second command, the test always passes for me.

It didn't work for me.

Another option is to just split this into two separate tests.

Deleting the original --index-document only test part or separating into two tests didn't work for me.

For me, the root cause seems to be integration tests' module search path.

When both --index-document and --error-document options are supplied,
--error-document argument was ignored.
@quiver
Copy link
Contributor Author

quiver commented Mar 29, 2014

Sorry, my fault. Even solving search path oddity, I still got key error.

I had a time.sleep(2) between the first and second command, the test always passes for me.
Another option is to just split this into two separate tests.

Both options fixed key error. It looks like reusing botocore's session is causing trouble. Following @jamesls advice, I modified the original test to two separate tests.

@jamesls jamesls merged commit 1be05d2 into aws:develop Apr 1, 2014
@jamesls
Copy link
Member

jamesls commented Apr 1, 2014

Thanks!

@quiver quiver deleted the fix-s3-get-bucket-website branch November 14, 2014 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants