We can (now?) use value_to_string and avoid problems with custom fields. #7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Chris Lamb <[email protected]>
|
Please limit the first line of your commit message to 50 characters maximum. |
We can (now?) use value_to_string and avoid problems with custom fields.
Otherwise we instantiate entities with a string representation
It doesn't add any value here.
Use Pickle for encoding, which avoids the cache_toolbox encoding issues
At some point `request.session[SESSION_KEY]` started returning a `unicode` rather than an `int`. As a result this would always fail, and thus always fall back to the parent mechanism.
This means that when there is a cache miss, the database that is currently in
use will be used to fetch the data.
For example, if you have a database slave, model Foo with a cached relation
bar, which has a property baz:
random_foo = Foo.objects.order_by('?').using('slave').first()
# On cache misses, this will hit the database slave, as that is what was
# used to fetch baz.
baz = random_foo.bar_cache.baz
* https://github.com/thread/django-cache-toolbox: (23 commits) Specify using for relations Reflow Don't assume a numeric primary key Fix the authentication middleware Run all encoding through Pickle Add a version number on the cache keys Fix this test Add cache invalidation tests Mark as skipped Add a simple (currently failing)_cached relation test Add a first test Delete this documentation Trailing comma Exclude the "tests" directory Add `install_requires` Correct these This link is no longer live Special case some field types. Lookup the right model parent_model was a private API and removed in Django 1.8 ... # Conflicts: # cache_toolbox/core.py # setup.py
This appears to work fine in practice on 2.1.
The previous code worked, but wasn't actually showing what it intended to show.
This matches what Django does. Fixes #15
If cache.get manages to raise an unexpected exception, then previously our relation handling would end up raising an `UnboundLocalError` due to the use of `instance` in the `finally` clause. This changes that behaviour by duplicating the `setattr` call.
This should prevent potential issues which could otherwise arise from using the primary key of the current model in the cache key for the cached model. Fixes #22
I'm about to make some changes in get_instance, so having these separate will make it easier.
I'm about to need these in the core logic.
This will be particularly useful when applied to relations on the User model, which is loaded by our middleware. In that case, this allows for efficient loading of several models upfront. Fixes #21
It turns out that select_related() when called without arguments (as might happen if we didn't have anything in our related_names collection) will try to select related any relations which are non-nullable. Since we don't want that, guard the addition of the select_related to our queryset.
Django itself seems fine with these being dict_keys, however the django-perf-rec library isn't. While it's their bug, fixing here is easy enough.
The main thing this fixes is the case where the primary model is not returned in the cached results. However it also changes the handling in the case where one of the related models isn't found. Previously the former would error and the latter would silently assign `None` into the local relation cache field (and would thus never be populated even if there should have been a value there).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.