No longer override Django's native timeout parameter handling #36
Conversation
|
The test failures are due to inconsistent negative timeout handling in pylibmc/libmemcached when using binary mode, for which I've filed lericson/pylibmc#202. These failures don't occur when running the Django test suite (from which the pylibmc tests are derived) since Django doesn't yet support binary mode (https://code.djangoproject.com/ticket/15815). I'd like to upstream much of this backend into Django, so I guess we'll need to figure out the best way to handle this at some point :-( |
https://docs.travis-ci.com/user/trusty-ci-environment In this new stack, Python 3.5 is not installed by default, so we set the global Python version to 3.5 to force this install. Tox will still use the appropriate version of Python for each test run within the job.
Django has since fixed the issue that prevented setting an infinite timeout, which can now be done using a timeout value of `None`. However their chosen solution is not consistent with the approach taken by django-pylibmc's custom `get_backend_timeout()`. Even though this is a non-backwards compatible change for people using a timeout value of `0` with django-pylibmc, the custom handling should still be removed, to: * simplify the django-pylibmc codebase * make django-pylibmc's behaviour consistent with the Django docs * avoid changes in behaviour when people switch from the native backend to django-pylibmc, expecting it to be a drop-in replacement. The adjusted test was copied from: https://github.com/django/django/blob/a6d463b0965bcdabc202450f00d2c09f944f8edb/tests/cache/tests.py#L522-L534 This change will need a major version bump of django-pylibmc. Fixes #35.
|
Did #42 handle this change, or are there additional fixes in this PR? |
|
Sorry for the delayed reply! #42 was just a subset of the removal, so this PR is still applicable. This PR was blocked on figuring out what to do with the test failures from negative timeouts. It turns out that with the binary protocol, negative timeouts are actually invalid, which I've had them clarify in the specification: Ideally libmemcached or pylibmc would raise a In the meantime I think django-pylibmc should just skip the negative timeout tests when using the binary protocol, and document that it's not a valid value. |
|
@edmorley are you interested in doing the rebase work to resolve the conflicts with the main branch? |
|
I was going to double back to this after the extra tests PR was merged :-) |
|
Hmm I missed PR #49. It's now merged, rebase away. |
|
I unfortunately won't have time to work on this any more, since we now use Redis/django-redis. |
Django has since fixed the issue that prevented setting an infinite timeout, which can now be done using a timeout value of
None. However their chosen solution is not consistent with the approach taken by django-pylibmc's customget_backend_timeout().Even though this is a non-backwards compatible change for people using a timeout value of
0with django-pylibmc, the custom handling should still be removed, to:to django-pylibmc, expecting it to be a drop-in replacement.
This change will need a major version bump of django-pylibmc.
Fixes #35.