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.

Sign up for GitHub

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

Jump to bottom

Django 1.9, 1.10, and 1.11 upgrades #125

Merged
merged 27 commits into from Oct 13, 2017
Merged

Django 1.9, 1.10, and 1.11 upgrades #125

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
bfca2e2
add newer django+python to tox+travis
tobiasmcnulty Jun 3, 2016
004261f
convert assigned lambda funcs to def statements
tobiasmcnulty Jun 3, 2016
3cf88d7
Merge branch 'master' into tox-updates
tobiasmcnulty Jun 3, 2016
23b28c6
fix flake8 errors
tobiasmcnulty Jun 3, 2016
f12a36c
make travis use tox
tobiasmcnulty Jun 3, 2016
d6c527e
specify only one python since tox does that now
tobiasmcnulty Jun 3, 2016
aed2def
pass TRAVIS environment variable into tox environment
tobiasmcnulty Jun 3, 2016
615585b
don't run flake8 as part of travis, use tox
tobiasmcnulty Jun 4, 2016
dcc81ab
Update tox and travis
vkurup Oct 7, 2017
2668094
Include flake8 on travis
vkurup Oct 7, 2017
d450cff
Workaround Travis bug
vkurup Oct 7, 2017
77d21e8
Try to get coverage working
vkurup Oct 7, 2017
79281be
Remove support for Python < 2.7, Django < 1.8
vkurup Oct 7, 2017
36589e9
Remove obsolete package
vkurup Oct 7, 2017
f6839e4
Django 1.9 support
vkurup Oct 7, 2017
3b562d4
Django 1.10 support
vkurup Oct 7, 2017
b7295ff
Tell Travis to test 1.10
vkurup Oct 7, 2017
6a547d8
Django 1.11 support
vkurup Oct 7, 2017
7bd4f77
Cleanup and docs updates
vkurup Oct 7, 2017
24c9779
Add tests for failure when running .values() or .values_list()
vkurup Oct 8, 2017
f98e32f
Typo
vkurup Oct 9, 2017
4f1fb8e
Attempt to tease out Django 1.8 differences
vkurup Oct 11, 2017
085bf35
enable --keepdb for faster test runs
tobiasmcnulty Oct 12, 2017
52d4ea6
try to simplify Django 1.8/Django 1.11 compatibility
tobiasmcnulty Oct 12, 2017
c220d61
Merge pull request #1 from tobiasmcnulty/django-upgrades
vkurup Oct 13, 2017
096f2b2
Minor doc updates
vkurup Oct 13, 2017
251e5e6
Bump version for release
vkurup Oct 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
View file
Open in desktop
Expand Up @@ -26,7 +26,7 @@ env:
- TOX_ENV="dj18-py27,dj18-py34,dj18-py35"
- TOX_ENV="dj19-py27,dj19-py34,dj19-py35,dj19-py36"
- TOX_ENV="dj110-py27,dj110-py34,dj110-py35,dj110-py36"
# - TOX_ENV="dj111-py27,dj111-py34,dj111-py35,dj111-py36"
- TOX_ENV="dj111-py27,dj111-py34,dj111-py35,dj111-py36"
- TOX_ENV="py27-flake8,py36-flake8"
- TOX_ENV="docs"
# Adding sudo: False tells Travis to use their container-based infrastructure, which is somewhat faster.
Expand Down
2 changes: 1 addition & 1 deletion README.rst
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@ For full docs, see https://cache-machine.readthedocs.org/en/latest/.
Requirements
------------

Cache Machine works with Django 1.8-1.10 and Python 2.7, 3.4, 3.5 and 3.6.
Cache Machine works with Django 1.8-1.11 and Python 2.7, 3.4, 3.5 and 3.6.


Installation
Expand Down
12 changes: 12 additions & 0 deletions caching/base.py
View file
Open in desktop
Expand Up @@ -157,6 +157,18 @@ def __setstate__(self, state):
if self.timeout == self._default_timeout_pickle_key:
self.timeout = DEFAULT_TIMEOUT

def _fetch_all(self):
"""
Django 1.11 changed _fetch_all to use self._iterable_class() rather than
self.iterator(). That bypasses our iterator, so override Queryset._fetch_all
to use our iterator.

https://github.com/django/django/commit/f3b7c059367a4e82bbfc7e4f0d42b10975e79f0c#diff-5b0dda5eb9a242c15879dc9cd2121379
"""
if self._result_cache is None:
self._result_cache = list(self.iterator())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this change will cause a server-side cursor to be used for _fetch_all (if not globally disabled via DISABLE_SERVER_SIDE_CURSORS) on Django 1.11.1+ due two these lines:

I think we need to make sure _fetch_all() still uses self._iterable_class() directly rather than self.iterator() on Django 1.11+, perhaps via something along the lines of the following:

  • Separate our iterator() logic into a separate method, e.g., _iterate_on(iterable)
  • In _fetch_all(), call list(self._iterate_on(iter(self._iterable_class(self)))) for Django 1.11+ or list(self._iterate_on(self.iterator())) for older Djangos (_iterable_class unfortunately does not exist in Django 1.8).
  • In iterator(), simply return self._iterate_on(super(CachingQuerySet, self).iterator())

Does that seem crazy? What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or better yet, rather than check the version directly, use duck typing as you already did here: 24c9779#diff-76150a51cd21d8d6b30aaaab22c7f114R179

_fetch_all() and iterator() are unchanged between Django 1.9-1.10, so it should be safe to use self._iterable_class directly in those versions (in addition to Django 1.11)

_fetch_all():

iterator():

super(CachingQuerySet, self)._fetch_all()

def flush_key(self):
return flush_key(self.query_key())

Expand Down