lint: Up-rev hacking

Last time we did this was nearly 4 years ago; drag ourselves into
something approaching the present. Address a few new pyflakes issues
that seem reasonable to enforce:

   E275 missing whitespace after keyword
   E231 missing whitespace after ','
   E721 do not compare types, for exact checks use `is` / `is not`,
        for instance checks use `isinstance()`

Main motivator is that the old hacking kept us on an old version
of flake8 et al., which no longer work with newer Pythons.

Change-Id: I54b46349fabb9776dcadc6def1cfb961c123aaa0

swift/common/middleware/s3api/acl_handlers.py

@@ -168,7 +168,7 @@ class BaseAclHandler(object):
                 elem = fromstring(body, ACL.root_tag)
                 acl = ACL.from_elem(
                     elem, True, self.req.conf.allow_no_owner)
-            except(XMLSyntaxError, DocumentInvalid):
+            except (XMLSyntaxError, DocumentInvalid):
                 raise MalformedACLError()
             except Exception as e:
                 self.logger.error(e)

swift/common/middleware/s3api/controllers/acl.py

@@ -19,8 +19,9 @@ from swift.common.utils import public
 
 from swift.common.middleware.s3api.exception import ACLError
 from swift.common.middleware.s3api.controllers.base import Controller
-from swift.common.middleware.s3api.s3response import HTTPOk, S3NotImplemented,\
-    MalformedACLError, UnexpectedContent, MissingSecurityHeader
+from swift.common.middleware.s3api.s3response import (
+    HTTPOk, S3NotImplemented, MalformedACLError, UnexpectedContent,
+    MissingSecurityHeader)
 from swift.common.middleware.s3api.etree import Element, SubElement, tostring
 from swift.common.middleware.s3api.acl_utils import swift_acl_translate, \
     XMLNS_XSI

swift/common/middleware/s3api/controllers/logging.py

@@ -18,8 +18,8 @@ from swift.common.utils import public
 from swift.common.middleware.s3api.controllers.base import Controller, \
     bucket_operation
 from swift.common.middleware.s3api.etree import Element, tostring
-from swift.common.middleware.s3api.s3response import HTTPOk, S3NotImplemented,\
-    NoLoggingStatusForKey
+from swift.common.middleware.s3api.s3response import (
+    HTTPOk, S3NotImplemented, NoLoggingStatusForKey)
 
 
 class LoggingStatusController(Controller):

swift/common/middleware/slo.py

@@ -1602,7 +1602,7 @@ class StaticLargeObject(object):
         vrs, account, _junk = req.split_path(2, 3, True)
         new_env = req.environ.copy()
         new_env['REQUEST_METHOD'] = 'GET'
-        del(new_env['wsgi.input'])
+        del new_env['wsgi.input']
         new_env['QUERY_STRING'] = 'multipart-manifest=get'
         if 'version-id' in req.params:
             new_env['QUERY_STRING'] += \

swift/common/storage_policy.py

@@ -160,7 +160,7 @@ class BaseStoragePolicy(object):
                  object_ring=None, aliases='',
                  diskfile_module='egg:swift#replication.fs'):
         # do not allow BaseStoragePolicy class to be instantiated directly
-        if type(self) == BaseStoragePolicy:
+        if type(self) is BaseStoragePolicy:
             raise TypeError("Can't instantiate BaseStoragePolicy directly")
         # policy parameter validation
         try:

swift/common/utils/__init__.py

@@ -5568,7 +5568,7 @@ class ShardRangeList(UserList):
     def __getitem__(self, index):
         # workaround for py3 - not needed for py2.7,py3.8
         result = self.data[index]
-        return ShardRangeList(result) if type(result) == list else result
+        return ShardRangeList(result) if type(result) is list else result
 
     @property
     def lower(self):
@@ -6365,7 +6365,7 @@ class Watchdog(object):
         :param key: timeout id, as returned by start()
         """
         try:
-            del(self._timeouts[key])
+            del self._timeouts[key]
         except KeyError:
             pass
 

swift/obj/auditor.py

@@ -24,7 +24,7 @@ from contextlib import closing
 from eventlet import Timeout
 
 from swift.obj import diskfile, replicator
-from swift.common.exceptions import DiskFileQuarantined, DiskFileNotExist,\
+from swift.common.exceptions import DiskFileQuarantined, DiskFileNotExist, \
     DiskFileDeleted, DiskFileExpired, QuarantineRequest
 from swift.common.daemon import Daemon
 from swift.common.storage_policy import POLICIES

test-requirements.txt

@@ -3,7 +3,7 @@
 # process, which may cause wedges in the gate later.
 
 # Hacking already pins down pep8, pyflakes and flake8
-hacking>=2.0,<2.1.0 # Apache-2.0
+hacking>=2.0,<6.2.0 # Apache-2.0
 coverage>=5.0.4 # Apache-2.0
 pytest>=4.6.11  # MIT
 pytest-cov>=2.12.1  # MIT

test/unit/cli/test_manage_shard_ranges.py

@@ -1381,7 +1381,7 @@ class TestManageShardRanges(unittest.TestCase):
         def do_compact(user_input, options, exp_changes, exit_code):
             out = StringIO()
             err = StringIO()
-            with mock.patch('sys.stdout', out),\
+            with mock.patch('sys.stdout', out), \
                     mock.patch('sys.stderr', err), \
                     mock.patch('swift.cli.manage_shard_ranges.input',
                                side_effect=[user_input]):

test/unit/cli/test_recon.py

@@ -643,7 +643,7 @@ aliases = %s
         stdout = StringIO()
         with mock.patch.object(sys, 'argv', [
                 "prog", "object", "--swiftdir=%s" % self.swift_dir,
-                "--validate-servers", '--policy=invalid']),\
+                "--validate-servers", '--policy=invalid']), \
                 mock.patch('sys.stdout', stdout):
             self.assertRaises(SystemExit, recon.main)
             self.assertIn('Invalid Storage Policy', stdout.getvalue())

test/unit/common/middleware/s3api/test_s3request.py

@@ -116,7 +116,7 @@ class TestRequest(S3ApiTestCase):
                       'check_permission') as m_check_permission:
             mock_get_resp.return_value = fake_swift_resp \
                 or FakeResponse(self.s3api.conf.s3_acl)
-            return mock_get_resp, m_check_permission,\
+            return mock_get_resp, m_check_permission, \
                 s3_req.get_response(self.s3api)
 
     def test_get_response_without_s3_acl(self):

test/unit/common/ring/test_builder.py

@@ -1987,7 +1987,7 @@ class TestRingBuilder(unittest.TestCase):
             # test old devs but no meta
             no_meta_builder = rb
             for dev in no_meta_builder.devs:
-                del(dev['meta'])
+                del dev['meta']
             fake_pickle.return_value = no_meta_builder
             pickle.load = fake_pickle
             builder = ring.RingBuilder.load('fake.builder', open=fake_open)

test/unit/common/test_db_auditor.py

@@ -129,7 +129,7 @@ class TestAuditor(unittest.TestCase):
 
         # force code coverage for logging path
         with mock.patch('swift.common.db_auditor.audit_location_generator',
-                        fake_audit_location_generator),\
+                        fake_audit_location_generator), \
                 mock.patch('time.time',
                            return_value=(test_auditor.logging_interval * 2)):
             test_auditor._one_audit_pass(0)

test/unit/common/test_internal_client.py

@@ -375,8 +375,8 @@ class TestInternalClient(unittest.TestCase):
         user_agent = 'some_user_agent'
         request_tries = 123
 
-        with mock.patch.object(
-                internal_client, 'loadapp', return_value=app) as mock_loadapp,\
+        with mock.patch.object(internal_client, 'loadapp',
+                               return_value=app) as mock_loadapp, \
                 self.assertRaises(ValueError):
             # First try with a bad arg
             internal_client.InternalClient(
@@ -432,8 +432,8 @@ class TestInternalClient(unittest.TestCase):
         app = FakeSwift()
         user_agent = 'some_user_agent'
 
-        with mock.patch.object(
-                internal_client, 'loadapp', return_value=app) as mock_loadapp,\
+        with mock.patch.object(internal_client, 'loadapp',
+                               return_value=app) as mock_loadapp, \
                 self.assertRaises(ValueError) as cm:
             internal_client.InternalClient(
                 conf_path, user_agent, 1, allow_modify_pipeline=True)
@@ -775,7 +775,8 @@ class TestInternalClient(unittest.TestCase):
             client = internal_client.InternalClient(
                 None, "some_agent", 3, use_replication_network=False,
                 app=fake_app)
-            with self.assertRaises(internal_client.UnexpectedResponse) as ctx,\
+            expected_error = internal_client.UnexpectedResponse
+            with self.assertRaises(expected_error) as ctx, \
                     mock.patch('swift.common.internal_client.sleep'):
                 # This is obvious strange tests to expect only 400 Bad Request
                 # but this test intended to avoid extra body drain if it's

test/unit/common/test_utils.py

@@ -9447,7 +9447,7 @@ class TestWatchdog(unittest.TestCase):
 
         now = time.time()
         timeout_value = 1.0
-        with patch('eventlet.greenthread.getcurrent', return_value=gth),\
+        with patch('eventlet.greenthread.getcurrent', return_value=gth), \
                 patch('time.time', return_value=now):
             # On first call, _next_expiration is None, it should unblock
             # greenthread that is blocked for ever

test/unit/helpers.py

@@ -267,7 +267,7 @@ def setup_servers(the_object_server=object_server, extra_conf=None):
                                                         {'X-Timestamp': ts,
                                                          'x-trans-id': 'test'})
         resp = conn.getresponse()
-        assert(resp.status == 201)
+        assert resp.status == 201
     # Create another account
     # used for account-to-account tests
     ts = normalize_timestamp(time.time())
@@ -281,7 +281,7 @@ def setup_servers(the_object_server=object_server, extra_conf=None):
                                                         {'X-Timestamp': ts,
                                                          'x-trans-id': 'test'})
         resp = conn.getresponse()
-        assert(resp.status == 201)
+        assert resp.status == 201
     # Create containers, 1 per test policy
     sock = connect_tcp(('localhost', prolis.getsockname()[1]))
     fd = sock.makefile('rwb')

test/unit/obj/test_server.py

@@ -1139,7 +1139,7 @@ class TestObjectController(BaseTestCase):
                         'X-Backend-Redirect-Timestamp': next(self.ts).internal}
 
         with mocked_http_conn(301, headers=[resp_headers]) as conn, \
-                mock.patch('swift.common.utils.HASH_PATH_PREFIX', b''),\
+                mock.patch('swift.common.utils.HASH_PATH_PREFIX', b''), \
                 fake_spawn():
             resp = req.get_response(self.object_controller)
 

tox.ini

@@ -146,6 +146,9 @@ commands = bandit -c bandit.yaml -r swift -n 5
 # it's not a bug that we aren't using all of hacking, ignore:
 # H101: Use TODO(NAME)
 # H202: assertRaises Exception too broad
+# H211/H212: Use assert{Is,IsNot}instance
+# H214: Use assertIn/NotIn ...
+# H216: The unittest.mock module should be used rather than ...
 # H301: one import per line
 # H306: imports not in alphabetical order (time, os)
 # H404: multi line docstring should start without a leading new line
@@ -159,7 +162,7 @@ commands = bandit -c bandit.yaml -r swift -n 5
 # Swift team needs to decide if they want to enable either of these:
 # W503: line break before binary operator
 # W504: line break after binary operator
-ignore = H101,H202,H301,H306,H404,H405,H501,W503,W504,E402,E731,E741
+ignore = H101,H202,H211,H212,H214,H216,H301,H306,H404,H405,H501,W503,W504,E402,E731,E741
 exclude = .venv,.tox,dist,*egg
 filename = *.py,bin/*
 show-source = True