From b1764fdc8351fa653644bf01812adc6281348ab2 Mon Sep 17 00:00:00 2001 From: Zhongyue Luo Date: Thu, 31 Jan 2013 14:58:50 +0800 Subject: [PATCH] Fixes 'not in' operator usage Change-Id: Id4c83e32f6dcb4f710c5a8d8b6f130038cf07648 --- HACKING.rst | 19 +++++++++++++++++++ cinder/api/contrib/types_extra_specs.py | 2 +- cinder/api/openstack/__init__.py | 2 +- cinder/api/openstack/wsgi.py | 2 +- cinder/api/v1/snapshots.py | 2 +- cinder/api/v1/volumes.py | 2 +- cinder/api/v2/snapshots.py | 2 +- cinder/api/v2/volumes.py | 2 +- cinder/service.py | 2 +- cinder/tests/api/fakes.py | 2 +- cinder/tests/integrated/api/client.py | 2 +- cinder/tests/integrated/integrated_helpers.py | 2 +- cinder/tests/integrated/test_volumes.py | 2 +- cinder/tests/test_storwize_svc.py | 12 ++++++------ cinder/tests/windows/mockproxy.py | 2 +- cinder/volume/drivers/lvm.py | 2 +- cinder/volume/drivers/netapp.py | 2 +- cinder/volume/drivers/rbd.py | 2 +- cinder/volume/drivers/sheepdog.py | 2 +- 19 files changed, 42 insertions(+), 23 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index b67867ee995..eecb286b51f 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -27,6 +27,25 @@ General mylist = Foo().list() # OKAY, does not shadow built-in +- Use the "is not" operator when testing for unequal identities. Example:: + + if not X is Y: # BAD, intended behavior is ambiguous + pass + + if X is not Y: # OKAY, intuitive + pass + +- Use the "not in" operator for evaluating membership in a collection. Example:: + + if not X in Y: # BAD, intended behavior is ambiguous + pass + + if X not in Y: # OKAY, intuitive + pass + + if not (X in Y or X in Z): # OKAY, still better than all those 'not's + pass + Imports ------- diff --git a/cinder/api/contrib/types_extra_specs.py b/cinder/api/contrib/types_extra_specs.py index 5c298d95d4d..e3a4804309b 100644 --- a/cinder/api/contrib/types_extra_specs.py +++ b/cinder/api/contrib/types_extra_specs.py @@ -98,7 +98,7 @@ class VolumeTypeExtraSpecsController(wsgi.Controller): expl = _('Request body empty') raise webob.exc.HTTPBadRequest(explanation=expl) self._check_type(context, type_id) - if not id in body: + if id not in body: expl = _('Request body and URI mismatch') raise webob.exc.HTTPBadRequest(explanation=expl) if len(body) > 1: diff --git a/cinder/api/openstack/__init__.py b/cinder/api/openstack/__init__.py index ae2dfeb0717..4a6702d9d33 100644 --- a/cinder/api/openstack/__init__.py +++ b/cinder/api/openstack/__init__.py @@ -41,7 +41,7 @@ class APIMapper(routes.Mapper): class ProjectMapper(APIMapper): def resource(self, member_name, collection_name, **kwargs): - if not ('parent_resource' in kwargs): + if 'parent_resource' not in kwargs: kwargs['path_prefix'] = '{project_id}/' else: parent_resource = kwargs['parent_resource'] diff --git a/cinder/api/openstack/wsgi.py b/cinder/api/openstack/wsgi.py index 70b9e36a9c4..688fed86f7d 100644 --- a/cinder/api/openstack/wsgi.py +++ b/cinder/api/openstack/wsgi.py @@ -89,7 +89,7 @@ class Request(webob.Request): Does not do any body introspection, only checks header """ - if not "Content-Type" in self.headers: + if "Content-Type" not in self.headers: return None allowed_types = SUPPORTED_CONTENT_TYPES diff --git a/cinder/api/v1/snapshots.py b/cinder/api/v1/snapshots.py index 9685a095d23..ac044bacfc9 100644 --- a/cinder/api/v1/snapshots.py +++ b/cinder/api/v1/snapshots.py @@ -188,7 +188,7 @@ class SnapshotsController(wsgi.Controller): if not body: raise exc.HTTPUnprocessableEntity() - if not 'snapshot' in body: + if 'snapshot' not in body: raise exc.HTTPUnprocessableEntity() snapshot = body['snapshot'] diff --git a/cinder/api/v1/volumes.py b/cinder/api/v1/volumes.py index 2e7c8e8d01d..b695d78d0e9 100644 --- a/cinder/api/v1/volumes.py +++ b/cinder/api/v1/volumes.py @@ -378,7 +378,7 @@ class VolumeController(wsgi.Controller): if not body: raise exc.HTTPUnprocessableEntity() - if not 'volume' in body: + if 'volume' not in body: raise exc.HTTPUnprocessableEntity() volume = body['volume'] diff --git a/cinder/api/v2/snapshots.py b/cinder/api/v2/snapshots.py index 5853a0180c8..b8e00d93c4b 100644 --- a/cinder/api/v2/snapshots.py +++ b/cinder/api/v2/snapshots.py @@ -188,7 +188,7 @@ class SnapshotsController(wsgi.Controller): if not body: raise exc.HTTPUnprocessableEntity() - if not 'snapshot' in body: + if 'snapshot' not in body: raise exc.HTTPUnprocessableEntity() snapshot = body['snapshot'] diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py index c1d897fa57a..8f53b690540 100644 --- a/cinder/api/v2/volumes.py +++ b/cinder/api/v2/volumes.py @@ -302,7 +302,7 @@ class VolumeController(wsgi.Controller): if not body: raise exc.HTTPUnprocessableEntity() - if not 'volume' in body: + if 'volume' not in body: raise exc.HTTPUnprocessableEntity() volume = body['volume'] diff --git a/cinder/service.py b/cinder/service.py index d4fbfc2027c..8cbe0c252cf 100644 --- a/cinder/service.py +++ b/cinder/service.py @@ -351,7 +351,7 @@ class WSGIService(object): """ fl = '%s_manager' % self.name - if not fl in FLAGS: + if fl not in FLAGS: return None manager_class_name = FLAGS.get(fl, None) diff --git a/cinder/tests/api/fakes.py b/cinder/tests/api/fakes.py index e2bbe21e235..3fd21cfbaf5 100644 --- a/cinder/tests/api/fakes.py +++ b/cinder/tests/api/fakes.py @@ -185,6 +185,6 @@ class FakeRateLimiter(object): def get_fake_uuid(token=0): - if not token in FAKE_UUIDS: + if token not in FAKE_UUIDS: FAKE_UUIDS[token] = str(uuid.uuid4()) return FAKE_UUIDS[token] diff --git a/cinder/tests/integrated/api/client.py b/cinder/tests/integrated/api/client.py index 2df9ba91263..516a2a0e705 100644 --- a/cinder/tests/integrated/api/client.py +++ b/cinder/tests/integrated/api/client.py @@ -150,7 +150,7 @@ class TestOpenStackClient(object): LOG.debug(_("%(relative_uri)s => code %(http_status)s") % locals()) if check_response_status: - if not http_status in check_response_status: + if http_status not in check_response_status: if http_status == 404: raise OpenStackApiNotFoundException(response=response) elif http_status == 401: diff --git a/cinder/tests/integrated/integrated_helpers.py b/cinder/tests/integrated/integrated_helpers.py index f7ed84eea28..ead8bc1dd67 100644 --- a/cinder/tests/integrated/integrated_helpers.py +++ b/cinder/tests/integrated/integrated_helpers.py @@ -51,7 +51,7 @@ def generate_new_element(items, prefix, numeric=False): candidate = prefix + generate_random_numeric(8) else: candidate = prefix + generate_random_alphanumeric(8) - if not candidate in items: + if candidate not in items: return candidate LOG.debug("Random collision on %s" % candidate) diff --git a/cinder/tests/integrated/test_volumes.py b/cinder/tests/integrated/test_volumes.py index e239b9db208..1ba3d6ea0ec 100755 --- a/cinder/tests/integrated/test_volumes.py +++ b/cinder/tests/integrated/test_volumes.py @@ -72,7 +72,7 @@ class VolumesTest(integrated_helpers._IntegratedTestBase): self.assertEqual(volume_id, found_volume['id']) - if not found_volume['status'] in continue_states: + if found_volume['status'] not in continue_states: break time.sleep(1) diff --git a/cinder/tests/test_storwize_svc.py b/cinder/tests/test_storwize_svc.py index a2102ce8039..567dee6424e 100644 --- a/cinder/tests/test_storwize_svc.py +++ b/cinder/tests/test_storwize_svc.py @@ -391,7 +391,7 @@ class StorwizeSVCManagementSimulator: return self._errors["CMMVC5701E"] vol_name = kwargs["obj"].strip('\'\"') - if not vol_name in self._volumes_list: + if vol_name not in self._volumes_list: return self._errors["CMMVC5753E"] if force == 0: @@ -661,10 +661,10 @@ class StorwizeSVCManagementSimulator: return self._errors["CMMVC5707E"] mapping_info["vol"] = kwargs["obj"].strip('\'\"') - if not mapping_info["vol"] in self._volumes_list: + if mapping_info["vol"] not in self._volumes_list: return self._errors["CMMVC5753E"] - if not mapping_info["host"] in self._hosts_list: + if mapping_info["host"] not in self._hosts_list: return self._errors["CMMVC5754E"] if mapping_info["vol"] in self._mappings_list: @@ -689,7 +689,7 @@ class StorwizeSVCManagementSimulator: return self._errors["CMMVC5701E"] vol = kwargs["obj"].strip('\'\"') - if not vol in self._mappings_list: + if vol not in self._mappings_list: return self._errors["CMMVC5753E"] if self._mappings_list[vol]["host"] != host: @@ -729,13 +729,13 @@ class StorwizeSVCManagementSimulator: if "source" not in kwargs: return self._errors["CMMVC5707E"] source = kwargs["source"].strip('\'\"') - if not source in self._volumes_list: + if source not in self._volumes_list: return self._errors["CMMVC5754E"] if "target" not in kwargs: return self._errors["CMMVC5707E"] target = kwargs["target"].strip('\'\"') - if not target in self._volumes_list: + if target not in self._volumes_list: return self._errors["CMMVC5754E"] if source == target: diff --git a/cinder/tests/windows/mockproxy.py b/cinder/tests/windows/mockproxy.py index a612d470b66..d39a96300df 100644 --- a/cinder/tests/windows/mockproxy.py +++ b/cinder/tests/windows/mockproxy.py @@ -164,7 +164,7 @@ class MockProxy(object): l.append(val) def _add_recorded_value(self, name, val): - if not name in self._recorded_values: + if name not in self._recorded_values: self._recorded_values[name] = [] self._recorded_values[name].append(val) diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index 181eb6ceeb6..d3dc9a2285c 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -70,7 +70,7 @@ class LVMVolumeDriver(driver.VolumeDriver): out, err = self._execute('vgs', '--noheadings', '-o', 'name', run_as_root=True) volume_groups = out.split() - if not FLAGS.volume_group in volume_groups: + if FLAGS.volume_group not in volume_groups: exception_message = (_("volume group %s doesn't exist") % FLAGS.volume_group) raise exception.VolumeBackendAPIException(data=exception_message) diff --git a/cinder/volume/drivers/netapp.py b/cinder/volume/drivers/netapp.py index f674c3573c1..d3d1316a384 100644 --- a/cinder/volume/drivers/netapp.py +++ b/cinder/volume/drivers/netapp.py @@ -1306,7 +1306,7 @@ class NetAppCmodeISCSIDriver(driver.ISCSIDriver): def _get_lun_handle(self, name): """Get the details for a LUN from our cache table.""" - if not name in self.lun_table: + if name not in self.lun_table: LOG.warn(_("Could not find handle for LUN named %s") % name) return None return self.lun_table[name].handle diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index a4241aa551d..d5880b4aec8 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -55,7 +55,7 @@ class RBDDriver(driver.VolumeDriver): """Returns an error if prerequisites aren't met""" (stdout, stderr) = self._execute('rados', 'lspools') pools = stdout.split("\n") - if not FLAGS.rbd_pool in pools: + if FLAGS.rbd_pool not in pools: exception_message = (_("rbd has no pool %s") % FLAGS.rbd_pool) raise exception.VolumeBackendAPIException(data=exception_message) diff --git a/cinder/volume/drivers/sheepdog.py b/cinder/volume/drivers/sheepdog.py index 175686332f2..9fb39048239 100644 --- a/cinder/volume/drivers/sheepdog.py +++ b/cinder/volume/drivers/sheepdog.py @@ -37,7 +37,7 @@ class SheepdogDriver(driver.VolumeDriver): # gives short output, but for compatibility reason we won't # use it and just check if 'running' is in the output. (out, err) = self._execute('collie', 'cluster', 'info') - if not 'running' in out.split(): + if 'running' not in out.split(): exception_message = (_("Sheepdog is not working: %s") % out) raise exception.VolumeBackendAPIException( data=exception_message)