openstack
/
trove
Code Issues Proposed changes

initial chkin of pylint 

this is a pylint wrapper for Trove's tox tests. This commit includes a
basic infrastructure for running pylint through tox.

It also fixes some very obvious import errors that are flagged by the
tool. One is to handle missing imports for _ and _LE.

There is one instance where an exception is being raised but
trove.common.exception isn't imported, and another where an exception
is being incorrectly thrown.

A short readme is also provided.

Change-Id: I0a38f5efde3cb491f1f6c27f6c6500ab29987968
Partial-Bug: #1621636
This commit is contained in:
Amrith Kumar 2016-09-09 09:33:49 -04:00
parent 92a78ea5e5
commit a0bc0dcb7d
15 changed files with 3036 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
pylintrc Normal file
View File

@ -0,0 +1,7 @@
# pylintrc
#
# For trove we use the defaults, this file is just to shut up an
# annoying error message from pylint.
#
# Don't set pylint options here.
#

2
test-requirements.txt
View File

@ -28,3 +28,5 @@ cassandra-driver!=3.6.0,>=2.1.4 # Apache-2.0
pycrypto>=2.6 # Public Domain
couchdb>=0.8 # Apache-2.0
os-testr>=0.7.0 # Apache-2.0
astroid<1.4.0 # LGPLv2.1 # breaks pylint 1.4.4
pylint==1.4.5 # GPLv2

167
tools/trove-pylint.README Normal file
View File

@ -0,0 +1,167 @@
trove-pylint
------------
trove-pylint.py is a wrapper around pylint which allows for some
custom processing relevant to the trove source tree, and suitable to
run as a CI job for trove.
The purpose is to perform a lint check on the code and detect obvious
(lintable) errors and fix them.
How trove-pylint works
----------------------
trove-pylint is driven by a configuration file which is by default,
located in tools/trove-pylint.config. This file is a json dump of the
configuration. A default configuraiton file looks like this.
{
"include": ["*.py"],
"folder": "trove",
"options": ["--rcfile=./pylintrc", "-E"],
"ignored_files": ['trove/tests'],
"ignored_codes": [],
"ignored_messages": [],
"ignored_file_codes": [],
"ignored_file_messages": [],
"ignored_file_code_messages": [],
"always_error_messages": [
"Undefined variable '_'",
"Undefined variable '_LE'",
"Undefined variable '_LI'",
"Undefined variable '_LW'",
"Undefined variable '_LC'"
]
}
include
-------
Provide a list of match specs (passed to fnmatch.fnmatch). The
default is only "*.py".
folder
------
Provide the name of the top level folder to lint. This is a single
value.
options
-------
These are the pylint launch options. The default is to specify an
rcfile and only errors. Specifying the rcfile is required, and the
file is a dummy, to suppress an annoying warning.
ignored_files
-------------
This is a list of paths that we wish to ignore. When a file is
considered for linting, if the path name begins with any of these
provided prefixes, the file will not be linted. We ignore the
tests directory because it has a high instance of false positives.
ignored_codes, ignored_messages, ignored_file_codes,
ignored_file_messages, and ignored_file_code_messages
-----------------------------------------------------
These settings identify specific failures that are to be
ignored. Each is a list, some are lists of single elements, others
are lists of lists.
ignored_codes, and ignored_messages are lists of single elements
that are to be ignored. You could specify either the code name, or
the code numeric representation. You must specify the exact
message.
ignored_file_codes and ignored_file_messages are lists of lists
where each element is a code and a message.
ignored_file_code_messages is a list of lists where each element
consists of a filename, an errorcode, a message, a line number and
a function name.
always_error_messages
---------------------
This is a list of messages which have a low chance of false
positives, which are always flagged as errors.
Using trove-pylint
------------------
You can check your code for errors by simply running:
tox -e pylint
The equivalent result can be obtained by running the command:
tools/trove-pylint.py
or
tools/trove-pylint.py check
For example, here is the result from such a run.
$ tools/trove-pylint.py check
ERROR: trove/common/extensions.py 575: E1003 bad-super-call, \
TroveExtensionMiddleware.__init__: Bad first argument \
'ExtensionMiddleware' given to super()
Check failed. 367 files processed, 1 had errors, 1 errors recorded.
I wish to ignore this error and keep going. To do this, I rebuild the
list of errors to ignore as follows.
$ tools/trove-pylint.py rebuild
Rebuild completed. 367 files processed, 177 exceptions recorded.
This caused the tool to add the following two things to the config file.
[
"trove/common/extensions.py",
"E1003",
"Bad first argument 'ExtensionMiddleware' given to super()",
"575",
"TroveExtensionMiddleware.__init__"
],
[
"trove/common/extensions.py",
"bad-super-call",
"Bad first argument 'ExtensionMiddleware' given to super()",
"575",
"TroveExtensionMiddleware.__init__"
],
With that done, I can recheck as shown below.
$ tools/trove-pylint.py check
Check succeeded. 367 files processed
You can review the errors that are being currently ignored by reading
the file tools/trove-pylint.config.
If you want to fix some of these errors, identify the configuration(s)
that are causing those errors to be ignored and re-run the check. Once
you see that the errors are in fact being reported by the tool, go
ahead and fix the problem(s) and retest.
Known issues
------------
1. The tool appears to be very sensitive to the version(s) of pylint
and astroid. In testing, I've found that if the version of either of
these changes, you could either have a failure of the tool (exceptions
thrown, ...) or a different set of errors reported.
Currently, test-requirements.txt sets these versions in this way.
astroid<1.4.0 # LGPLv2.1 # breaks pylint 1.4.4
pylint==1.4.5 # GPLv2
If you run the tool on your machine and find that there are no errors,
but find that either the CI generates errors, or that the tool run
through tox generates errors, check what versions of astroid and
pylint are being run in each configuration.

2504
tools/trove-pylint.config Normal file
View File

File diff suppressed because it is too large Load Diff

339
tools/trove-pylint.py Executable file
View File

@ -0,0 +1,339 @@
#!/usr/bin/env python
# Copyright 2016 Tesora, Inc.
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from __future__ import print_function
import fnmatch
import json
import os
import re
import sys
from pylint import lint
from pylint.reporters import text
from six.moves import cStringIO as csio
DEFAULT_CONFIG_FILE = "tools/trove-pylint.config"
DEFAULT_IGNORED_FILES = ['trove/tests']
DEFAULT_IGNORED_CODES = []
DEFAULT_IGNORED_MESSAGES = []
DEFAULT_ALWAYS_ERROR = [
"Undefined variable '_'",
"Undefined variable '_LE'",
"Undefined variable '_LI'",
"Undefined variable '_LW'",
"Undefined variable '_LC'"]
MODE_CHECK = "check"
MODE_REBUILD = "rebuild"
class Config(object):
def __init__(self, filename=DEFAULT_CONFIG_FILE):
self.default_config = {
"include": ["*.py"],
"folder": "trove",
"options": ["--rcfile=./pylintrc", "-E"],
"ignored_files": DEFAULT_IGNORED_FILES,
"ignored_codes": DEFAULT_IGNORED_CODES,
"ignored_messages": DEFAULT_IGNORED_MESSAGES,
"ignored_file_codes": [],
"ignored_file_messages": [],
"ignored_file_code_messages": [],
"always_error_messages": DEFAULT_ALWAYS_ERROR
}
self.config = self.default_config
def save(self, filename=DEFAULT_CONFIG_FILE):
if os.path.isfile(filename):
os.rename(filename, "%s~" % filename)
with open(filename, 'w') as fp:
json.dump(self.config, fp, encoding="utf-8",
indent=2, separators=(',', ': '))
def load(self, filename=DEFAULT_CONFIG_FILE):
self.config = self.default_config
try:
with open(filename) as fp:
_c = json.load(fp, encoding="utf-8")
self.config = _c
except Exception:
print("An error occured loading configuration, using default.")
return self
def get(self, attribute):
return self.config[attribute]
def is_file_ignored(self, f):
if any(f.startswith(i)
for i in self.config['ignored_files']):
return True
return False
def is_file_included(self, f):
if any(fnmatch.fnmatch(f, wc) for wc in self.config['include']):
return True
return False
def is_always_error(self, message):
if message in self.config['always_error_messages']:
return True
return False
def ignore(self, filename, code, codename, message):
# the high priority checks
if self.is_file_ignored(filename):
return True
# never ignore messages
if self.is_always_error(message):
return False
if code in self.config['ignored_codes']:
return True
if codename in self.config['ignored_codes']:
return True
if message and any(message.startswith(ignore_message)
for ignore_message
in self.config['ignored_messages']):
return True
if filename and message and (
[filename, message] in self.config['ignored_file_messages']):
return True
if filename and code and (
[filename, code] in self.config['ignored_file_codes']):
return True
if filename and codename and (
[filename, codename] in self.config['ignored_file_codes']):
return True
fcm_ignore1 = [filename, codename, message]
fcm_ignore2 = [filename, codename, message]
for fcm in self.config['ignored_file_code_messages']:
if fcm_ignore1 == [fcm[0], fcm[1], fcm[2]]:
return True
if fcm_ignore2 == [fcm[0], fcm[1], fcm[2]]:
return True
return False
def ignore_code(self, c):
_c = set(self.config['ignored_codes'])
_c.add(c)
self.config['ignored_codes'] = list(_c)
def ignore_files(self, f):
_c = set(self.config['ignored_files'])
_c.add(f)
self.config['ignored_files'] = list(_c)
def ignore_message(self, m):
_c = set(self.config['ignored_messages'])
_c.add(m)
self.config['ignored_messages'] = list(_c)
def ignore_file_code(self, f, c):
_c = set(self.config['ignored_file_codes'])
_c.add((f, c))
self.config['ignored_file_codes'] = list(_c)
def ignore_file_message(self, f, m):
_c = set(self.config['ignored_file_messages'])
_c.add((f, m))
self.config['ignored_file_messages'] = list(_c)
def ignore_file_code_message(self, f, c, m, l, fn):
_c = set(self.config['ignored_file_code_messages'])
_c.add((f, c, m, l, fn))
self.config['ignored_file_code_messages'] = list(_c)
def main():
if len(sys.argv) == 1 or sys.argv[1] == "check":
return check()
elif sys.argv[1] == "rebuild":
return rebuild()
elif sys.argv[1] == "initialize":
return initialize()
else:
return usage()
def usage():
print("Usage: %s [check|rebuild]" % sys.argv[0])
print("\tUse this tool to perform a lint check of the trove project.")
print("\t check: perform the lint check.")
print("\t rebuild: rebuild the list of exceptions to ignore.")
return 0
class LintRunner(object):
def __init__(self):
self.config = Config()
self.idline = re.compile("^[*]* Module .*")
self.detail = re.compile("(\S+):(\d+): \[(\S+)\((\S+)\), (\S+)?] (.*)")
def dolint(self, filename):
exceptions = set()
buffer = csio()
reporter = text.ParseableTextReporter(output=buffer)
options = list(self.config.get('options'))
options.append(filename)
lint.Run(options, reporter=reporter, exit=False)
output = buffer.getvalue()
buffer.close()
for line in output.splitlines():
if self.idline.match(line):
continue
if self.detail.match(line):
mo = self.detail.search(line)
tokens = mo.groups()
fn = tokens[0]
ln = tokens[1]
code = tokens[2]
codename = tokens[3]
func = tokens[4]
message = tokens[5]
if not self.config.ignore(fn, code, codename, message):
exceptions.add((fn, ln, code, codename, func, message))
return exceptions
def process(self, mode=MODE_CHECK):
files_processed = 0
files_with_errors = 0
errors_recorded = 0
exceptions_recorded = 0
for (root, dirs, files) in os.walk(self.config.get('folder')):
# if we shouldn't even bother about this part of the
# directory structure, we can punt quietly
if self.config.is_file_ignored(root):
continue
# since we are walking top down, let's clean up the dirs
# that we will walk by eliminating any dirs that will
# end up getting ignored
for d in dirs:
p = os.path.join(root, d)
if self.config.is_file_ignored(p):
dirs.remove(d)
# check if we can ignore the file and process if not
for f in files:
p = os.path.join(root, f)
if self.config.is_file_ignored(p):
continue
if not self.config.is_file_included(f):
continue
files_processed += 1
exceptions = self.dolint(p)
file_had_errors = 0
for e in exceptions:
# what we do with this exception depents on the
# kind of exception, and the mode
if self.config.is_always_error(e[5]):
print("ERROR: %s %s: %s %s, %s: %s" %
(e[0], e[1], e[2], e[3], e[4], e[5]))
errors_recorded += 1
file_had_errors += 1
elif mode == MODE_REBUILD:
# parameters to ignore_file_code_message are
# filename, code, message, linenumber, and function
self.config.ignore_file_code_message(e[0], e[2], e[-1], e[1], e[4])
self.config.ignore_file_code_message(e[0], e[3], e[-1], e[1], e[4])
exceptions_recorded += 1
elif mode == MODE_CHECK:
print("ERROR: %s %s: %s %s, %s: %s" %
(e[0], e[1], e[2], e[3], e[4], e[5]))
errors_recorded += 1
file_had_errors += 1
if file_had_errors:
files_with_errors += 1
return (files_processed, files_with_errors, errors_recorded,
exceptions_recorded)
def rebuild(self):
self.initialize()
(files_processed,
files_with_errors,
errors_recorded,
exceptions_recorded) = self.process(mode=MODE_REBUILD)
if files_with_errors > 0:
print("Rebuild failed. %s files processed, %s had errors, "
"%s errors recorded." % (
files_processed, files_with_errors, errors_recorded))
return 1
self.config.save()
print("Rebuild completed. %s files processed, %s exceptions recorded." %
(files_processed, exceptions_recorded))
return 0
def check(self):
self.config.load()
(files_processed,
files_with_errors,
errors_recorded,
exceptions_recorded) = self.process(mode=MODE_CHECK)
if files_with_errors > 0:
print("Check failed. %s files processed, %s had errors, "
"%s errors recorded." % (
files_processed, files_with_errors, errors_recorded))
return 1
print("Check succeeded. %s files processed" % files_processed)
return 0
def initialize(self):
self.config.save()
return 0
def check():
exit(LintRunner().check())
def rebuild():
exit(LintRunner().rebuild())
def initialize():
exit(LintRunner().initialize())
if __name__ == "__main__":
main()

8
tox.ini
View File

@ -1,5 +1,5 @@
[tox]
envlist = py{27,34,35},pep8,apiexamples,cover,api-ref,releasenotes,bandit,fakemodetests
envlist = py{27,34,35},pep8,apiexamples,cover,api-ref,releasenotes,bandit,fakemodetests,pylint
minversion = 1.6
skipsdist = True
@ -91,3 +91,9 @@ commands = bandit -r trove -n5 -x tests
[testenv:install-guide]
commands = sphinx-build -a -E -W -d install-guide/build/doctrees -b html install-guide/source install-guide/build/html
[testenv:pylint]
deps = -r{toxinidir}/requirements.txt
-r{toxinidir}/test-requirements.txt
commands = {[testenv]commands}
python tools/trove-pylint.py check

1
trove/common/cfg.py
View File

@ -23,6 +23,7 @@ from oslo_log import log as logging
from oslo_middleware import cors
from osprofiler import opts as profiler
from trove.common.i18n import _
from trove.version import version_info as version

1
trove/common/debug_utils.py
View File

@ -21,6 +21,7 @@ import sys
from oslo_config import cfg
from oslo_log import log as logging
from trove.common.i18n import _
LOG = logging.getLogger(__name__)
CONF = cfg.CONF

1
trove/datastore/models.py
View File

@ -19,6 +19,7 @@ from oslo_log import log as logging
from trove.common import cfg
from trove.common import exception
from trove.common.i18n import _
from trove.common.remote import create_nova_client
from trove.common import utils
from trove.db import get_db_api

1
trove/guestagent/datastore/experimental/mariadb/service.py
View File

@ -16,6 +16,7 @@
from oslo_log import log as logging
from trove.common.i18n import _
from trove.guestagent.common import operating_system
from trove.guestagent.datastore.galera_common import service as galera_service
from trove.guestagent.datastore.mysql_common import service as mysql_service

2
trove/guestagent/datastore/experimental/percona/service.py
View File

@ -15,6 +15,8 @@
#
from oslo_log import log as logging
from trove.common.i18n import _
from trove.guestagent.datastore.mysql_common import service
LOG = logging.getLogger(__name__)

1
trove/guestagent/datastore/experimental/postgresql/pgutil.py
View File

@ -16,6 +16,7 @@
import psycopg2
from trove.common import exception
from trove.common.i18n import _
PG_ADMIN = 'os_admin'

1
trove/guestagent/datastore/experimental/postgresql/service/status.py
View File

@ -16,6 +16,7 @@
from oslo_log import log as logging
import psycopg2
from trove.common.i18n import _
from trove.common import instance
from trove.common import utils
from trove.guestagent.datastore.experimental.postgresql import pgutil

1
trove/guestagent/strategies/restore/experimental/couchbase_impl.py
View File

@ -21,6 +21,7 @@ import time
from oslo_log import log as logging
from trove.common import exception
from trove.common.i18n import _
from trove.common import utils
from trove.guestagent.common import operating_system
from trove.guestagent.datastore.experimental.couchbase import service

3
trove/guestagent/strategies/restore/experimental/postgresql_impl.py
View File

@ -20,7 +20,6 @@ from eventlet.green import subprocess
from oslo_log import log as logging
from trove.common import cfg
from trove.common import exception
from trove.common.i18n import _
from trove.common import stream_codecs
from trove.guestagent.common import operating_system
@ -89,7 +88,7 @@ class PgDump(base.RestoreRunner):
for message in err.splitlines(False):
if not any(regex.match(message)
for regex in self.IGNORED_ERROR_PATTERNS):
raise exception(message)
raise Exception(message)
except OSError:
pass