dennisv / django-storage-swift

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

Adds option for lazy connection to swift #97

Closed pomegranited closed 6 years ago

pomegranited commented 6 years ago

Instead of connecting to SWIFT on storage init, this adds an option to allow the SWIFT connection to be created only when required.

Django takes great care to initialise Storage classes lazily, and so connecting on creation is generally a valid option. However there are some cases where this is difficult to do, e.g., when using FileField.storage, the Storage instance is instantiated on model import time, which can result in unintended SWIFT authentication/connections.

Reviewer

codecov-io commented 6 years ago

Codecov Report

Merging #97 into master will increase coverage by 0.12%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #97      +/-   ##
==========================================
+ Coverage   97.77%   97.89%   +0.12%     
==========================================
  Files           1        1              
  Lines         225      238      +13     
==========================================
+ Hits          220      233      +13     
  Misses          5        5
Impacted Files Coverage Δ
swift/storage.py 97.89% <100%> (+0.12%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d6b52cf...83b3ba4. Read the comment docs.

smarnach commented 6 years ago

:+1: Looking good to me. I've read through the code and tested that it still works for our use case (an Open edX instance).

I'm wondering whether there is any disadvantage to connecting lazily, or whether lazy connections should be the default, but it's probably safer to keep the current behaviour.

pomegranited commented 6 years ago

Hi @dennisv ! Would you consider merging this PR? Please let me know if you need more information or tests added.

pomegranited commented 6 years ago

Thank you @dennisv !

dennisv commented 6 years ago

It's also pushed to PyPI: https://pypi.python.org/pypi/django-storage-swift/1.2.18

pomegranited commented 6 years ago

So quick -- awesome!