Fix the bug that can not remove the file on aws

[Tideorz]

This fix is used to fix the Issue #123 , which I found that you can't delete the file when using delete api. As the remove() without decorator @return_future, and it will never be executed. My test environment: Python 2.7.13

You can easily test by using localstack.

pip install localstack thumbor==6.1.5 botocore==1.5.70 tc-aws==6.2.10
export SERVICES=s3
export DEBUG=1
localstack start

create a 'test' bucket on localstack.

import boto3
s3 = boto3.client('s3', endpoint_url='http://localhost:4572', use_ssl=False)
s3.create_bucket(Bucket='test')

change thumbor.conf using localstack s3. TC_AWS_ENDPOINT='http://localhost:4572'

Upload and image to 'test' bucket and then delete.

[Bladrak]

Hello and thanks for the contribution! Seems good so far, could you update the test https://github.com/thumbor-community/aws/blob/master/vows/storage_vows.py#L92 accordingly? Thanks again!

[Tideorz]

hey @Bladrak, I've do some changes to CanRemoveImage, please take a look :)

[Tideorz]

hi @Bladrak , I'm tring to fix the broken test cases. After checking broken console log in the former commit. I did some changes in the latest commit. Add botocore==1.5.70 into install_requires. moto==1.3.3 to extras_require.

The reason why I do this, is because some broken tese cases shows as follow:

Raises if invalid config
        ✗ should be an error
            ("Expected topic('Endpoint' object has no attribute 'verify') to be an error of type <type 'exceptions.RuntimeError'>, but it was a <type 'exceptions.AttributeError'>", AttributeError("'Endpoint' object has no attribute 'verify'",), <type 'exceptions.RuntimeError'>, <type 'exceptions.AttributeError'>)

Traceback (most recent call last):
              File "/usr/local/lib/python2.7/site-packages/pyvows/runner/abc.py", line 65, in run_vow
    result = vow(ctx_obj, topic)
              File "/root/project/vows/storage_vows.py", line 190, in should_be_an_error
    expect(topic).to_be_an_error_like(RuntimeError)
              File "/usr/local/lib/python2.7/site-packages/preggy/core.py", line 285, in _assert_topic
    return _registered_assertions[method_name](self.topic, *args, **kw)
              File "/usr/local/lib/python2.7/site-packages/preggy/core.py", line 58, in wrapper
    func(*args, **kw)
              File "/usr/local/lib/python2.7/site-packages/preggy/assertions/types/errors.py", line 25, in to_be_an_error_like
    raise err
AssertionError: ("Expected topic('Endpoint' object has no attribute 'verify') to be an error of type <type 'exceptions.RuntimeError'>, but it was a <type 'exceptions.AttributeError'>", AttributeError("'Endpoint' object has no attribute 'verify'",), <type 'exceptions.RuntimeError'>, <type 'exceptions.AttributeError'>)

And i checked the tornado_botocore code. https://github.com/nanvel/tornado-botocore/blob/master/tornado_botocore/base.py#L105 It will raise Exception('Endpoint' object has no attribute 'verify'.) when try to log debug. After stepping into code, I found tornado_botocore will finally use botocore's Endpoint class, https://github.com/boto/botocore/blob/develop/botocore/endpoint.py#L73 And you can see that the Endpoint class doesn't have the verify() method. which makes test broken. But in botocore==1.5.70, Endpoint class has verify(). https://pypi.org/project/botocore/1.5.70/ So I changed the botocore version to 1.5.70, which is the latest version which supports verify().

And for the moto==1.3.3, I found that in my local environment, after use the latest moto-1.3.5, it will shows

pyvows -c -l tc_aws
Traceback (most recent call last):
  File "/data/home/china/tzhu/local/image-service/environ/bin/pyvows", line 11, in <module>
    sys.exit(main())
  File "/data/home/china/tzhu/local/image-service/environ/lib/python2.7/site-packages/pyvows/cli.py", line 191, in main
    capture_output=arguments.capture_output
  File "/data/home/china/tzhu/local/image-service/environ/lib/python2.7/site-packages/pyvows/cli.py", line 143, in run
    Vows.collect(path, pattern)
  File "/data/home/china/tzhu/local/image-service/environ/lib/python2.7/site-packages/pyvows/core.py", line 166, in collect
    __import__(module_name)
  File "/data/home/china/tzhu/merge/aws/vows/storage_vows.py", line 14, in <module>
    from moto import mock_s3_deprecated as mock_s3
  File "/data/home/china/tzhu/local/image-service/environ/lib/python2.7/site-packages/moto/__init__.py", line 38, in <module>
    from .secretsmanager import mock_secretsmanager  # flake8: noqa
  File "/data/home/china/tzhu/local/image-service/environ/lib/python2.7/site-packages/moto/secretsmanager/__init__.py", line 5, in <module>
    secretsmanager_backend = secretsmanager_backends['us-east-1']
KeyError: u'us-east-1'
make: *** [test] Error 1

On Stackoverflow, someone suggested that upgrades you botocore's version to latest. But I can't, so i try this version moto-1.3.3, which can work with the botocore==1.5.70.

Now all the test cases passed. Not sure whether there is a better solution. If you have good idea, tell me, Thanks :)

[Bladrak]

Ok, for the first issue, I think we should rather submit a PR on tornado_botocore to update the code to remove the verify call (given it's only for the logger). For the second, it might be related with your local configuration of S3 (maybe you're using a different region than the default)?

[roxylu]

Hi @Bladrak,

I am afraid the following error is not related to region settings:

Traceback (most recent call last):
  File "/data/home/china/tzhu/local/image-service/environ/bin/pyvows", line 11, in <module>
    sys.exit(main())
  File "/data/home/china/tzhu/local/image-service/environ/lib/python2.7/site-packages/pyvows/cli.py", line 191, in main
    capture_output=arguments.capture_output
  File "/data/home/china/tzhu/local/image-service/environ/lib/python2.7/site-packages/pyvows/cli.py", line 143, in run
    Vows.collect(path, pattern)
  File "/data/home/china/tzhu/local/image-service/environ/lib/python2.7/site-packages/pyvows/core.py", line 166, in collect
    __import__(module_name)
  File "/data/home/china/tzhu/merge/aws/vows/storage_vows.py", line 14, in <module>
    from moto import mock_s3_deprecated as mock_s3
  File "/data/home/china/tzhu/local/image-service/environ/lib/python2.7/site-packages/moto/__init__.py", line 38, in <module>
    from .secretsmanager import mock_secretsmanager  # flake8: noqa
  File "/data/home/china/tzhu/local/image-service/environ/lib/python2.7/site-packages/moto/secretsmanager/__init__.py", line 5, in <module>
    secretsmanager_backend = secretsmanager_backends['us-east-1']
KeyError: u'us-east-1'

See issue here: https://github.com/spulec/moto/issues/1767

Cheers, Roxy

[Tideorz]

@Bladrak , I changed the botocore==1.5.70 to extras_requires, now this package installed only influences the CI environment. And I found that, comment the log.debug in tornado_botocore will still be failed in another method(), shows'URLLib3Session' object has no attribute 'get_adapter', seems it's not a simple issue. I'll raise an issue to tornado_botocore, and put the issue link here. But need write simple example for reproducing the bug, which will take some time, not sure when this will be fixed :( . Can you please help to merge this to master, and push to pypi, as my project need use this package recently.

Thanks :D

[Tideorz]

@Bladrak , I've change the setup.py back, Now the code should be safe to be merged. Test cases are broken is because of the compatible issue between latest tornado_botocore and botocore. Not only my branch, the master code also will get broken CI status. I've raised a Issue https://github.com/nanvel/tornado-botocore/issues/18 for this bug, Hope someone will help to fix it. Do you think everything is ok to merge to master ？ :p

[Bladrak]

Thanks @Tideorz I'll take a look at the tests. Won't be able to release it to pypi now though, I'll need to fix the tests first as the release is automated :)

[Bladrak]

@Tideorz I've set the versions constraints while we wait for the issue https://github.com/nanvel/tornado-botocore/issues/18 to be fixed (as you said, it seems tornado botocore is not compatible with the latest botocore versions, and moto require the most recent versions of botocore to work, so we'll have to wait before removing those constraints). If you can rebase your PR (and maybe squash it as well) to ensure your tests pass, we shall be able to merge it then :-)

[Tideorz]

hey @Bladrak , I've rebased my PR and squashed the commits. Now test passed. :-)

[Bladrak]

Thanks for the contribution @Tideorz :)