From abe70e8323825a3a6465148ac0d50aa3c9edf575 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Wed, 30 Jan 2013 16:33:28 +1100 Subject: [PATCH] Cleanup based on pyflakes. pyflakes itself can't be used in any automated gating way, because there are two sets of false errors it raises. However, as an exercise, cleaning up the 'valid' ones uncovered three actual bugs. The other changes (mostly unused variables) are included here for fun. Command run: pyflakes swift | grep -v "undefined name '_'" Change-Id: I18696bf047dedad1a9fdbde3463e214fba95f7c6 --- swift/account/auditor.py | 2 +- swift/account/server.py | 1 - swift/common/bench.py | 6 +-- swift/common/constraints.py | 3 +- swift/common/db_replicator.py | 2 +- swift/common/memcached.py | 5 +-- swift/common/middleware/recon.py | 6 +-- swift/common/middleware/staticweb.py | 7 +--- swift/common/middleware/tempauth.py | 2 +- swift/common/ring/__init__.py | 6 +++ swift/common/ring/ring.py | 7 +--- swift/common/swob.py | 9 +++-- swift/common/utils.py | 8 ++-- swift/container/auditor.py | 2 +- swift/container/server.py | 2 - swift/obj/replicator.py | 4 +- swift/obj/server.py | 11 +++--- swift/proxy/controllers/__init__.py | 7 ++++ swift/proxy/controllers/base.py | 8 ++-- swift/proxy/controllers/obj.py | 2 +- swift/proxy/server.py | 8 ++-- test/unit/proxy/test_server.py | 55 ++++++++++++++-------------- 22 files changed, 74 insertions(+), 89 deletions(-) diff --git a/swift/account/auditor.py b/swift/account/auditor.py index eeb73d8dfe..bb44b94da2 100644 --- a/swift/account/auditor.py +++ b/swift/account/auditor.py @@ -112,7 +112,7 @@ class AccountAuditor(Daemon): return broker = AccountBroker(path) if not broker.is_deleted(): - info = broker.get_info() + broker.get_info() self.logger.increment('passes') self.account_passes += 1 self.logger.debug(_('Audit passed for %s') % broker.db_file) diff --git a/swift/account/server.py b/swift/account/server.py index 5a6ca3fa52..86800d1fb2 100644 --- a/swift/account/server.py +++ b/swift/account/server.py @@ -18,7 +18,6 @@ from __future__ import with_statement import os import time import traceback -from urllib import unquote from xml.sax import saxutils from eventlet import Timeout diff --git a/swift/common/bench.py b/swift/common/bench.py index c6da4100e4..3b8070b2d0 100644 --- a/swift/common/bench.py +++ b/swift/common/bench.py @@ -32,11 +32,7 @@ from swift.common.utils import config_true_value, LogAdapter import swiftclient as client from swift.common import direct_client from swift.common.http import HTTP_CONFLICT - -try: - import simplejson as json -except ImportError: - import json +from swift.common.utils import json def _func_on_containers(logger, conf, concurrency_key, func): diff --git a/swift/common/constraints.py b/swift/common/constraints.py index ee6822e45c..35c9eb5efe 100644 --- a/swift/common/constraints.py +++ b/swift/common/constraints.py @@ -15,8 +15,7 @@ import os import urllib -from ConfigParser import ConfigParser, NoSectionError, NoOptionError, \ - RawConfigParser +from ConfigParser import ConfigParser, NoSectionError, NoOptionError from swift.common.swob import HTTPBadRequest, HTTPLengthRequired, \ HTTPRequestEntityTooLarge diff --git a/swift/common/db_replicator.py b/swift/common/db_replicator.py index 72eaa920b8..370b602e32 100644 --- a/swift/common/db_replicator.py +++ b/swift/common/db_replicator.py @@ -37,7 +37,7 @@ from swift.common.bufferedhttp import BufferedHTTPConnection from swift.common.exceptions import DriveNotMounted, ConnectionTimeout from swift.common.daemon import Daemon from swift.common.swob import Response, HTTPNotFound, HTTPNoContent, \ - HTTPAccepted, HTTPInsufficientStorage, HTTPBadRequest + HTTPAccepted, HTTPBadRequest DEBUG_TIMINGS_THRESHOLD = 10 diff --git a/swift/common/memcached.py b/swift/common/memcached.py index 42e9b34efd..123f5beb97 100644 --- a/swift/common/memcached.py +++ b/swift/common/memcached.py @@ -27,10 +27,7 @@ import time from bisect import bisect from hashlib import md5 -try: - import simplejson as json -except ImportError: - import json +from swift.common.utils import json DEFAULT_MEMCACHED_PORT = 11211 diff --git a/swift/common/middleware/recon.py b/swift/common/middleware/recon.py index 17c95dd189..9e15269121 100644 --- a/swift/common/middleware/recon.py +++ b/swift/common/middleware/recon.py @@ -17,14 +17,10 @@ import errno import os from swift.common.swob import Request, Response -from swift.common.utils import get_logger, config_true_value +from swift.common.utils import get_logger, config_true_value, json from swift.common.constraints import check_mount from resource import getpagesize from hashlib import md5 -try: - import simplejson as json -except ImportError: - import json class ReconMiddleware(object): diff --git a/swift/common/middleware/staticweb.py b/swift/common/middleware/staticweb.py index 6aaeaa0ee9..5944af2961 100644 --- a/swift/common/middleware/staticweb.py +++ b/swift/common/middleware/staticweb.py @@ -110,18 +110,13 @@ Example usage of this middleware via ``swift``: """ -try: - import simplejson as json -except ImportError: - import json - import cgi import time from urllib import unquote, quote as urllib_quote from swift.common.utils import cache_from_env, get_logger, human_readable, \ - split_path, config_true_value + split_path, config_true_value, json from swift.common.wsgi import make_pre_authed_env, make_pre_authed_request, \ WSGIContext from swift.common.http import is_success, is_redirection, HTTP_NOT_FOUND diff --git a/swift/common/middleware/tempauth.py b/swift/common/middleware/tempauth.py index 7a2f2462e9..6e945861d2 100644 --- a/swift/common/middleware/tempauth.py +++ b/swift/common/middleware/tempauth.py @@ -27,7 +27,7 @@ from swift.common.swob import HTTPBadRequest, HTTPForbidden, HTTPNotFound, \ HTTPUnauthorized from swift.common.middleware.acl import clean_acl, parse_acl, referrer_allowed -from swift.common.utils import cache_from_env, get_logger, get_remote_client, \ +from swift.common.utils import cache_from_env, get_logger, \ split_path, config_true_value from swift.common.http import HTTP_CLIENT_CLOSED_REQUEST diff --git a/swift/common/ring/__init__.py b/swift/common/ring/__init__.py index 6040b860e3..7027aab2db 100644 --- a/swift/common/ring/__init__.py +++ b/swift/common/ring/__init__.py @@ -1,2 +1,8 @@ from ring import RingData, Ring from builder import RingBuilder + +__all__ = [ + 'RingData', + 'Ring', + 'RingBuilder', +] diff --git a/swift/common/ring/ring.py b/swift/common/ring/ring.py index bc76e42e17..870a04c435 100644 --- a/swift/common/ring/ring.py +++ b/swift/common/ring/ring.py @@ -23,14 +23,9 @@ from time import time import os from io import BufferedReader -from swift.common.utils import hash_path, validate_configuration +from swift.common.utils import hash_path, validate_configuration, json from swift.common.ring.utils import tiers_for_dev -try: - import simplejson as json -except ImportError: - import json - class RingData(object): """Partitioned consistent hashing ring data (used for serialization).""" diff --git a/swift/common/swob.py b/swift/common/swob.py index f745ce79dd..f40ff22a34 100755 --- a/swift/common/swob.py +++ b/swift/common/swob.py @@ -26,7 +26,7 @@ from cStringIO import StringIO import UserDict import time from functools import partial -from datetime import datetime, date, timedelta, tzinfo +from datetime import datetime, timedelta, tzinfo from email.utils import parsedate import urlparse import urllib2 @@ -1067,19 +1067,20 @@ def wsgify(func): argspec = inspect.getargspec(func) if argspec.args and argspec.args[0] == 'self': @functools.wraps(func) - def _wsgify(self, env, start_response): + def _wsgify_self(self, env, start_response): try: return func(self, Request(env))(env, start_response) except HTTPException, err_resp: return err_resp(env, start_response) + return _wsgify_self else: @functools.wraps(func) - def _wsgify(env, start_response): + def _wsgify_bare(env, start_response): try: return func(Request(env))(env, start_response) except HTTPException, err_resp: return err_resp(env, start_response) - return _wsgify + return _wsgify_bare class StatusMap(object): diff --git a/swift/common/utils.py b/swift/common/utils.py index a9f72c893d..d771e5bcd5 100644 --- a/swift/common/utils.py +++ b/swift/common/utils.py @@ -39,9 +39,7 @@ except ImportError: import cPickle as pickle import glob from urlparse import urlparse as stdlib_urlparse, ParseResult -import socket import itertools -import types import eventlet from eventlet import GreenPool, sleep, Timeout @@ -286,7 +284,7 @@ def renamer(old, new): try: mkdirs(os.path.dirname(new)) os.rename(old, new) - except OSError, err: + except OSError: mkdirs(os.path.dirname(new)) os.rename(old, new) @@ -1558,8 +1556,8 @@ def reiterate(iterable): try: chunk = '' while not chunk: - chunk = next(iterable) - return itertools.chain([chunk], iterable) + chunk = next(iterator) + return itertools.chain([chunk], iterator) except StopIteration: return [] diff --git a/swift/container/auditor.py b/swift/container/auditor.py index c2921b1175..437163589b 100644 --- a/swift/container/auditor.py +++ b/swift/container/auditor.py @@ -111,7 +111,7 @@ class ContainerAuditor(Daemon): return broker = ContainerBroker(path) if not broker.is_deleted(): - info = broker.get_info() + broker.get_info() self.logger.increment('passes') self.container_passes += 1 self.logger.debug(_('Audit passed for %s'), broker.db_file) diff --git a/swift/container/server.py b/swift/container/server.py index 12df62b2bd..9cf1d2b392 100644 --- a/swift/container/server.py +++ b/swift/container/server.py @@ -15,11 +15,9 @@ from __future__ import with_statement -import itertools import os import time import traceback -from urllib import unquote from xml.sax import saxutils from datetime import datetime diff --git a/swift/obj/replicator.py b/swift/obj/replicator.py index c5c28ad940..25b6988f40 100644 --- a/swift/obj/replicator.py +++ b/swift/obj/replicator.py @@ -566,7 +566,7 @@ class ObjectReplicator(Daemon): if not os.path.exists(obj_path): try: mkdirs(obj_path) - except Exception, err: + except Exception: self.logger.exception('ERROR creating %s' % obj_path) continue for partition in os.listdir(obj_path): @@ -589,7 +589,7 @@ class ObjectReplicator(Daemon): nodes=nodes, delete=len(nodes) > len(part_nodes) - 1, partition=partition)) - except ValueError, OSError: + except (ValueError, OSError): continue random.shuffle(jobs) self.job_count = len(jobs) diff --git a/swift/obj/server.py b/swift/obj/server.py index d6e5865d87..d092bbdac7 100755 --- a/swift/obj/server.py +++ b/swift/obj/server.py @@ -18,7 +18,6 @@ from __future__ import with_statement import cPickle as pickle import errno -import itertools import os import time import traceback @@ -224,8 +223,8 @@ class DiskFile(object): def _handle_close_quarantine(self): """Check if file needs to be quarantined""" try: - obj_size = self.get_data_file_size() - except DiskFileError, e: + self.get_data_file_size() + except DiskFileError: self.quarantine() return except DiskFileNotExist: @@ -505,8 +504,8 @@ class ObjectController(object): self.logger.error(_('ERROR Container update failed: different ' 'numbers of hosts and devices in request: ' '"%s" vs "%s"' % - (req.headers.get('X-Container-Host', ''), - req.headers.get('X-Container-Device', '')))) + (headers_in.get('X-Container-Host', ''), + headers_in.get('X-Container-Device', '')))) return if contpartition: @@ -590,7 +589,7 @@ class ObjectController(object): if file.is_deleted() or file.is_expired(): return HTTPNotFound(request=request) try: - file_size = file.get_data_file_size() + file.get_data_file_size() except (DiskFileError, DiskFileNotExist): file.quarantine() return HTTPNotFound(request=request) diff --git a/swift/proxy/controllers/__init__.py b/swift/proxy/controllers/__init__.py index 516f3c2859..cb50226734 100644 --- a/swift/proxy/controllers/__init__.py +++ b/swift/proxy/controllers/__init__.py @@ -2,3 +2,10 @@ from swift.proxy.controllers.base import Controller from swift.proxy.controllers.obj import ObjectController from swift.proxy.controllers.account import AccountController from swift.proxy.controllers.container import ContainerController + +__all__ = [ + 'AccountController', + 'ContainerController', + 'Controller', + 'ObjectController', +] diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py index 3c93c6731d..7e0dd82bac 100644 --- a/swift/proxy/controllers/base.py +++ b/swift/proxy/controllers/base.py @@ -28,7 +28,7 @@ import time import functools import inspect -from eventlet import spawn_n, GreenPile, Timeout +from eventlet import spawn_n, GreenPile from eventlet.queue import Queue, Empty, Full from eventlet.timeout import Timeout @@ -40,7 +40,7 @@ from swift.common.http import is_informational, is_success, is_redirection, \ is_server_error, HTTP_OK, HTTP_PARTIAL_CONTENT, HTTP_MULTIPLE_CHOICES, \ HTTP_BAD_REQUEST, HTTP_NOT_FOUND, HTTP_SERVICE_UNAVAILABLE, \ HTTP_INSUFFICIENT_STORAGE, HTTP_UNAUTHORIZED -from swift.common.swob import Request, Response, status_map +from swift.common.swob import Request, Response def update_headers(response, headers): @@ -330,7 +330,7 @@ class Controller(object): path, headers) with Timeout(self.app.node_timeout): resp = conn.getresponse() - body = resp.read() + resp.read() if is_success(resp.status): result_code = HTTP_OK container_count = int( @@ -421,7 +421,7 @@ class Controller(object): path, headers) with Timeout(self.app.node_timeout): resp = conn.getresponse() - body = resp.read() + resp.read() if is_success(resp.status): container_info.update( headers_to_container_info(resp.getheaders())) diff --git a/swift/proxy/controllers/obj.py b/swift/proxy/controllers/obj.py index 386a1cb40d..ee2b7da6fb 100644 --- a/swift/proxy/controllers/obj.py +++ b/swift/proxy/controllers/obj.py @@ -33,7 +33,7 @@ from urllib import unquote, quote from hashlib import md5 from random import shuffle -from eventlet import sleep, GreenPile, Timeout +from eventlet import sleep, GreenPile from eventlet.queue import Queue from eventlet.timeout import Timeout diff --git a/swift/proxy/server.py b/swift/proxy/server.py index cddec4e3dc..40234276d0 100644 --- a/swift/proxy/server.py +++ b/swift/proxy/server.py @@ -26,7 +26,6 @@ import mimetypes import os -import time from ConfigParser import ConfigParser import uuid @@ -37,11 +36,10 @@ from swift.common.utils import cache_from_env, get_logger, \ get_remote_client, split_path, config_true_value from swift.common.constraints import check_utf8 from swift.proxy.controllers import AccountController, ObjectController, \ - ContainerController, Controller -from swift.common.swob import HTTPAccepted, HTTPBadRequest, HTTPForbidden, \ + ContainerController +from swift.common.swob import HTTPBadRequest, HTTPForbidden, \ HTTPMethodNotAllowed, HTTPNotFound, HTTPPreconditionFailed, \ - HTTPRequestEntityTooLarge, HTTPRequestTimeout, HTTPServerError, \ - HTTPServiceUnavailable, HTTPClientDisconnect, status_map, Request, Response + HTTPServerError, Request class Application(object): diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index 15349e44e2..c122f5fa25 100755 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -28,7 +28,7 @@ from cStringIO import StringIO from gzip import GzipFile from httplib import HTTPException from shutil import rmtree -from time import time +import time from urllib import unquote, quote from hashlib import md5 from tempfile import mkdtemp @@ -150,7 +150,7 @@ def setup(): _test_coros = \ (prospa, acc1spa, acc2spa, con1spa, con2spa, obj1spa, obj2spa) # Create account - ts = normalize_timestamp(time()) + ts = normalize_timestamp(time.time()) partition, nodes = prosrv.account_ring.get_nodes('a') for node in nodes: conn = swift.proxy.controllers.obj.http_connect(node['ip'], @@ -390,11 +390,12 @@ class FakeMemcacheReturnsNone(FakeMemcache): def save_globals(): orig_http_connect = getattr(swift.proxy.controllers.base, 'http_connect', None) - orig_account_info = getattr(proxy_server.Controller, 'account_info', None) + orig_account_info = getattr(swift.proxy.controllers.Controller, + 'account_info', None) try: yield True finally: - proxy_server.Controller.account_info = orig_account_info + swift.proxy.controllers.Controller.account_info = orig_account_info swift.proxy.controllers.base.http_connect = orig_http_connect swift.proxy.controllers.obj.http_connect = orig_http_connect swift.proxy.controllers.account.http_connect = orig_http_connect @@ -430,7 +431,7 @@ class TestController(unittest.TestCase): account_ring=self.account_ring, container_ring=self.container_ring, object_ring=FakeRing()) - self.controller = proxy_server.Controller(app) + self.controller = swift.proxy.controllers.Controller(app) self.account = 'some_account' self.container = 'some_container' @@ -580,7 +581,7 @@ class TestController(unittest.TestCase): return None, None with save_globals(): - proxy_server.Controller.account_info = account_info + swift.proxy.controllers.Controller.account_info = account_info ret = self.controller.container_info(self.account, self.container) self.check_container_info_return(ret, True) @@ -593,7 +594,7 @@ class TestController(unittest.TestCase): with save_globals(): headers = {'x-container-read': self.read_acl, 'x-container-write': self.write_acl} - proxy_server.Controller.account_info = account_info + swift.proxy.controllers.Controller.account_info = account_info set_http_connect(200, headers=headers) ret = self.controller.container_info(self.account, self.container) @@ -616,7 +617,7 @@ class TestController(unittest.TestCase): return True, True, 0 with save_globals(): - proxy_server.Controller.account_info = account_info + swift.proxy.controllers.Controller.account_info = account_info set_http_connect(404, 404, 404) ret = self.controller.container_info(self.account, self.container) @@ -1831,7 +1832,7 @@ class TestObjectController(unittest.TestCase): for dev in self.app.account_ring.devs.values(): dev['errors'] = self.app.error_suppression_limit + 1 - dev['last_error'] = time() + dev['last_error'] = time.time() set_http_connect(200) # acct [isn't actually called since everything # is error limited] @@ -1842,7 +1843,7 @@ class TestObjectController(unittest.TestCase): dev['errors'] = 0 for dev in self.app.container_ring.devs.values(): dev['errors'] = self.app.error_suppression_limit + 1 - dev['last_error'] = time() + dev['last_error'] = time.time() set_http_connect(200, 200) # acct cont [isn't actually called since # everything is error limited] @@ -3423,10 +3424,10 @@ class TestObjectController(unittest.TestCase): 'container', 'object') set_http_connect(200, 200, 200, 200, 200, 202, 202, 202) self.app.memcache.store = {} - orig_time = proxy_server.time.time + orig_time = time.time try: - t = time() - proxy_server.time.time = lambda: t + t = time.time() + time.time = lambda: t req = Request.blank('/a/c/o', {}, headers={'Content-Type': 'foo/bar', 'X-Delete-After': '60'}) @@ -3451,7 +3452,7 @@ class TestObjectController(unittest.TestCase): self.assertEquals(req.headers.get('x-delete-at'), str(int(t + 60))) finally: - proxy_server.time.time = orig_time + time.time = orig_time def test_POST_non_int_delete_after(self): with save_globals(): @@ -3495,7 +3496,7 @@ class TestObjectController(unittest.TestCase): controller.make_requests = fake_make_requests set_http_connect(200, 200) self.app.memcache.store = {} - t = str(int(time() + 100)) + t = str(int(time.time() + 100)) req = Request.blank('/a/c/o', {}, headers={'Content-Type': 'foo/bar', 'X-Delete-At': t}) @@ -3506,7 +3507,7 @@ class TestObjectController(unittest.TestCase): self.assertTrue('X-Delete-At-Device' in given_headers) self.assertTrue('X-Delete-At-Partition' in given_headers) - t = str(int(time() + 100)) + '.1' + t = str(int(time.time() + 100)) + '.1' req = Request.blank('/a/c/o', {}, headers={'Content-Type': 'foo/bar', 'X-Delete-At': t}) @@ -3515,7 +3516,7 @@ class TestObjectController(unittest.TestCase): self.assertEquals(resp.status_int, 400) self.assertTrue('Non-integer X-Delete-At' in resp.body) - t = str(int(time() - 100)) + t = str(int(time.time() - 100)) req = Request.blank('/a/c/o', {}, headers={'Content-Type': 'foo/bar', 'X-Delete-At': t}) @@ -3530,10 +3531,10 @@ class TestObjectController(unittest.TestCase): 'container', 'object') set_http_connect(200, 200, 201, 201, 201) self.app.memcache.store = {} - orig_time = proxy_server.time.time + orig_time = time.time try: - t = time() - proxy_server.time.time = lambda: t + t = time.time() + time.time = lambda: t req = Request.blank('/a/c/o', {}, headers={'Content-Length': '0', 'Content-Type': 'foo/bar', @@ -3544,7 +3545,7 @@ class TestObjectController(unittest.TestCase): self.assertEquals(req.headers.get('x-delete-at'), str(int(t + 60))) finally: - proxy_server.time.time = orig_time + time.time = orig_time def test_PUT_non_int_delete_after(self): with save_globals(): @@ -3589,7 +3590,7 @@ class TestObjectController(unittest.TestCase): controller._connect_put_node = fake_connect_put_node set_http_connect(200, 200) self.app.memcache.store = {} - t = str(int(time() + 100)) + t = str(int(time.time() + 100)) req = Request.blank('/a/c/o', {}, headers={'Content-Length': '0', 'Content-Type': 'foo/bar', @@ -3601,7 +3602,7 @@ class TestObjectController(unittest.TestCase): self.assertTrue('X-Delete-At-Device' in given_headers) self.assertTrue('X-Delete-At-Partition' in given_headers) - t = str(int(time() + 100)) + '.1' + t = str(int(time.time() + 100)) + '.1' req = Request.blank('/a/c/o', {}, headers={'Content-Length': '0', 'Content-Type': 'foo/bar', @@ -3611,7 +3612,7 @@ class TestObjectController(unittest.TestCase): self.assertEquals(resp.status_int, 400) self.assertTrue('Non-integer X-Delete-At' in resp.body) - t = str(int(time() - 100)) + t = str(int(time.time() - 100)) req = Request.blank('/a/c/o', {}, headers={'Content-Length': '0', 'Content-Type': 'foo/bar', @@ -3977,7 +3978,7 @@ class TestObjectController(unittest.TestCase): req = Request.blank('/a/c/o', environ={'REQUEST_METHOD': 'PUT'}, headers={'Content-Type': 'application/stuff', 'Content-Length': '0', - 'X-Delete-At': int(time()) + 100000}) + 'X-Delete-At': int(time.time()) + 100000}) controller = proxy_server.ObjectController(self.app, 'a', 'c', 'o') seen_headers = self._gather_x_container_headers( controller.PUT, req, @@ -4005,7 +4006,7 @@ class TestObjectController(unittest.TestCase): req = Request.blank('/a/c/o', environ={'REQUEST_METHOD': 'PUT'}, headers={'Content-Type': 'application/stuff', 'Content-Length': 0, - 'X-Delete-At': int(time()) + 100000}) + 'X-Delete-At': int(time.time()) + 100000}) controller = proxy_server.ObjectController(self.app, 'a', 'c', 'o') seen_headers = self._gather_x_container_headers( controller.PUT, req, @@ -4179,7 +4180,7 @@ class TestContainerController(unittest.TestCase): for dev in self.app.account_ring.devs.values(): dev['errors'] = self.app.error_suppression_limit + 1 - dev['last_error'] = time() + dev['last_error'] = time.time() set_http_connect(200, 200, 200, 200, 200, 200) resp = getattr(controller, meth)(req) self.assertEquals(resp.status_int, 404)