dennisv / django-storage-swift

OpenStack Swift storage backend for Django
MIT License
86 stars 60 forks source link

PUT object with Content-Length header. #73

Closed valerytschopp closed 7 years ago

valerytschopp commented 7 years ago

This is required by some Swift object storage implementation, like Ceph RADOS Gateway

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.03%) to 92.105% when pulling aa2f026c147e8f4c7b81024eff75f678245324a0 on valerytschopp:put_object_content_length into 169d41a7890d9628aae57c7e0cdbec76c660f0c1 on blacktorn:master.

einarf commented 7 years ago

👍 for tests.

Looks perfectly fine to me, but I don't know too much about what content can possibly be. Will it always be something that can be considered an fd?

dennisv commented 7 years ago

Looking at the source of swiftclient it can be other things as well: https://github.com/openstack/python-swiftclient/blob/70c90b2243c2df9857a188cbd61d340b7e191d0d/swiftclient/client.py#L1228-L1230

    :param contents: a string, a file-like object or an iterable
                     to read object data from;
                     if None, a zero-byte put will be done
einarf commented 7 years ago

https://docs.djangoproject.com/en/1.9/_modules/django/core/files/storage/ "The content should be a proper File object or any python file-like object"

It seems like the Django File class is used that does a lot of magic.

@blacktorn What matters is probably more what the the Django Storage supports and not Swift in itself.

Maybe we should use File or ContentFile in tests from django.core.files passing those into save. If that passes I think we're fine. As long as people can stream upload without actually dealing with a file on a storage device we are good (I'm assuming the django wrappers handle this and implements an interface for streams and bytes to acts as an file)

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.03%) to 92.105% when pulling bdaf962b3884ed8a01cc37485a97fd42a1ecbcbe on valerytschopp:put_object_content_length into 169d41a7890d9628aae57c7e0cdbec76c660f0c1 on blacktorn:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.03%) to 92.105% when pulling bdaf962b3884ed8a01cc37485a97fd42a1ecbcbe on valerytschopp:put_object_content_length into 169d41a7890d9628aae57c7e0cdbec76c660f0c1 on blacktorn:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.03%) to 92.105% when pulling bdaf962b3884ed8a01cc37485a97fd42a1ecbcbe on valerytschopp:put_object_content_length into 169d41a7890d9628aae57c7e0cdbec76c660f0c1 on blacktorn:master.

valerytschopp commented 7 years ago

Pull request updated assuming we always get a Django File object (or a subclass) and use File.size to get the byte size of the object.

einarf commented 7 years ago

Perfect. I think we probably should expand test_save with different variants also calling the proper save method instead of _save, but that is probably a bit outside the scope of this pull request.

Looks ready for merge 👍

dennisv commented 7 years ago

Thanks @valerytschopp !