From 952fd4938d3ea6fff1d771b41f1ed4459ca9bf06 Mon Sep 17 00:00:00 2001 From: Swann Croiset Date: Thu, 11 Feb 2016 16:19:41 +0100 Subject: [PATCH] Add python syntax check using OpenStack hacking rules Change-Id: I42db1b87e216383009dbadc4fec4a1a7e580d898 --- .../modules/lma_collector/files/collectd/base.py | 15 +++++---------- .../files/collectd/build_ceph_perf_types.py | 10 +++++----- .../lma_collector/files/collectd/ceph_osd_perf.py | 6 +++--- .../files/collectd/ceph_pg_mon_status.py | 3 +-- .../files/collectd/check_openstack_api.py | 9 +++++---- .../files/collectd/elasticsearch_cluster.py | 1 + .../files/collectd/hypervisor_stats.py | 3 +-- .../lma_collector/files/collectd/influxdb.py | 3 ++- .../lma_collector/files/collectd/openstack.py | 9 +++++---- .../files/collectd/openstack_cinder.py | 2 +- .../lma_collector/files/collectd/rabbitmq_info.py | 10 ++++++---- test-requirements.txt | 2 ++ tox.ini | 14 +++++++++++++- 13 files changed, 50 insertions(+), 37 deletions(-) diff --git a/deployment_scripts/puppet/modules/lma_collector/files/collectd/base.py b/deployment_scripts/puppet/modules/lma_collector/files/collectd/base.py index a602c2c7b..6aaeb73ba 100644 --- a/deployment_scripts/puppet/modules/lma_collector/files/collectd/base.py +++ b/deployment_scripts/puppet/modules/lma_collector/files/collectd/base.py @@ -24,8 +24,7 @@ import collectd class Base(object): - """ Base class for writing Python plugins. - """ + """Base class for writing Python plugins.""" MAX_IDENTIFIER_LENGTH = 63 @@ -54,8 +53,7 @@ class Base(object): return def itermetrics(self): - """ - Iterate over the collected metrics + """Iterate over the collected metrics This class must be implemented by the subclass and should yield dict objects that represent the collected values. Each dict has 3 keys: @@ -96,8 +94,7 @@ class Base(object): v.dispatch() def execute(self, cmd, shell=True, cwd=None): - """ - Executes a program with arguments. + """Executes a program with arguments. Args: cmd: a list of program arguments where the first item is the @@ -150,8 +147,7 @@ class Base(object): return (stdout, stderr) def execute_to_json(self, *args, **kwargs): - """ - Executes a program and decodes the output as a JSON string. + """Executes a program and decodes the output as a JSON string. See execute(). @@ -165,8 +161,7 @@ class Base(object): @staticmethod def restore_sigchld(): - """ - Restores the SIGCHLD handler for Python <= v2.6. + """Restores the SIGCHLD handler for Python <= v2.6. This should be provided to collectd as the init callback by plugins that execute external programs. diff --git a/deployment_scripts/puppet/modules/lma_collector/files/collectd/build_ceph_perf_types.py b/deployment_scripts/puppet/modules/lma_collector/files/collectd/build_ceph_perf_types.py index bbb49f325..b7c0eb0f2 100755 --- a/deployment_scripts/puppet/modules/lma_collector/files/collectd/build_ceph_perf_types.py +++ b/deployment_scripts/puppet/modules/lma_collector/files/collectd/build_ceph_perf_types.py @@ -49,7 +49,7 @@ class CephPerfSchema(object): (stdout, stderr) = proc.communicate() stdout = stdout.rstrip('\n') except Exception as e: - print "Cannot execute command '%s': %s" % (cmd, str(e)) + print("Cannot execute command '%s': %s" % (cmd, str(e))) raise e return json.loads(stdout) @@ -69,15 +69,15 @@ class CephPerfSchema(object): def main(): script_name = os.path.basename(sys.argv[0]) if len(sys.argv) < 2 or len(sys.argv) > 3: - print "usage: %s [namespace]" % script_name + print("usage: %s [namespace]" % script_name) else: schema = CephPerfSchema(sys.argv[1]) collection = sys.argv[2] if len(sys.argv) == 3 else None - print "# File generated automatically by the %s script" % script_name - print "# Ceph version: %s" % schema.ceph_version() + print("# File generated automatically by the %s script" % script_name) + print("# Ceph version: %s" % schema.ceph_version()) for item in schema.itertypes(): if collection is None or item.collection == collection: - print item + print(item) if __name__ == '__main__': main() diff --git a/deployment_scripts/puppet/modules/lma_collector/files/collectd/ceph_osd_perf.py b/deployment_scripts/puppet/modules/lma_collector/files/collectd/ceph_osd_perf.py index f01975da1..6bab6d56d 100644 --- a/deployment_scripts/puppet/modules/lma_collector/files/collectd/ceph_osd_perf.py +++ b/deployment_scripts/puppet/modules/lma_collector/files/collectd/ceph_osd_perf.py @@ -24,8 +24,7 @@ RE_OSD_ID = re.compile(".*?osd\.(\d+)\.asok$") class CephOSDPerfPlugin(base.CephBase): - """ Collect OSD performance counters of all OSD daemons running on the host. - """ + """Collect OSD performance counters of OSD daemons running on the host.""" # Collect only metrics from the 'osd' namespace PREFIXES = ('osd') @@ -46,9 +45,10 @@ class CephOSDPerfPlugin(base.CephBase): @staticmethod def convert_to_collectd_value(value): + # See for details + # https://www.mail-archive.com/ceph-users@lists.ceph.com/msg18705.html if isinstance(value, dict): if value['avgcount'] > 0: - # See https://www.mail-archive.com/ceph-users@lists.ceph.com/msg18705.html return value['sum'] / value['avgcount'] else: return 0.0 diff --git a/deployment_scripts/puppet/modules/lma_collector/files/collectd/ceph_pg_mon_status.py b/deployment_scripts/puppet/modules/lma_collector/files/collectd/ceph_pg_mon_status.py index dba4238fc..23d19d084 100644 --- a/deployment_scripts/puppet/modules/lma_collector/files/collectd/ceph_pg_mon_status.py +++ b/deployment_scripts/puppet/modules/lma_collector/files/collectd/ceph_pg_mon_status.py @@ -26,8 +26,7 @@ HEALTH_MAP = { class CephMonPlugin(base.CephBase): - """ Collect states and information about ceph cluster and placement groups. - """ + """ Collect states and metrics about ceph cluster and placement groups.""" def __init__(self, *args, **kwargs): super(CephMonPlugin, self).__init__(*args, **kwargs) diff --git a/deployment_scripts/puppet/modules/lma_collector/files/collectd/check_openstack_api.py b/deployment_scripts/puppet/modules/lma_collector/files/collectd/check_openstack_api.py index 1262bdfac..b81aee8fd 100644 --- a/deployment_scripts/puppet/modules/lma_collector/files/collectd/check_openstack_api.py +++ b/deployment_scripts/puppet/modules/lma_collector/files/collectd/check_openstack_api.py @@ -24,13 +24,13 @@ INTERVAL = openstack.INTERVAL class APICheckPlugin(openstack.CollectdPlugin): - """ Class to check the status of OpenStack API services. - """ + """Class to check the status of OpenStack API services.""" + FAIL = 0 OK = 1 UNKNOWN = 2 - # TODO: sahara, murano + # TODO(all): sahara, murano CHECK_MAP = { 'keystone': { 'path': '/', 'expect': 300, 'name': 'keystone-public-api'}, @@ -67,7 +67,8 @@ class APICheckPlugin(openstack.CollectdPlugin): for service in catalog: name = service['name'] if name not in self.CHECK_MAP: - self.logger.notice("No check found for service '%s', skipping it" % name) + self.logger.notice( + "No check found for service '%s', skipping it" % name) status = self.UNKNOWN else: check = self.CHECK_MAP[name] diff --git a/deployment_scripts/puppet/modules/lma_collector/files/collectd/elasticsearch_cluster.py b/deployment_scripts/puppet/modules/lma_collector/files/collectd/elasticsearch_cluster.py index 8f5a5c7cd..a3cbdcd10 100644 --- a/deployment_scripts/puppet/modules/lma_collector/files/collectd/elasticsearch_cluster.py +++ b/deployment_scripts/puppet/modules/lma_collector/files/collectd/elasticsearch_cluster.py @@ -30,6 +30,7 @@ METRICS = ['number_of_nodes', 'active_primary_shards', 'active_primary_shards', HEALTH_ON_ERROR = {'type_instance': 'health', 'values': HEALTH_MAP['red']} + class ElasticsearchClusterHealthPlugin(base.Base): def __init__(self, *args, **kwargs): super(ElasticsearchClusterHealthPlugin, self).__init__(*args, **kwargs) diff --git a/deployment_scripts/puppet/modules/lma_collector/files/collectd/hypervisor_stats.py b/deployment_scripts/puppet/modules/lma_collector/files/collectd/hypervisor_stats.py index b627dd9f8..096716606 100644 --- a/deployment_scripts/puppet/modules/lma_collector/files/collectd/hypervisor_stats.py +++ b/deployment_scripts/puppet/modules/lma_collector/files/collectd/hypervisor_stats.py @@ -22,8 +22,7 @@ INTERVAL = openstack.INTERVAL class HypervisorStatsPlugin(openstack.CollectdPlugin): - """ Class to report the statistics on Nova hypervisors. - """ + """ Class to report the statistics on Nova hypervisors.""" VALUE_MAP = { 'current_workload': 'running_tasks', 'running_vms': 'running_instances', diff --git a/deployment_scripts/puppet/modules/lma_collector/files/collectd/influxdb.py b/deployment_scripts/puppet/modules/lma_collector/files/collectd/influxdb.py index 1b9e6309a..2657e730f 100644 --- a/deployment_scripts/puppet/modules/lma_collector/files/collectd/influxdb.py +++ b/deployment_scripts/puppet/modules/lma_collector/files/collectd/influxdb.py @@ -21,7 +21,8 @@ import requests NAME = 'influxdb' METRICS_BY_NAME = { 'cluster': { - 'writeShardPointsReq': ('cluster_write_shard_points_requests', 'gauge'), + 'writeShardPointsReq': ('cluster_write_shard_points_requests', + 'gauge'), 'writeShardReq': ('cluster_write_shard_requests', 'gauge')}, 'httpd': { diff --git a/deployment_scripts/puppet/modules/lma_collector/files/collectd/openstack.py b/deployment_scripts/puppet/modules/lma_collector/files/collectd/openstack.py index efb6b5106..fe157cd1d 100644 --- a/deployment_scripts/puppet/modules/lma_collector/files/collectd/openstack.py +++ b/deployment_scripts/puppet/modules/lma_collector/files/collectd/openstack.py @@ -49,8 +49,10 @@ class OSClient(object): # but not on timeout and backoff time is not supported. # (at this time we ship requests 2.2.1 and urllib3 1.6.1 or 1.7.1) self.session = requests.Session() - self.session.mount('http://', requests.adapters.HTTPAdapter(max_retries=max_retries)) - self.session.mount('https://', requests.adapters.HTTPAdapter(max_retries=max_retries)) + self.session.mount( + 'http://', requests.adapters.HTTPAdapter(max_retries=max_retries)) + self.session.mount( + 'https://', requests.adapters.HTTPAdapter(max_retries=max_retries)) self.get_token() @@ -72,10 +74,9 @@ class OSClient(object): { 'username': self.username, 'password': self.password - } } } - ) + }) self.logger.info("Trying to get token from '%s'" % self.keystone_url) r = self.make_request('post', '%s/tokens' % self.keystone_url, data=data, diff --git a/deployment_scripts/puppet/modules/lma_collector/files/collectd/openstack_cinder.py b/deployment_scripts/puppet/modules/lma_collector/files/collectd/openstack_cinder.py index 52d28e89c..5c24aa72f 100644 --- a/deployment_scripts/puppet/modules/lma_collector/files/collectd/openstack_cinder.py +++ b/deployment_scripts/puppet/modules/lma_collector/files/collectd/openstack_cinder.py @@ -39,7 +39,7 @@ class CinderStatsPlugin(openstack.CollectdPlugin): return d.get('status', 'unknown').lower() def count_size_bytes(d): - return d.get('size', 0) * 10**9 + return d.get('size', 0) * 10 ** 9 status = self.count_objects_group_by(volumes_details, group_by_func=groupby) diff --git a/deployment_scripts/puppet/modules/lma_collector/files/collectd/rabbitmq_info.py b/deployment_scripts/puppet/modules/lma_collector/files/collectd/rabbitmq_info.py index 48753ca4f..3aebe1a6c 100644 --- a/deployment_scripts/puppet/modules/lma_collector/files/collectd/rabbitmq_info.py +++ b/deployment_scripts/puppet/modules/lma_collector/files/collectd/rabbitmq_info.py @@ -92,7 +92,7 @@ class RabbitMqPlugin(base.Base): mem_str = re.findall('{memory,\s+\[([^\]]+)\]\}', out) # We are only interested by the total of memory used - # TODO: Get all informations about memory usage from mem_str + # TODO(all): Get all informations about memory usage from mem_str try: stats['used_memory'] = int(re.findall('total,([0-9]+)', mem_str[0])[0]) @@ -101,9 +101,11 @@ class RabbitMqPlugin(base.Base): self.rabbitmqctl_bin) if 'vm_memory_limit' in stats and 'used_memory' in stats: - stats['remaining_memory'] = stats['vm_memory_limit'] - stats['used_memory'] + stats['remaining_memory'] = \ + stats['vm_memory_limit'] - stats['used_memory'] if 'disk_free' in stats and 'disk_free_limit' in stats: - stats['remaining_disk'] = stats['disk_free'] - stats['disk_free_limit'] + stats['remaining_disk'] = \ + stats['disk_free'] - stats['disk_free_limit'] out, err = self.execute([self.rabbitmqctl_bin, '-q', 'cluster_status'], shell=False) @@ -112,7 +114,7 @@ class RabbitMqPlugin(base.Base): self.rabbitmqctl_bin) return - # TODO: Need to be modified in case we are using RAM nodes. + # TODO(all): Need to be modified in case we are using RAM nodes. status = CLUSTER_STATUS.findall(out) if len(status) == 0: self.logger.error('%s: Failed to parse (%s)' % diff --git a/test-requirements.txt b/test-requirements.txt index b6e4a81f6..40cc1c6e4 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,2 +1,4 @@ Sphinx -e git+https://github.com/openstack/fuel-plugins.git#egg=fuel-plugin-builder +# Hacking already pins down pep8, pyflakes and flake8 +hacking<0.11,>=0.10.0 diff --git a/tox.ini b/tox.ini index 0f43e4ae0..b8aab3340 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = manifests,heka,lma_collector,docs,qa_docs,build_plugin +envlist = manifests,heka,lma_collector,docs,qa_docs,build_plugin,collectd_python skipsdist = True [testenv] @@ -39,6 +39,18 @@ commands = bundle install --path {toxinidir}/.bundled_gems bundle exec rake test +[flake8] +ignore = H105,H201,E241,H401 +exclude = haproxy.py +show-source = True + +[testenv:collectd_python] +changedir = {toxinidir}/deployment_scripts/puppet/modules/lma_collector/files/collectd +whitelist_externals = + flake8 +commands = + flake8 . + [testenv:docs] changedir = {toxinidir}/doc whitelist_externals = make