From b9d97599e4e77738ad2b41960fc6d75029c4f50e Mon Sep 17 00:00:00 2001 From: Dina Belova Date: Tue, 29 Apr 2014 18:24:25 +0400 Subject: [PATCH] Use None instead of mutables in method params defaults Mutables in the method params defaults might cause errors and that's why it's anti-pattern in most of the cases and should be removed. Change-Id: Ifb59a16c927a4bc285f7b70fac87f719054020f4 --- ceilometer/agent.py | 5 +- ceilometer/api/controllers/v2.py | 52 ++++++++++++++----- ceilometer/compute/pollsters/util.py | 3 +- ceilometer/energy/kwapi.py | 4 +- ceilometer/hardware/plugin.py | 3 +- ceilometer/image/glance.py | 4 +- ceilometer/network/floatingip.py | 2 +- ceilometer/network/statistics/__init__.py | 3 +- ceilometer/objectstore/swift.py | 10 ++-- ceilometer/pipeline.py | 3 +- ceilometer/plugin.py | 2 +- ceilometer/storage/__init__.py | 8 +-- ceilometer/storage/base.py | 12 ++--- ceilometer/storage/impl_db2.py | 7 ++- ceilometer/storage/impl_hbase.py | 17 ++++-- ceilometer/storage/impl_log.py | 4 +- ceilometer/storage/impl_mongodb.py | 13 +++-- ceilometer/storage/impl_sqlalchemy.py | 8 ++- ceilometer/storage/pymongo_base.py | 4 +- ceilometer/tests/agentbase.py | 6 ++- ceilometer/tests/api/__init__.py | 4 +- ceilometer/tests/api/v2/test_acl_scenarios.py | 4 +- ceilometer/tests/api/v2/test_complex_query.py | 4 +- ceilometer/transformer/conversions.py | 4 +- 24 files changed, 121 insertions(+), 65 deletions(-) diff --git a/ceilometer/agent.py b/ceilometer/agent.py index 789153fc20..a407a8bdff 100644 --- a/ceilometer/agent.py +++ b/ceilometer/agent.py @@ -99,8 +99,9 @@ class PollingTask(object): class AgentManager(os_service.Service): - def __init__(self, namespace, default_discovery=[]): + def __init__(self, namespace, default_discovery=None): super(AgentManager, self).__init__() + default_discovery = default_discovery or [] self.default_discovery = default_discovery self.pollster_manager = self._extensions('poll', namespace) self.discovery_manager = self._extensions('discover') @@ -156,7 +157,7 @@ class AgentManager(os_service.Service): return d.obj return None - def discover(self, discovery=[]): + def discover(self, discovery=None): resources = [] for url in (discovery or self.default_discovery): name, param = self._parse_discoverer(url) diff --git a/ceilometer/api/controllers/v2.py b/ceilometer/api/controllers/v2.py index c7bb614135..dbe870825a 100644 --- a/ceilometer/api/controllers/v2.py +++ b/ceilometer/api/controllers/v2.py @@ -332,7 +332,7 @@ def _verify_query_segregation(query, auth_project=None): raise ProjectNotAuthorized(q.value) -def _validate_query(query, db_func, internal_keys=[], +def _validate_query(query, db_func, internal_keys=None, allow_timestamps=True): """Validates the syntax of the query and verifies that the query request is authorized for the included project. @@ -356,6 +356,7 @@ def _validate_query(query, db_func, internal_keys=[], """ + internal_keys = internal_keys or [] _verify_query_segregation(query) valid_keys = inspect.getargspec(db_func)[0] @@ -446,8 +447,9 @@ def _validate_timestamp_fields(query, field_name, operator_list, return False -def _query_to_kwargs(query, db_func, internal_keys=[], +def _query_to_kwargs(query, db_func, internal_keys=None, allow_timestamps=True): + internal_keys = internal_keys or [] _validate_query(query, db_func, internal_keys=internal_keys, allow_timestamps=allow_timestamps) query = _sanitize_query(query, db_func) @@ -519,7 +521,7 @@ def _validate_groupby_fields(groupby_fields): return list(set(groupby_fields)) -def _get_query_timestamps(args={}): +def _get_query_timestamps(args=None): """Return any optional timestamp information in the request. Determine the desired range, if any, from the GET arguments. Set @@ -536,6 +538,12 @@ def _get_query_timestamps(args={}): search_offset: search_offset parameter from request """ + if args is None: + return {'query_start': None, + 'query_end': None, + 'start_timestamp': None, + 'end_timestamp': None, + 'search_offset': 0} search_offset = int(args.get('search_offset', 0)) start_timestamp = args.get('start_timestamp') @@ -645,8 +653,9 @@ class OldSample(_Base): message_id = wtypes.text "A unique identifier for the sample" - def __init__(self, counter_volume=None, resource_metadata={}, + def __init__(self, counter_volume=None, resource_metadata=None, timestamp=None, **kwds): + resource_metadata = resource_metadata or {} if counter_volume is not None: counter_volume = float(counter_volume) resource_metadata = _flatten_metadata(resource_metadata) @@ -810,12 +819,13 @@ class MeterController(rest.RestController): self.meter_name = meter_name @wsme_pecan.wsexpose([OldSample], [Query], int) - def get_all(self, q=[], limit=None): + def get_all(self, q=None, limit=None): """Return samples for the meter. :param q: Filter rules for the data to be returned. :param limit: Maximum number of samples to return. """ + q = q or [] if limit and limit < 0: raise ClientSideError(_("Limit must be positive")) kwargs = _query_to_kwargs(q, storage.SampleFilter.__init__) @@ -886,7 +896,7 @@ class MeterController(rest.RestController): return samples @wsme_pecan.wsexpose([Statistics], [Query], [unicode], int, [Aggregate]) - def statistics(self, q=[], groupby=[], period=None, aggregate=[]): + def statistics(self, q=None, groupby=None, period=None, aggregate=None): """Computes the statistics of the samples in the time range given. :param q: Filter rules for the data to be returned. @@ -895,6 +905,10 @@ class MeterController(rest.RestController): period long of that number of seconds. :param aggregate: The selectable aggregation functions to be applied. """ + q = q or [] + groupby = groupby or [] + aggregate = aggregate or [] + if period and period < 0: raise ClientSideError(_("Period must be positive.")) @@ -979,11 +993,13 @@ class MetersController(rest.RestController): return MeterController(meter_name), remainder @wsme_pecan.wsexpose([Meter], [Query]) - def get_all(self, q=[]): + def get_all(self, q=None): """Return all known meters, based on the data recorded so far. :param q: Filter rules for the meters to be returned. """ + q = q or [] + #Timestamp field is not supported for Meter queries kwargs = _query_to_kwargs(q, pecan.request.storage_conn.get_meters, allow_timestamps=False) @@ -1067,12 +1083,14 @@ class SamplesController(rest.RestController): """Controller managing the samples.""" @wsme_pecan.wsexpose([Sample], [Query], int) - def get_all(self, q=[], limit=None): + def get_all(self, q=None, limit=None): """Return all known samples, based on the data recorded so far. :param q: Filter rules for the samples to be returned. :param limit: Maximum number of samples to be returned. """ + q = q or [] + if limit and limit < 0: raise ClientSideError(_("Limit must be positive")) kwargs = _query_to_kwargs(q, storage.SampleFilter.__init__) @@ -1145,8 +1163,9 @@ class ValidatedComplexQuery(object): timestamp_fields = ["timestamp", "state_timestamp"] - def __init__(self, query, db_model, additional_name_mapping={}, + def __init__(self, query, db_model, additional_name_mapping=None, metadata_allowed=False): + additional_name_mapping = additional_name_mapping or {} self.name_mapping = {"user": "user_id", "project": "project_id"} self.name_mapping.update(additional_name_mapping) @@ -1404,7 +1423,8 @@ class Resource(_Base): source = wtypes.text "The source where the resource come from" - def __init__(self, metadata={}, **kwds): + def __init__(self, metadata=None, **kwds): + metadata = metadata or {} metadata = _flatten_metadata(metadata) super(Resource, self).__init__(metadata=metadata, **kwds) @@ -1457,12 +1477,13 @@ class ResourcesController(rest.RestController): self._resource_links(resource_id)) @wsme_pecan.wsexpose([Resource], [Query], int) - def get_all(self, q=[], meter_links=1): + def get_all(self, q=None, meter_links=1): """Retrieve definitions of all of the resources. :param q: Filter rules for the resources to be returned. :param meter_links: option to include related meter links """ + q = q or [] kwargs = _query_to_kwargs(q, pecan.request.storage_conn.get_resources) resources = [ Resource.from_db_and_links(r, @@ -1974,11 +1995,12 @@ class AlarmController(rest.RestController): # TODO(eglynn): add pagination marker to signature once overall # API support for pagination is finalized @wsme_pecan.wsexpose([AlarmChange], [Query]) - def history(self, q=[]): + def history(self, q=None): """Assembles the alarm history requested. :param q: Filter rules for the changes to be described. """ + q = q or [] # allow history to be returned for deleted alarms, but scope changes # returned to those carried out on behalf of the auth'd tenant, to # avoid inappropriate cross-tenant visibility of alarm history @@ -2105,11 +2127,12 @@ class AlarmsController(rest.RestController): return Alarm.from_db_model(alarm) @wsme_pecan.wsexpose([Alarm], [Query]) - def get_all(self, q=[]): + def get_all(self, q=None): """Return all alarms, based on the query provided. :param q: Filter rules for the alarms to be returned. """ + q = q or [] #Timestamp is not supported field for Simple Alarm queries kwargs = _query_to_kwargs(q, pecan.request.storage_conn.get_alarms, @@ -2310,11 +2333,12 @@ class EventsController(rest.RestController): @requires_admin @wsme_pecan.wsexpose([Event], [EventQuery]) - def get_all(self, q=[]): + def get_all(self, q=None): """Return all events matching the query filters. :param q: Filter arguments for which Events to return """ + q = q or [] event_filter = _event_query_to_event_filter(q) return [Event(message_id=event.message_id, event_type=event.event_type, diff --git a/ceilometer/compute/pollsters/util.py b/ceilometer/compute/pollsters/util.py index a8aea437be..ed60bf6656 100644 --- a/ceilometer/compute/pollsters/util.py +++ b/ceilometer/compute/pollsters/util.py @@ -77,7 +77,8 @@ def _get_metadata_from_object(instance): def make_sample_from_instance(instance, name, type, unit, volume, - additional_metadata={}): + additional_metadata=None): + additional_metadata = additional_metadata or {} resource_metadata = _get_metadata_from_object(instance) resource_metadata.update(additional_metadata) return sample.Sample( diff --git a/ceilometer/energy/kwapi.py b/ceilometer/energy/kwapi.py index 9a9479e7b5..069ccc718d 100644 --- a/ceilometer/energy/kwapi.py +++ b/ceilometer/energy/kwapi.py @@ -82,7 +82,7 @@ class _Base(plugin.CentralPollster): class EnergyPollster(_Base): """Measures energy consumption.""" - def get_samples(self, manager, cache, resources=[]): + def get_samples(self, manager, cache, resources=None): """Returns all samples.""" for probe in self._iter_probes(manager.keystone, cache): yield sample.Sample( @@ -102,7 +102,7 @@ class EnergyPollster(_Base): class PowerPollster(_Base): """Measures power consumption.""" - def get_samples(self, manager, cache, resources=[]): + def get_samples(self, manager, cache, resources=None): """Returns all samples.""" for probe in self._iter_probes(manager.keystone, cache): yield sample.Sample( diff --git a/ceilometer/hardware/plugin.py b/ceilometer/hardware/plugin.py index 243cf4d358..b62e53b1d0 100644 --- a/ceilometer/hardware/plugin.py +++ b/ceilometer/hardware/plugin.py @@ -44,13 +44,14 @@ class HardwarePollster(plugin.CentralPollster): super(HardwarePollster, self).__init__() self.inspectors = {} - def get_samples(self, manager, cache, resources=[]): + def get_samples(self, manager, cache, resources=None): """Return an iterable of Sample instances from polling the resources. :param manager: The service manager invoking the plugin :param cache: A dictionary for passing data between plugins :param resources: end point to poll data from """ + resources = resources or [] h_cache = cache.setdefault(self.CACHE_KEY, {}) sample_iters = [] for res in resources: diff --git a/ceilometer/image/glance.py b/ceilometer/image/glance.py index 54c263d666..20c3b9a986 100644 --- a/ceilometer/image/glance.py +++ b/ceilometer/image/glance.py @@ -104,7 +104,7 @@ class _Base(plugin.PollsterBase): class ImagePollster(_Base): - def get_samples(self, manager, cache, resources=[]): + def get_samples(self, manager, cache, resources=None): for image in self._iter_images(manager.keystone, cache): yield sample.Sample( name='image', @@ -121,7 +121,7 @@ class ImagePollster(_Base): class ImageSizePollster(_Base): - def get_samples(self, manager, cache, resources=[]): + def get_samples(self, manager, cache, resources=None): for image in self._iter_images(manager.keystone, cache): yield sample.Sample( name='image.size', diff --git a/ceilometer/network/floatingip.py b/ceilometer/network/floatingip.py index d8e429e627..64ebd44f31 100644 --- a/ceilometer/network/floatingip.py +++ b/ceilometer/network/floatingip.py @@ -40,7 +40,7 @@ class FloatingIPPollster(plugin.CentralPollster): cache['floating_ips'] = list(self._get_floating_ips()) return iter(cache['floating_ips']) - def get_samples(self, manager, cache, resources=[]): + def get_samples(self, manager, cache, resources=None): for ip in self._iter_floating_ips(cache): self.LOG.info(_("FLOATING IP USAGE: %s") % ip.ip) # FIXME (flwang) Now Nova API /os-floating-ips can't provide those diff --git a/ceilometer/network/statistics/__init__.py b/ceilometer/network/statistics/__init__.py index 29df6e9931..3b8dc2bb5a 100644 --- a/ceilometer/network/statistics/__init__.py +++ b/ceilometer/network/statistics/__init__.py @@ -63,7 +63,8 @@ class _Base(plugin.CentralPollster): scheme).driver() return _Base.drivers[scheme] - def get_samples(self, manager, cache, resources=[]): + def get_samples(self, manager, cache, resources=None): + resources = resources or [] for resource in resources: parse_url, params = self._parse_my_resource(resource) ext = self.get_driver(parse_url.scheme) diff --git a/ceilometer/objectstore/swift.py b/ceilometer/objectstore/swift.py index abfc75c385..99a2b19363 100644 --- a/ceilometer/objectstore/swift.py +++ b/ceilometer/objectstore/swift.py @@ -88,7 +88,7 @@ class ObjectsPollster(_Base): """Iterate over all accounts, using keystone. """ - def get_samples(self, manager, cache, resources=[]): + def get_samples(self, manager, cache, resources=None): for tenant, account in self._iter_accounts(manager.keystone, cache): yield sample.Sample( name='storage.objects', @@ -107,7 +107,7 @@ class ObjectsSizePollster(_Base): """Iterate over all accounts, using keystone. """ - def get_samples(self, manager, cache, resources=[]): + def get_samples(self, manager, cache, resources=None): for tenant, account in self._iter_accounts(manager.keystone, cache): yield sample.Sample( name='storage.objects.size', @@ -126,7 +126,7 @@ class ObjectsContainersPollster(_Base): """Iterate over all accounts, using keystone. """ - def get_samples(self, manager, cache, resources=[]): + def get_samples(self, manager, cache, resources=None): for tenant, account in self._iter_accounts(manager.keystone, cache): yield sample.Sample( name='storage.objects.containers', @@ -147,7 +147,7 @@ class ContainersObjectsPollster(_Base): METHOD = 'get' - def get_samples(self, manager, cache, resources=[]): + def get_samples(self, manager, cache, resources=None): for project, account in self._iter_accounts(manager.keystone, cache): containers_info = account[1] for container in containers_info: @@ -170,7 +170,7 @@ class ContainersSizePollster(_Base): METHOD = 'get' - def get_samples(self, manager, cache, resources=[]): + def get_samples(self, manager, cache, resources=None): for project, account in self._iter_accounts(manager.keystone, cache): containers_info = account[1] for container in containers_info: diff --git a/ceilometer/pipeline.py b/ceilometer/pipeline.py index 2b1c265011..f7d2828fc8 100644 --- a/ceilometer/pipeline.py +++ b/ceilometer/pipeline.py @@ -55,7 +55,8 @@ class PipelineException(Exception): class PublishContext(object): - def __init__(self, context, pipelines=[]): + def __init__(self, context, pipelines=None): + pipelines = pipelines or [] self.pipelines = set(pipelines) self.context = context diff --git a/ceilometer/plugin.py b/ceilometer/plugin.py index 7c443abe37..7e7e4db4b7 100644 --- a/ceilometer/plugin.py +++ b/ceilometer/plugin.py @@ -130,7 +130,7 @@ class PollsterBase(PluginBase): """Base class for plugins that support the polling API.""" @abc.abstractmethod - def get_samples(self, manager, cache, resources=[]): + def get_samples(self, manager, cache, resources=None): """Return a sequence of Counter instances from polling the resources. :param manager: The service manager class invoking the plugin. diff --git a/ceilometer/storage/__init__.py b/ceilometer/storage/__init__.py index 1aa1e0ef7c..eefa5985bb 100644 --- a/ceilometer/storage/__init__.py +++ b/ceilometer/storage/__init__.py @@ -104,7 +104,7 @@ class SampleFilter(object): end=None, end_timestamp_op=None, resource=None, meter=None, source=None, message_id=None, - metaquery={}): + metaquery=None): self.user = user self.project = project self.start = utils.sanitize_timestamp(start) @@ -114,7 +114,7 @@ class SampleFilter(object): self.resource = resource self.meter = meter self.source = source - self.metaquery = metaquery + self.metaquery = metaquery or {} self.message_id = message_id @@ -137,12 +137,12 @@ class EventFilter(object): """ def __init__(self, start_time=None, end_time=None, event_type=None, - message_id=None, traits_filter=[]): + message_id=None, traits_filter=None): self.start_time = utils.sanitize_timestamp(start_time) self.end_time = utils.sanitize_timestamp(end_time) self.message_id = message_id self.event_type = event_type - self.traits_filter = traits_filter + self.traits_filter = traits_filter or [] def __repr__(self): return ("