Adjust models and views for LA Metro Councilmatic upgrade

.travis.yml

@@ -12,10 +12,10 @@ env:
     - db_user=postgres
 
 addons:
-  postgresql: '9.4'
+  postgresql: '9.6'
   apt:
     packages:
-    - postgresql-9.4-postgis-2.3
+    - postgresql-9.6-postgis-2.3
 
 before_script:
   - psql -U postgres -c "create extension postgis"
@@ -25,8 +25,8 @@ install:
     - pip install -r tests/requirements.txt --upgrade
     - pip install -e .
 
-sudo: required 
-dist: trusty 
+sudo: required
+dist: trusty
 group: deprecated-2017Q4
 
 script:

README.rst

@@ -88,15 +88,6 @@ Run the management command to update the test fixture.
 Run the tests and commit your updated fixture with your PR!
 
 
-Team
-----
-
--  Forest Gregg, DataMade - Open Civic Data (OCD) and Legistar scraping
--  Cathy Deng, DataMade - data models and loading
--  Derek Eder, DataMade - front end
--  Eric van Zanten, DataMade - search and dev ops
-
-
 Patches and Contributions
 -------------------------
 We continue to improve django-councilmatic, and we welcome your ideas! You can make suggestions in the form of `github issues <https://github.com/datamade/django-councilmatic/issues>`_ (bug reports, feature requests, general questions), or you can submit a code contribution via a pull request.

councilmatic_core/haystack_indexes.py

@@ -65,3 +65,9 @@ def prepare_ocr_full_text(self, obj):
 
     def get_updated_field(self):
         return 'updated_at'
+
+    def prepare_last_action_date(self, obj):
+        # Solr seems to be fussy about the time format, and we do not need the time, just the date stamp.
+        # https://lucene.apache.org/solr/guide/7_5/working-with-dates.html#date-formatting
+        if obj.last_action_date:
+            return obj.last_action_date.date()

councilmatic_core/management/commands/convert_attachment_text.py

@@ -1,27 +1,24 @@
-import os
+import itertools
 import logging
 import logging.config
-import sqlalchemy as sa
+import os
 import requests
 import tempfile
-import itertools
+import tqdm
 
-from django.core.management.base import BaseCommand
 from django.conf import settings
+from django.core.management.base import BaseCommand
+from django.db import connection
 from django.db.models import Max, Q
 
-from opencivicdata.legislative.models import BillDocumentLink
-from councilmatic_core.models import BillDocument
+from opencivicdata.legislative.models import BillDocumentLink, BillDocument
 
+
+# Configure logging
 logging.config.dictConfig(settings.LOGGING)
 logging.getLogger("requests").setLevel(logging.WARNING)
 logger = logging.getLogger(__name__)
 
-DB_CONN = 'postgresql://{USER}:{PASSWORD}@{HOST}:{PORT}/{NAME}'
-
-engine = sa.create_engine(DB_CONN.format(**settings.DATABASES['default']),
-                          convert_unicode=True,
-                          server_side_cursors=True)
 
 class Command(BaseCommand):
     help = 'Converts bill attachments into plain text'
@@ -38,17 +35,23 @@ def handle(self, *args, **options):
         self.add_plain_text()
 
     def get_document_url(self):
-        # Only apply this query to most recently updated (or created) bill documents.
+        '''
+        By default, convert text for recently updated files, or files that
+        do not have attachment text. Otherwise, convert text for all files.
+        '''
         max_updated = BillDocument.objects.all().aggregate(max_updated_at=Max('bill__updated_at'))['max_updated_at']
 
-        is_null = Q(document__councilmatic_document__full_text__isnull=True)
         is_file = Q(url__iendswith='pdf') | Q(url__iendswith='docx') | Q(url__iendswith='docx')
-        after_max_update = Q(document__bill__updated_at__gt=max_updated)
+        is_null = Q(document__extras__full_text__isnull=True)
+        after_max_update = Q(document__bill__updated_at__gte=max_updated)
 
         if max_updated is None or self.update_all:
-            qs = BillDocumentLink.objects.filter(is_null & is_file)
+            qs = BillDocumentLink.objects.filter(is_file)
         else:
-            qs = BillDocumentLink.objects.filter(is_null & is_file & after_max_update)
+            # Always try to convert null files, because files may have failed
+            # in a reparable manner, e.g., Legistar server errors, during a
+            # previous conversion.
+            qs = BillDocumentLink.objects.filter(is_file & (after_max_update | is_null))
 
         for item in qs:
             yield item.url, item.document.id
@@ -58,15 +61,20 @@ def convert_document_to_plaintext(self):
         # installing it, import the library here.
         import textract
 
-        for document_data in self.get_document_url():
-            document_data = dict(document_data)
-            url = document_data['url']
-            document_id = document_data['id']
-            response = requests.get(url)
-            # Sometimes, Metro Legistar has a URL that retuns a bad status code (e.g., 404 from http://metro.legistar1.com/metro/attachments/95d5007e-720b-4cdd-9494-c800392b9265.pdf).
+        for url, document_id in tqdm.tqdm(self.get_document_url()):
+            try:
+                response = requests.get(url)
+            except (requests.exceptions.Timeout, requests.exceptions.ConnectionError):
+                # Don't fail due to server errors, as these tend to resolve themselves.
+                # https://requests.readthedocs.io/en/master/user/quickstart/#errors-and-exceptions
+                logger.warning('Document URL {} raised a server error - Could not get attachment text!'.format(url))
+                continue
+
+            # Sometimes, Metro Legistar has a URL that retuns a bad status code,
+            # e.g., 404 from http://metro.legistar1.com/metro/attachments/95d5007e-720b-4cdd-9494-c800392b9265.pdf.
             # Skip these documents.
             if response.status_code != 200:
-                logger.error('Document URL {} returns {} - Could not get attachment text!'.format(url, response.status_code))
+                logger.warning('Document URL {} returns {} - Could not get attachment text!'.format(url, response.status_code))
                 continue
 
             extension = os.path.splitext(url)[1]
@@ -77,27 +85,37 @@ def convert_document_to_plaintext(self):
                 try:
                     plain_text = textract.process(tfp.name)
                 except textract.exceptions.ShellError as e:
-                    logger.error('{} - Could not convert Councilmatic Document ID {}!'.format(e, document_id))
+                    logger.warning('{} - Could not convert Councilmatic Document ID {}!'.format(e, document_id))
+                    continue
+                except TypeError as e:
+                    if 'decode() argument 1 must be str, not None' in str(e):
+                        logger.warning('{} - Could not convert Councilmatic Document ID {}!'.format(e, document_id))
+                        continue
+                    else:
+                        raise
+                except UnicodeDecodeError as e:
+                    logger.warning('{} - Could not convert Councilmatic Document ID {}!'.format(e, document_id))
                     continue
 
                 logger.info('Councilmatic Document ID {} - conversion complete'.format(document_id))
 
-            yield {'plain_text': plain_text.decode('utf-8'), 'id': document_id}
+            yield (plain_text.decode('utf-8'), document_id)
 
     def add_plain_text(self):
         '''
-        Metro has over 2,000 attachments that should be converted into plain text.
-        When updating all documents with `--update_all`, this function insures that the database updates only 20 documents per connection (mainly, to avoid unexpected memory consumption).
-        It fetches up to 20 elements from a generator object, runs the UPDATE query, and then fetches up to 20 more.
-
-        Inspired by: https://stackoverflow.com/questions/30510593/how-can-i-use-server-side-cursors-with-django-and-psycopg2/41088159#41088159
-
-        More often, this script updates just a handful of documents: so, the incremental, fetch-just-20 approach may prove unnecessary. Possible refactor?
+        Metro has over 2,000 attachments that should be converted into plain
+        text. When updating all documents with `--update_all`, this function
+        ensures that the database updates only 20 documents per connection
+        (mainly, to avoid unexpected memory consumption). It fetches up to 20
+        elements from a generator object, runs the UPDATE query, and then
+        fetches up to 20 more.
+
+        Inspired by https://stackoverflow.com/questions/30510593/how-can-i-use-server-side-cursors-with-django-and-psycopg2/41088159#41088159
         '''
         update_statement = '''
-            UPDATE councilmatic_core_billdocument AS bill_docs
-            SET full_text = :plain_text
-            WHERE bill_docs.document_id = :id
+            UPDATE opencivicdata_billdocument AS bill_docs
+            SET extras = jsonb_set(extras, '{full_text}', to_jsonb(cast(%s as text)))
+            WHERE bill_docs.id = %s
         '''
 
         plaintexts = self.convert_document_to_plaintext()
@@ -109,7 +127,7 @@ def add_plain_text(self):
             if not plaintexts_fetched_from_generator:
                 break
             else:
-                with engine.begin() as connection:
-                    connection.execute(sa.text(update_statement), plaintexts_fetched_from_generator)
+                with connection.cursor() as cursor:
+                    cursor.executemany(update_statement, plaintexts_fetched_from_generator)
 
         logger.info('SUCCESS')

councilmatic_core/migrations/0049_auto_20191114_1142.py

@@ -0,0 +1,37 @@
+# Generated by Django 2.1.14 on 2019-11-14 19:42
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('councilmatic_core', '0048_post_shape'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='membership',
+            options={'base_manager_name': 'objects'},
+        ),
+        migrations.AlterField(
+            model_name='bill',
+            name='slug',
+            field=models.SlugField(unique=True),
+        ),
+        migrations.AlterField(
+            model_name='event',
+            name='slug',
+            field=models.SlugField(max_length=200, unique=True),
+        ),
+        migrations.AlterField(
+            model_name='organization',
+            name='slug',
+            field=models.SlugField(max_length=200, unique=True),
+        ),
+        migrations.AlterField(
+            model_name='person',
+            name='slug',
+            field=models.SlugField(unique=True),
+        ),
+    ]

councilmatic_core/migrations/0050_remove_billdocument.py

@@ -0,0 +1,16 @@
+# Generated by Django 2.2.9 on 2020-01-17 21:30
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('councilmatic_core', '0049_auto_20191114_1142'),
+    ]
+
+    operations = [
+        migrations.DeleteModel(
+            name='BillDocument',
+        ),
+    ]

councilmatic_core/migrations/0051_bill_last_action_date.py

@@ -0,0 +1,18 @@
+# Generated by Django 2.2.9 on 2020-01-30 19:03
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('councilmatic_core', '0050_remove_billdocument'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='bill',
+            name='last_action_date',
+            field=models.DateTimeField(blank=True, null=True),
+        ),
+    ]