Merge lp:~canonical-ca-hackers/ubuntu-webcatalog/fix-scroll-backwards-error into lp:ubuntu-webcatalog

[Łukasz Czyżykowski]

Fix 'cursor can only scan forward' error on cleanup tasks.

Overview
========
This branch fixes error with cleanup tasks.

Details
=======
If the remove code would run longer than specified (or default) batch_time the calculation of batch_size would turn it to 0 or -1, which supplied to SQL FETCH would result in an error.

The simplest fix is to just set batch_size to 1 in that case. Documentation of PostgreSQL states that in the event that the cursor was exhausted it will just return 0 rows and not error.

[Anthony Lenton (elachuni)]

review: Approve

=== modified file 'src/webcatalog/management/commands/cleanup.py'
--- src/webcatalog/management/commands/cleanup.py	2012-07-02 13:51:23 +0000
+++ src/webcatalog/management/commands/cleanup.py	2012-09-11 14:43:24 +0000
@@ -131,7 +131,7 @@
                    "Supported tables are: %s" % (table, CLEANERS.keys()))
             raise CommandError(msg)
         cleaner = cleaner_cls(self)
-        cur = connection.cursor()
+        cursor = connection.cursor()
 
         removed = -1
         total_removed = 0
@@ -142,13 +142,14 @@
             # We don't want to hold the cursor open too long to allow
             # vacuum to reclaim rows. 1 hour default before it is reopened.
             if time.time() - cursor_opened_time > cursor_secs:
-                cleaner.declare_expired_keys_cursor(cur)
+                cleaner.declare_expired_keys_cursor(cursor)
                 cursor_opened_time = time.time()
 
             # Remove a batch of session rows.
             batch_start = time.time()
-            removed = cleaner.remove_batch(cur, batch_size)
+            removed = cleaner.remove_batch(cursor, batch_size)
             actual_batch_time = time.time() - batch_start
+
             total_removed += removed
             message = "Removed %d rows (%d total removed). Batch size %d\n"
             self.output(message % (removed, total_removed, batch_size), 1)

=== modified file 'src/webcatalog/tasks.py'
--- src/webcatalog/tasks.py	2012-06-29 19:51:06 +0000
+++ src/webcatalog/tasks.py	2012-09-11 14:43:24 +0000
@@ -1,7 +1,6 @@
 from time import sleep
 
 from celery.task import task
-from django.conf import settings
 from django.core.management import call_command
 
 

=== modified file 'src/webcatalog/tests/test_commands.py'
--- src/webcatalog/tests/test_commands.py	2012-08-29 21:46:54 +0000
+++ src/webcatalog/tests/test_commands.py	2012-09-11 14:43:24 +0000
@@ -765,7 +765,7 @@
         # relevant stats.
         onion = self.factory.make_distroseries(code_name='onion')
         orig_timestamp = datetime.utcnow() - timedelta(2)
-        import_record = ReviewStatsImport.objects.create(
+        ReviewStatsImport.objects.create(
             distroseries=onion, last_import=orig_timestamp)
 
         call_command('import_ratings_stats', 'onion')
@@ -1078,10 +1078,7 @@
 
     @skipUnless(connection.vendor == 'postgresql', "Requires postgresql")
     def test_cleanup_sessions(self):
-        now = datetime.now()
-        for i in range(8):
-            expire_date = now + timedelta(seconds=5000 * (4 - i))
-            self.factory.make_session(expire_date=expire_date)
+        self.create_django_sessions()
         self.assertEqual(8, Session.objects.count())
 
         call_command('cleanup', 'django_session')
@@ -1089,6 +1086,20 @@
         self.assertEqual(4, Session.objects.count())
         self.check_expected_output()
 
+    def create_django_sessions(self):
+        now = datetime.now()
+        for i in range(8):
+            expire_date = now + timedelta(seconds=5000 * (4 - i))
+            self.factory.make_session(expire_date=expire_date)
+
+    @skipUnless(connection.vendor == 'postgresql', "Requires postgresql")
+    def test_cleanup_session_when_it_takes_too_long(self):
+        self.create_django_sessions()
+
+        call_command('cleanup', 'django_session', batch_secs=0)
+
+        self.assertEqual(0, Session.objects.count())
+
 
 class ImportAllAppInstallDataTestCase(TestCaseWithFactory):
     @patch('sys.stdout')