Pull requests

#17 Merged
Repository
tobias.mcnulty
Branch
default
Repository
david
Branch
default

Include support for S3's server-side encryption

Author
  1. Tobias McNulty
Reviewers
Description

Boto now supports S3's server-side encryption. This pull request adds support for passing the encrypt_key argument to set_contents_from_file() in _save()

  • Learn about pull requests

Comments (14)

  1. Rich Leland

    Thanks Tobias - I'll take a look at this on Friday. I think this brings up a minor issue w/django-storages and that's the fact that we don't have a solid way to declare requirements. I've messaged David about the possibility of namespacing. We could also just have a few requirements files - one for each backend. But I feel like a namespaced setup would allow us to specify requirements individually and then users would get new requirements when updating django-storages as well. Have any thoughts?

    1. Tobias McNulty author

      Sorry I didn't see this till just now. I'm not sure how namespacing would look, but my thoughts mostly just that each project will have its own way of installing requirements (and will want to list the needed requirements for django-storages manually rather than relying on it to install them), so we shouldn't make too many assumptions. I think simply documenting the dependency (a comment in s3boto.py and a line in the documentation page for the backend) would be plenty for nearly all purposes. Thanks - looking forward to your thoughts on the code!

    2. Donald Stufft

      For requirements you could use setuptools extras.

      extras_require={
              "s3boto": ["boto"],
          },
      

      Then people can select them by doing pip install django-storages[s3boto]

  2. Ian Lewis

    If you could make it so that the encrypt_key keyword argument was only passed to set_contents_from_file() if encryption was enabled, then we could easily merge this pull request. Can you make those changes and update the pull request?

    1. Tobias McNulty author

      Happy to do that, but what's the reason exactly? It seems like it would be better not to rely on the default value of encrypt_key being False, but instead just pass in that value no matter what. Am I missing something?

  3. machineghost

    Is there any chance of this pull request being accepted anytime soon? I'd really like to get encryption on my S3 File Fields without having to rewrite my own storage engine ...

  4. machineghost

    My company has "bet heavily" on Boto as our sole means of interacting with S3, and at the same time we're facing a compliance deadline that requires us to encrypt our files at rest. If this pull request isn't accepted soon our only options will be to hand-patch our django-storages code or worse abandon it entirely.

    Please don't make us do that! Does django-storages have any kind of bounty system or any other way for a 3rd party like us to incentivize this pull request?

  5. David Larlet repo owner

    Thanks for all your contributions and sorry for the delay of the merge (1 year, ouch!). I plan to release a new version of the lib once you provide some feedback on the merge.