From 76ca11773eb6508e1140da2b04ddb2ebc59cd753 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Wed, 7 Feb 2024 22:27:13 +0000 Subject: [PATCH] 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 | 2 +- swift/common/middleware/s3api/controllers/acl.py | 5 +++-- swift/common/middleware/s3api/controllers/logging.py | 4 ++-- swift/common/middleware/slo.py | 2 +- swift/common/storage_policy.py | 2 +- swift/common/utils/__init__.py | 4 ++-- swift/obj/auditor.py | 2 +- test-requirements.txt | 2 +- test/unit/cli/test_manage_shard_ranges.py | 2 +- test/unit/cli/test_recon.py | 2 +- test/unit/common/middleware/s3api/test_s3request.py | 2 +- test/unit/common/ring/test_builder.py | 2 +- test/unit/common/test_db_auditor.py | 2 +- test/unit/common/test_internal_client.py | 11 ++++++----- test/unit/common/test_utils.py | 2 +- test/unit/helpers.py | 4 ++-- test/unit/obj/test_server.py | 2 +- tox.ini | 5 ++++- 18 files changed, 31 insertions(+), 26 deletions(-) diff --git a/swift/common/middleware/s3api/acl_handlers.py b/swift/common/middleware/s3api/acl_handlers.py index 699b387713..a96f9723a2 100644 --- a/swift/common/middleware/s3api/acl_handlers.py +++ b/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) diff --git a/swift/common/middleware/s3api/controllers/acl.py b/swift/common/middleware/s3api/controllers/acl.py index 79e4f9d1bb..09d49a0bb1 100644 --- a/swift/common/middleware/s3api/controllers/acl.py +++ b/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 diff --git a/swift/common/middleware/s3api/controllers/logging.py b/swift/common/middleware/s3api/controllers/logging.py index 21407d3b1b..5eec0151bc 100644 --- a/swift/common/middleware/s3api/controllers/logging.py +++ b/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): diff --git a/swift/common/middleware/slo.py b/swift/common/middleware/slo.py index d8b86218dc..8b7fd911d9 100644 --- a/swift/common/middleware/slo.py +++ b/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'] += \ diff --git a/swift/common/storage_policy.py b/swift/common/storage_policy.py index 88c892aa67..dcad56b9c9 100644 --- a/swift/common/storage_policy.py +++ b/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: diff --git a/swift/common/utils/__init__.py b/swift/common/utils/__init__.py index ce4e099f44..82d0d202aa 100644 --- a/swift/common/utils/__init__.py +++ b/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 diff --git a/swift/obj/auditor.py b/swift/obj/auditor.py index b34c3d7f32..1f94f97c9a 100644 --- a/swift/obj/auditor.py +++ b/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 diff --git a/test-requirements.txt b/test-requirements.txt index 488080ae5c..1e77045c9d 100644 --- a/test-requirements.txt +++ b/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 diff --git a/test/unit/cli/test_manage_shard_ranges.py b/test/unit/cli/test_manage_shard_ranges.py index c40d0644b7..9efcf2ad2f 100644 --- a/test/unit/cli/test_manage_shard_ranges.py +++ b/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]): diff --git a/test/unit/cli/test_recon.py b/test/unit/cli/test_recon.py index 421e68aeb1..128a62be6e 100644 --- a/test/unit/cli/test_recon.py +++ b/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()) diff --git a/test/unit/common/middleware/s3api/test_s3request.py b/test/unit/common/middleware/s3api/test_s3request.py index ac2e1cc738..01cc8f2ac3 100644 --- a/test/unit/common/middleware/s3api/test_s3request.py +++ b/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): diff --git a/test/unit/common/ring/test_builder.py b/test/unit/common/ring/test_builder.py index 9e6405ee1a..3a9c86e877 100644 --- a/test/unit/common/ring/test_builder.py +++ b/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) diff --git a/test/unit/common/test_db_auditor.py b/test/unit/common/test_db_auditor.py index d52e992041..c35e46e936 100644 --- a/test/unit/common/test_db_auditor.py +++ b/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) diff --git a/test/unit/common/test_internal_client.py b/test/unit/common/test_internal_client.py index c4084f62f9..e32d16f9a2 100644 --- a/test/unit/common/test_internal_client.py +++ b/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 diff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py index bd692c8ea1..f48eaf03d3 100644 --- a/test/unit/common/test_utils.py +++ b/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 diff --git a/test/unit/helpers.py b/test/unit/helpers.py index 1cb89fd0d9..ab50e7200b 100644 --- a/test/unit/helpers.py +++ b/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') diff --git a/test/unit/obj/test_server.py b/test/unit/obj/test_server.py index 5cebef7a36..52ab294585 100644 --- a/test/unit/obj/test_server.py +++ b/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) diff --git a/tox.ini b/tox.ini index 69353b888f..01bfcda6b1 100644 --- a/tox.ini +++ b/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