Re: [Django] #10646: memcached's incr and decr should throw ValueError if keys don't exist

[Django]

#10646: memcached's incr and decr should throw ValueError if keys don't exist
+---
  Reporter:  dauerbaustelle | Owner:  anonymous 

Status:  assigned   | Milestone:  1.2   

 Component:  Cache system   |   Version:  1.1-beta-1

Resolution: |  Keywords:  memcached, cache, 
incr, decr, sprint200912
 Stage:  Ready for checkin  | Has_patch:  1 

Needs_docs:  0  |   Needs_tests:  0 

Needs_better_patch:  0  |  
+---
Changes (by jdunck):

  * keywords:  memcached, cache, incr, decr => memcached, cache, incr,
   decr, sprint200912

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

--

You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #10646: memcached's incr and decr should throw ValueError if keys don't exist

[Django]

#10646: memcached's incr and decr should throw ValueError if keys don't exist
+---
  Reporter:  dauerbaustelle | Owner:  anonymous 
  
Status:  assigned   | Milestone:  1.2   
  
 Component:  Cache system   |   Version:  1.1-beta-1
  
Resolution: |  Keywords:  memcached, cache, 
incr, decr
 Stage:  Ready for checkin  | Has_patch:  1 
  
Needs_docs:  0  |   Needs_tests:  0 
  
Needs_better_patch:  0  |  
+---
Changes (by jdunck):

  * needs_better_patch:  1 => 0
  * stage:  Accepted => Ready for checkin

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

--

You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #10646: memcached's incr and decr should throw ValueError if keys don't exist

[Django]

#10646: memcached's incr and decr should throw ValueError if keys don't exist
-+--
  Reporter:  dauerbaustelle  | Owner:  anonymous
   
Status:  assigned| Milestone:  1.2  
   
 Component:  Cache system|   Version:  1.1-beta-1   
   
Resolution:  |  Keywords:  memcached, cache, incr, 
decr
 Stage:  Accepted| Has_patch:  1
   
Needs_docs:  0   |   Needs_tests:  0
   
Needs_better_patch:  1   |  
-+--
Comment (by prestontimmons):

 The anonymous post was by me, by the way.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

--

You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #10646: memcached's incr and decr should throw ValueError if keys don't exist

[Django]

#10646: memcached's incr and decr should throw ValueError if keys don't exist
-+--
  Reporter:  dauerbaustelle  | Owner:  anonymous
   
Status:  assigned| Milestone:  1.2  
   
 Component:  Cache system|   Version:  1.1-beta-1   
   
Resolution:  |  Keywords:  memcached, cache, incr, 
decr
 Stage:  Accepted| Has_patch:  1
   
Needs_docs:  0   |   Needs_tests:  0
   
Needs_better_patch:  1   |  
-+--
Changes (by anonymous):

  * needs_docs:  1 => 0
  * needs_tests:  1 => 0

Comment:

 After looking at this ticket I think a little clarification would help to
 explain where it's at:

 1) Malcolm noted above the need to write a test case to catch this problem
 but the existing test cases for test_incr and test_decr already catch this
 problem. Running the tests with the memcached backend using cmemcache will
 cause these tests to fail.

 See here for where this is tested:
 
http://code.djangoproject.com/browser/django/trunk/tests/regressiontests/cache/tests.py#L183

 2) As for documentation, raising a ValueError is documented behavior for
 the incr/decr operation in the existing documentation and this bug
 violates this behavior. I don't think extra documentation is necessary.

 You can see the documentation here:
 http://docs.djangoproject.com/en/dev/topics/cache/#topics-cache

 3) Jacob's concern has been addressed about the incr/decr operation not
 being atomic and it is now implemented with try/except blocks after the
 incr/decr operation.

 4) After applying the patch I have verified that the tests pass for both
 cmemcache and python-memcache.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

--

You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #10646: memcached's incr and decr should throw ValueError if keys don't exist

[Django]

#10646: memcached's incr and decr should throw ValueError if keys don't exist
-+--
  Reporter:  dauerbaustelle  | Owner:  anonymous
   
Status:  assigned| Milestone:  1.2  
   
 Component:  Cache system|   Version:  1.1-beta-1   
   
Resolution:  |  Keywords:  memcached, cache, incr, 
decr
 Stage:  Accepted| Has_patch:  1
   
Needs_docs:  1   |   Needs_tests:  1
   
Needs_better_patch:  1   |  
-+--
Comment (by andrewfong):

 As of r11267, unit tests for the memcached backend were failing because
 the test expected a value error to be raised and the incr / decr did not
 do so. It was bugging me, so I made a quick fix (see attached patch). Hope
 it helps.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #10646: memcached's incr and decr should throw ValueError if keys don't exist

[Django]

#10646: memcached's incr and decr should throw ValueError if keys don't exist
-+--
  Reporter:  dauerbaustelle  | Owner:  anonymous
   
Status:  assigned| Milestone:  1.2  
   
 Component:  Cache system|   Version:  1.1-beta-1   
   
Resolution:  |  Keywords:  memcached, cache, incr, 
decr
 Stage:  Accepted| Has_patch:  1
   
Needs_docs:  1   |   Needs_tests:  1
   
Needs_better_patch:  1   |  
-+--
Changes (by jacob):

  * milestone:  1.1 => 1.2

Comment:

 This patch is bad: it defeats the whole purpose of an atomic, single-call
 incr/decr. The whole point is to make those operations screamingly fast,
 and doing a get first is just silly.

 I agree that we should be consistent -- `ValueError` is right -- but
 should be a try/except/error code check for the different backends.

 Also: punting to 1.2 since there's an easy workaround.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #10646: memcached's incr and decr should throw ValueError if keys don't exist

[Django]

#10646: memcached's incr and decr should throw ValueError if keys don't exist
-+--
  Reporter:  dauerbaustelle  | Owner:  anonymous
   
Status:  assigned| Milestone:  1.1  
   
 Component:  Cache system|   Version:  1.1-beta-1   
   
Resolution:  |  Keywords:  memcached, cache, incr, 
decr
 Stage:  Accepted| Has_patch:  1
   
Needs_docs:  1   |   Needs_tests:  1
   
Needs_better_patch:  1   |  
-+--
Changes (by mtredinnick):

  * needs_better_patch:  0 => 1
  * needs_docs:  0 => 1

Comment:

 Agree with making the behaviour uniform (right now, client code would have
 to handle every possible case in a reusable app, which is unnecessarily
 error-prone and adding this check can be done without breaking atomicity
 guarantees). I don't have a firm opinion at the moment whether we should
 do this with a pre-lookup key check, a la this patch, or a post-lookup
 handling of the error cases.

  1. This needs tests added to demonstrate the problem and so that we can
 verify the behaviour is going to be the same across *all* backends. You
 can't write a single test that tests all backends, but we can run the same
 test against different settings files. So write a test that increments and
 decrements non-existent values (there are existing cache tests in
 `tests/regressionstests/cache/`) and make sure that they behave the same
 for all current cache backends.
  2. Also sounds like it might be worth a documentation update to describe
 the behaviour here.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #10646: memcached's incr and decr should throw ValueError if keys don't exist

[Django]

#10646: memcached's incr and decr should throw ValueError if keys don't exist
-+--
  Reporter:  dauerbaustelle  | Owner:  anonymous
   
Status:  assigned| Milestone:  1.1  
   
 Component:  Cache system|   Version:  1.1-beta-1   
   
Resolution:  |  Keywords:  memcached, cache, incr, 
decr
 Stage:  Accepted| Has_patch:  1
   
Needs_docs:  0   |   Needs_tests:  1
   
Needs_better_patch:  0   |  
-+--
Comment (by Alex):

 I think we should try to use the results that are already returned,
 instead of making this operation slower(after all, it's purpose is to be
 fast and atomic).

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #10646: memcached's incr and decr should throw ValueError if keys don't exist

[Django]

#10646: memcached's incr and decr should throw ValueError if keys don't exist
-+--
  Reporter:  dauerbaustelle  | Owner:  anonymous
   
Status:  assigned| Milestone:  1.1  
   
 Component:  Cache system|   Version:  1.1-beta-1   
   
Resolution:  |  Keywords:  memcached, cache, incr, 
decr
 Stage:  Accepted| Has_patch:  1
   
Needs_docs:  0   |   Needs_tests:  1
   
Needs_better_patch:  0   |  
-+--
Changes (by russellm):

  * milestone:  => 1.1

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---

Re: [Django] #10646: memcached's incr and decr should throw ValueError if keys don't exist

[Django]

#10646: memcached's incr and decr should throw ValueError if keys don't exist
-+--
  Reporter:  dauerbaustelle  | Owner:  nobody   
   
Status:  new | Milestone:   
   
 Component:  Uncategorized   |   Version:  1.0  
   
Resolution:  |  Keywords:  memcached, cache, incr, 
decr
 Stage:  Unreviewed  | Has_patch:  1
   
Needs_docs:  0   |   Needs_tests:  0
   
Needs_better_patch:  0   |  
-+--
Changes (by dauerbaustelle):

  * needs_better_patch:  => 0
  * needs_tests:  => 0
  * needs_docs:  => 0

Comment:

 Aargh, some typos. (How can I update the previous post?)

 The `incr` and `decr` methods of the
 `django.core.cache.backends.memcached.CacheClass` should throw a
 `ValueError` if the `key` to increase/decrease does not exist.

 Right now, two things can happen trying to increase/decrease an invalid
 key, depending on the python memcached backend you're using: Using
 `cmemcache`, you simply get a `None` as return value. Using `memcache`,
 you'll get some weird `ValueError`s telling you that you cannot convert
 "NOT_FOUND" to an int (`memcache` tries to return the memcache server's
 return value as int).

 See the attached patch for a fix.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en
-~--~~~~--~~--~--~---