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

fix delete file scenario. Was missing normalization from path to abspath #141

Merged
merged 3 commits into from
Feb 24, 2020

Conversation

eitanshapiro
Copy link

@eitanshapiro eitanshapiro commented Jan 29, 2020

Thumbor report successful delete while in practice the delete fails.

Branch Master
Also release 6.2.12

Copy/paste the result of the pip freeze command here. We'll be most interested in the versions of

boto==2.49.0
boto3==1.11.6
botocore==1.14.6
thumbor==6.7.0
tornado==5.1.1
tornado-botocore==1.5.0
tornado-pyvows==0.6.1

Configuration

TC_AWS_STORAGE_BUCKET = 'vo-thumbor-bucket' # S3 bucket for Storage
TC_AWS_STORAGE_ROOT_PATH = 'testDeleteImage' # S3 path prefix for Storage bucket
TC_AWS_REGION = 'us-east-1'

Steps to reproduce

curl -i -H "Content-Type: image/png" -H "Slug: test.png" -XPOST http://localhost:8080/image --data-binary "@Avatar-85-256.png"

curl -i -XDELETE http://localhost:8080/image/1e98c320dd714290a33403e5c8e75b86/test.png

Delete is returning 204 but the file is not deleted in S3 :(

Adding this code to s3_storage.py solves the issue

@return_future
def remove(self, path, callback=None):
"""
Deletes data at path
:param string path: Path to delete
"""
super(Storage, self).remove(self._normalize_path(path), callback)

@eitanshapiro
Copy link
Author

Looking at #131 it actually fixes the same thing

@eitanshapiro
Copy link
Author

I think the tests fail because the other pull request is not in master - #139

@Bladrak
Copy link

Bladrak commented Feb 3, 2020

@eitanshapiro thanks for your submission! Could you add a test regarding the removing of a file, as well as rebase it?

@Bladrak
Copy link

Bladrak commented Feb 24, 2020

@kkopachev if you can take a look at this as well :)

@eitanshapiro
Copy link
Author

Adding TC_AWS_STORAGE_ROOT_PATH='nana' to the test is showing the issue that was fixed by calling normalization.

@kkopachev kkopachev merged commit bc10f51 into thumbor-community:master Feb 24, 2020
@MrLesh
Copy link

MrLesh commented Mar 15, 2020

Is there a target release for this pull request? we really need it in our project

@Bladrak
Copy link

Bladrak commented Mar 15, 2020

I'll release this next week, I'll let you know

@Bladrak
Copy link

Bladrak commented Mar 16, 2020

Hi, @MrLesh you should be able to use the latest release (6.2.15).

@kkopachev I've fixed the publishing process which needed to migrate to twine for some reason. If you need to release, you can simply create a release from GitHub with an unexisting version, and it should publish a new release to pypi.

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.

4 participants