Set up flake8 (#662)

* Fix Django files for flake8.

* A couple other style fixes for flake8.

* setup.cfg file for flake8 and pylint

* Replace yapf with flake8 in tools/lint.

* Include wsgi.py in lint checks.

* Use flake8 in all_tests and travis_tests.

* Install flake8, don't install yapf.

.travis.yml

@@ -35,14 +35,14 @@ install:
   - |
     pip install \
       cssselect \
+      flake8 \
       lxml \
       mock \
       modernize \
       parameterized \
       pillow==4.1.0 \
       pylint==1.9.4 \
-      pytest \
-      yapf
+      pytest
   - pip install -r requirements.txt -t app/vendors
   - npm --prefix ui install
 

app/tasksmodule/sitemap_ping.py

@@ -97,7 +97,7 @@ def _ping_indexer(self, search_engine):
         if response.status_code == 200:
             return True
         else:
-            #TODO(nworden): Retry or email konbit-personfinder on failure.
+            # TODO(nworden): Retry or email konbit-personfinder on failure.
             logging.error('Received %d pinging %s',
                           response.status_code, ping_url)
             return False

app/views/admin/acls.py

@@ -102,13 +102,13 @@ def post(self, request, *args, **kwargs):
             self.params.expiration_date, AdminAclsView._EXPIRATION_DATE_FORMAT)
         # TODO(nworden): add logging for this
         if (self.params.get('edit_button', '') or
-            self.params.get('revoke_button', '')):
+                self.params.get('revoke_button', '')):
             acl = admin_acls_model.AdminPermission.get(
                 self.env.repo, email_address)
             # You can't edit or revoke the permissions of someone at a higher
             # level.
             if self.env.user_admin_permission.compare_level_to(
-                acl.access_level) < 0:
+                    acl.access_level) < 0:
                 raise django.core.exceptions.PermissionDenied
             if self.params.get('edit_button', ''):
                 acl.access_level = level

app/views/admin/base.py

@@ -163,6 +163,7 @@ def _enforce_admin_level(user_admin_permission, min_level):
     if user_admin_permission.compare_level_to(min_level) < 0:
         raise django.core.exceptions.PermissionDenied
 
+
 def enforce_moderator_admin_level(func):
     """Require that the user be a moderator (or return a 403)."""
     def inner(self, *args, **kwargs):
@@ -173,6 +174,7 @@ def inner(self, *args, **kwargs):
         return func(self, *args, **kwargs)
     return inner
 
+
 def enforce_manager_admin_level(func):
     """Require that the user be a manager (or return a 403)."""
     def inner(self, *args, **kwargs):
@@ -183,6 +185,7 @@ def inner(self, *args, **kwargs):
         return func(self, *args, **kwargs)
     return inner
 
+
 def enforce_superadmin_admin_level(func):
     """Require that the user be a superadmin (or return a 403)."""
     def inner(self, *args, **kwargs):

app/views/admin/delete_record.py

@@ -55,8 +55,9 @@ def post(self, request, *args, **kwargs):
         del request, args, kwargs  # unused
         self.enforce_xsrf(self.ACTION_ID)
         action = ('delete', self.params.id)
-        path = '/delete?' + utils.urlencode(
-            {'id': self.params.id,
-             'signature': reveal.sign(action),})
+        path = '/delete?' + utils.urlencode({
+            'id': self.params.id,
+            'signature': reveal.sign(action),
+        })
         return django.shortcuts.redirect(
             self.build_absolute_uri(path, self.env.repo))

app/views/base.py

@@ -270,8 +270,8 @@ def render(self, template_name, status_code=200, **template_vars):
         def get_vars():
             """A function returning vars, for use by the resources module."""
             template_vars['env'] = self.env
-            # TODO(nworden): change templates to access config through env, which
-            # already has the config anyway
+            # TODO(nworden): change templates to access config through env,
+            # which already has the config anyway
             template_vars['config'] = self.env.config
             template_vars['params'] = self.params
             template_vars['csp_nonce'] = self.request.csp_nonce

setup.cfg

@@ -0,0 +1,5 @@
+[flake8]
+max-line-length=80
+
+[pylint]
+disable=fixme

tests/tasks/test_deletion.py

@@ -15,24 +15,16 @@
 
 import datetime
 import logging
-import os
 import sys
 
-import django
-import django.test
-from google.appengine import runtime
-from google.appengine.api import users
 from google.appengine.ext import db
-from google.appengine.api import quota
 from google.appengine.api import taskqueue
-from google.appengine.ext import testbed
 # We should be moving off mox and to mock. However, taskqueue doesn't play nice
 # with mock, so dropping mox for these tests means moving to GAE's stuff for
 # this. Since taskqueue has to get replaced by Cloud Tasks for Python 3 anyway,
 # it's not worth rewriting these tests now just to avoid mox.
 import mox
 
-import config
 import model
 import utils
 
@@ -136,7 +128,7 @@ def assert_past_due_count(expected):
         assert db.get(self.key_p1).source_date == datetime.datetime(2010, 2, 2)
         assert db.get(self.key_p1).entry_date == datetime.datetime(2010, 2, 2)
         assert db.get(self.key_p1).expiry_date == datetime.datetime(2010, 2, 1)
-        assert db.get(self.key_p1).is_expired == True
+        assert db.get(self.key_p1).is_expired is True
         assert model.Note.get('haiti', self.note_id) is None  # Note is hidden
         assert db.get(self.n1_1.key())  # but the Note entity still exists
         assert db.get(self.photo_key)
@@ -150,7 +142,7 @@ def assert_past_due_count(expected):
         assert db.get(self.key_p1).source_date == datetime.datetime(2010, 2, 2)
         assert db.get(self.key_p1).entry_date == datetime.datetime(2010, 2, 2)
         assert db.get(self.key_p1).expiry_date == datetime.datetime(2010, 2, 1)
-        assert db.get(self.key_p1).is_expired == True
+        assert db.get(self.key_p1).is_expired is True
         assert model.Note.get('haiti', self.note_id) is None  # Note is hidden
         assert db.get(self.n1_1.key())  # but the Note entity still exists
         assert db.get(self.photo_key)
@@ -165,7 +157,7 @@ def assert_past_due_count(expected):
         assert db.get(self.key_p1).source_date == datetime.datetime(2010, 2, 2)
         assert db.get(self.key_p1).entry_date == datetime.datetime(2010, 2, 2)
         assert db.get(self.key_p1).expiry_date == datetime.datetime(2010, 2, 1)
-        assert db.get(self.key_p1).is_expired == True
+        assert db.get(self.key_p1).is_expired is True
         assert db.get(self.key_p1).given_name is None
         assert model.Note.get('haiti', self.note_id) is None  # Note is hidden
         assert db.get(self.n1_1.key()) is None  # Note entity is actually gone
@@ -190,12 +182,12 @@ def assert_past_due_count(expected):
         # Confirm that the task wiped self.p2 as well.
         assert model.Person.all().count() == 0
         assert_past_due_count(2)
-        assert db.get(self.key_p1).is_expired == True
+        assert db.get(self.key_p1).is_expired is True
         assert db.get(self.key_p1).given_name is None
         assert db.get(self.key_p1).source_date == datetime.datetime(2010, 2, 2)
         assert db.get(self.key_p1).entry_date == datetime.datetime(2010, 2, 2)
         assert db.get(self.key_p1).expiry_date == datetime.datetime(2010, 2, 1)
-        assert db.get(self.key_p2).is_expired == True
+        assert db.get(self.key_p2).is_expired is True
         assert db.get(self.key_p2).given_name is None
         assert db.get(self.key_p2).source_date == datetime.datetime(2010, 3, 15)
         assert db.get(self.key_p2).entry_date == datetime.datetime(2010, 3, 15)

tests/tasks/test_sitemap_ping.py

@@ -36,8 +36,8 @@ def test_get(self):
         # pinging anything.
         config.set(ping_sitemap_indexers=True)
         with mock.patch('requests.get') as requests_mock:
-            res = self.run_task('/global/tasks/sitemap_ping',
-                                {'search_engine': 'google'})
+            self.run_task(
+                '/global/tasks/sitemap_ping', {'search_engine': 'google'})
             self.assertEqual(len(requests_mock.call_args_list), 1)
             call_args, _ = requests_mock.call_args_list[0]
             self.assertEqual(call_args[0],

tests/test_searcher.py

@@ -21,7 +21,8 @@ class SearcherTests(unittest.TestCase):
     def test_full_text_search_results(self):
         """Use full_text_search.search results when enabled."""
         with mock.patch('full_text_search.search') as full_text_search_mock:
-            full_text_search_mock.return_value = SearcherTests.FULLTEXT_RETURN_VALUE
+            full_text_search_mock.return_value = (
+                SearcherTests.FULLTEXT_RETURN_VALUE)
             searcher = Searcher(
                 SearcherTests.REPO_NAME,
                 enable_fulltext_search=True,
@@ -38,7 +39,8 @@ def test_full_text_results_with_loc(self):
         """Use full_text_search.search results when enabled, including location.
         """
         with mock.patch('full_text_search.search') as full_text_search_mock:
-            full_text_search_mock.return_value = SearcherTests.FULLTEXT_RETURN_VALUE
+            full_text_search_mock.return_value = (
+                SearcherTests.FULLTEXT_RETURN_VALUE)
             searcher = Searcher(
                 SearcherTests.REPO_NAME,
                 enable_fulltext_search=True,

tests/testutils/data_generator.py

@@ -110,7 +110,7 @@ def note(self, store=True, repo_id='haiti', person_id=None, **kwargs):
         return note
 
     def authorization(
-        self, store=True, repo_id='haiti', key='secret_key', **kwargs):
+            self, store=True, repo_id='haiti', key='secret_key', **kwargs):
         params = copy.deepcopy(TestDataGenerator._DEFAULT_AUTHORIZATION_PARAMS)
         params.update(kwargs)
         authorization = model.Authorization.create(repo_id, key, **params)
@@ -125,9 +125,9 @@ def photo(self, store=True, repo_id='haiti', image_data='xyz'):
         return photo
 
     def subscription(
-        self, store=True, repo_id='haiti', person_id=None,
-        email='fred@example.net', language='en',
-        timestamp=datetime.datetime(2010, 1, 2)):
+            self, store=True, repo_id='haiti', person_id=None,
+            email='fred@example.net', language='en',
+            timestamp=datetime.datetime(2010, 1, 2)):
         subscription = model.Subscription(
             repo=repo_id,
             person_record_id=person_id,
@@ -139,9 +139,9 @@ def subscription(
         return subscription
 
     def admin_permission(
-        self, store=True, repo_id='haiti', email_address='l@mib.gov',
-        access_level=admin_acls_model.AdminPermission.AccessLevel.MODERATOR,
-        expiration_date=datetime.datetime(2011, 1, 20)):
+            self, store=True, repo_id='haiti', email_address='l@mib.gov',
+            access_level=admin_acls_model.AdminPermission.AccessLevel.MODERATOR,
+            expiration_date=datetime.datetime(2011, 1, 20)):
         admin_permission = admin_acls_model.AdminPermission.create(
             repo_id, email_address, access_level, expiration_date)
         if store:

tests/views/test_admin_acls.py

@@ -15,12 +15,6 @@
 
 import datetime
 
-import django
-import django.http
-import django.test
-
-import config
-import model
 import modelmodule.admin_acls as admin_acls_model
 import utils
 
@@ -46,12 +40,12 @@ def test_get_with_none_editable(self):
         """
         self.login_as_manager()
         self.data_generator.admin_permission(
-            access_level=
-            admin_acls_model.AdminPermission.AccessLevel.SUPERADMIN)
+            access_level=(
+                admin_acls_model.AdminPermission.AccessLevel.SUPERADMIN))
         self.data_generator.admin_permission(
             email_address='m@mib.gov',
-            access_level=
-            admin_acls_model.AdminPermission.AccessLevel.SUPERADMIN)
+            access_level=(
+                admin_acls_model.AdminPermission.AccessLevel.SUPERADMIN))
         res = self.client.get('/haiti/admin/acls/', secure=True)
         self.assertEqual(len(res.context['editable_acls']), 0)
         self.assertEqual(len(res.context['fixed_acls']), 2)
@@ -62,8 +56,8 @@ def test_get_with_some_editable(self):
         self.data_generator.admin_permission()
         self.data_generator.admin_permission(
             email_address='m@mib.gov',
-            access_level=
-            admin_acls_model.AdminPermission.AccessLevel.SUPERADMIN)
+            access_level=(
+                admin_acls_model.AdminPermission.AccessLevel.SUPERADMIN))
         res = self.client.get('/haiti/admin/acls/', secure=True)
         self.assertEqual(len(res.context['editable_acls']), 1)
         self.assertEqual(len(res.context['fixed_acls']), 1)
@@ -93,7 +87,7 @@ def test_add_moderator(self):
             '/haiti/admin/acls/', secure=True))
         xsrf_token = get_doc.cssselect_one('input[name="xsrf_token"]').get(
             'value')
-        post_resp = self.client.post('/haiti/admin/acls', {
+        self.client.post('/haiti/admin/acls', {
             'xsrf_token': xsrf_token,
             'email_address': 'l@mib.gov',
             'expiration_date': '2019-04-25',
@@ -118,7 +112,7 @@ def test_add_manager(self):
             '/haiti/admin/acls/', secure=True))
         xsrf_token = get_doc.cssselect_one('input[name="xsrf_token"]').get(
             'value')
-        post_resp = self.client.post('/haiti/admin/acls', {
+        self.client.post('/haiti/admin/acls', {
             'xsrf_token': xsrf_token,
             'email_address': 'l@mib.gov',
             'expiration_date': '2019-04-25',
@@ -143,7 +137,7 @@ def test_add_superadmin(self):
             '/haiti/admin/acls/', secure=True))
         xsrf_token = get_doc.cssselect_one('input[name="xsrf_token"]').get(
             'value')
-        post_resp = self.client.post('/haiti/admin/acls', {
+        self.client.post('/haiti/admin/acls', {
             'xsrf_token': xsrf_token,
             'email_address': 'l@mib.gov',
             'expiration_date': '2019-04-25',
@@ -187,7 +181,7 @@ def test_edit_access_level(self):
             '/haiti/admin/acls/', secure=True))
         xsrf_token = get_doc.cssselect('input[name="xsrf_token"]')[1].get(
             'value')
-        post_resp = self.client.post('/haiti/admin/acls', {
+        self.client.post('/haiti/admin/acls', {
             'xsrf_token': xsrf_token,
             'email_address': 'l@mib.gov',
             'expiration_date': '2019-04-25',
@@ -208,8 +202,8 @@ def test_managers_cant_edit_superadmins(self):
         self.data_generator.admin_permission(
             email_address='l@mib.gov',
             expiration_date=datetime.datetime(2019, 4, 25),
-            access_level=
-            admin_acls_model.AdminPermission.AccessLevel.SUPERADMIN)
+            access_level=(
+                admin_acls_model.AdminPermission.AccessLevel.SUPERADMIN))
         get_doc = self.to_doc(self.client.get(
             '/haiti/admin/acls/', secure=True))
         xsrf_token = get_doc.cssselect('input[name="xsrf_token"]')[0].get(
@@ -252,7 +246,7 @@ def test_edit_expiration_date(self):
             '/haiti/admin/acls/', secure=True))
         xsrf_token = get_doc.cssselect('input[name="xsrf_token"]')[1].get(
             'value')
-        post_resp = self.client.post('/haiti/admin/acls', {
+        self.client.post('/haiti/admin/acls', {
             'xsrf_token': xsrf_token,
             'email_address': 'l@mib.gov',
             'expiration_date': '2019-05-25',
@@ -275,7 +269,7 @@ def test_revoke_access(self):
             '/haiti/admin/acls/', secure=True))
         xsrf_token = get_doc.cssselect('input[name="xsrf_token"]')[1].get(
             'value')
-        post_resp = self.client.post('/haiti/admin/acls', {
+        self.client.post('/haiti/admin/acls', {
             'xsrf_token': xsrf_token,
             'email_address': 'l@mib.gov',
             'expiration_date': '2019-04-25',
@@ -290,8 +284,8 @@ def test_managers_cant_revoke_superadmins(self):
         self.login_as_manager()
         self.data_generator.admin_permission(
             email_address='l@mib.gov',
-            access_level=
-            admin_acls_model.AdminPermission.AccessLevel.SUPERADMIN)
+            access_level=(
+                admin_acls_model.AdminPermission.AccessLevel.SUPERADMIN))
         get_doc = self.to_doc(self.client.get(
             '/haiti/admin/acls/', secure=True))
         xsrf_token = get_doc.cssselect('input[name="xsrf_token"]')[0].get(

tests/views/test_admin_delete_record.py

@@ -13,22 +13,12 @@
 # limitations under the License.
 """Tests for the admin delete-record page."""
 
-import datetime
-
 import django
 import django.http
 import django.test
 import six.moves.urllib.parse as urlparse
 
-# pylint: disable=wrong-import-order
-# pylint sometimes thinks config is a standard import that belongs before the
-# django import. It's mistaken; config is our own module (if you run
-# python -c "import config", it produces an error saying config doesn't exist).
-# Filed issue #626 to move us out of the global namespace someday, which would
-# prevent stuff like this.
-import config
 import const
-import model
 
 import view_tests_base
 

tests/views/test_admin_statistics.py

@@ -1,6 +1,3 @@
-import django
-import django.test
-
 import model
 
 import view_tests_base

tests/views/test_meta_sitemap.py

@@ -22,7 +22,6 @@
 # Filed issue #626 to move us out of the global namespace someday, which would
 # prevent stuff like this.
 import config
-import model
 
 import view_tests_base
 

tests/views/view_tests_base.py

@@ -17,8 +17,8 @@ def setUp(self):
         self._xsrf_tool = utils.XsrfTool()
         self.data_generator.admin_permission(
             repo_id='global', email_address='z@mib.gov',
-            access_level=
-            admin_acls_model.AdminPermission.AccessLevel.SUPERADMIN,
+            access_level=(
+                admin_acls_model.AdminPermission.AccessLevel.SUPERADMIN),
             expiration_date=datetime.datetime(2051, 1, 20))
         self.data_generator.admin_permission(
             repo_id='global', email_address='k@mib.gov',