Use core django for caching signed urls.#13
Conversation
Use the caching feature of django core to cache urls. Using this feature we can create multi-day urls and keep them cached in whatever caching backend is configured. May only help in certain use cases, but one of those use cases is mine :)
Just in case it's not throwing an exception.
|
Hey, thanks for the PR! I'm a bit busy because of Easter holidays, but I'll try to look into this soon!
This should also be included in the unit tests, especially for covering multiple Django versions. I should update those for recent Django versions anyway, now that I think about it.
Am 30. März 2018 12:11:33 MESZ schrieb Matt Clark <notifications@github.com>:
…Use the caching feature of django core to cache urls. Using this
feature we can create multi-day urls and keep them cached in
whatever caching backend is configured. May only help in certain
use cases, but one of those use cases is mine :)
You can view, comment on, or merge this pull request online at:
#13
-- Commit Summary --
* Use core django for caching signed urls.
-- File Changes --
M django_gcloud_storage/__init__.py (43)
-- Patch Links --
https://github.com/Strayer/django-gcloud-storage/pull/13.patch
https://github.com/Strayer/django-gcloud-storage/pull/13.diff
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
#13
|
strayer
left a comment
There was a problem hiding this comment.
Hey @scattym,
sorry for taking so long… thanks again for the PR!
The code itself looks fine, but this really needs tests. I think there should be at least three tests:
test_should_cache_urls_when_enabled
Basically check wether a second call returns the identical URL. This may generate false positives if the gcloud API itself generates an identical URL, but I this would come up with the other test.
test_should_not_cache_urls_by_default
As before, but simply checking a second call returns a different URL.
test_should_not_return_cached_urls_when_cache_disabled
This is a little more complicated. This should test that a different URL is returned when caching is disabled but this cache_key does have an entry in the cache.
For reference: Changing settings within a test is already done here: https://github.com/Strayer/django-gcloud-storage/blob/db78650/tests/test_class.py#L256-L258
I don't think testing cache expiry and such is required, that should be handled by Djangos core tests already.
Use the caching feature of django core to cache urls. Using this
feature we can create multi-day urls and keep them cached in
whatever caching backend is configured. May only help in certain
use cases, but one of those use cases is mine :)