a2d336de2a
Since the config is not in a deterministic order makes it hard to compare two config's and see what changed. Personally, I'm not positive I understand this use-case; i.e. you have an existing config file, you save it, and then rebuild and then diff the two files. I'd have thought you'd just run check and the output of the tool was the diff. I however do see the value in sorting the file so that when someone submits a change that includes a change to the config, reviewers can see more easily what the change is doing. Similarly, the output from pylint (errors) are generated one file at a time and os.walk makes no guarantee of deterministic order. So we should collect all errors (across all files) and then print an ordered list for human consumption. The intent is also to make pylint voting soon (in master). the changes to contributing.rst and tox.ini are to make that easier. The config file has also been sorted in place. This change was motivated by an email exchange with Peter so I am marking him as a co-conspirator. The line numbers were removed from the tools/trove-pylint.config file as these would change whenever the line numbers in the file changed (since they are currently not being used in the comparison; they can be re-added if deemed necessary at the cost of having every 'rebuild' run create a different file). The tools/trove-pylint.config was regenerated as well, since the remaining two errors seem to be innocuous: ERROR: trove/taskmanager/manager.py 392: E1101 no-member, Manager.upgrade: Instance of 'BuiltInstance' has no 'upgrade' member (new method introduced by instance upgrade; other BuiltInstance member errors are already ignored.) and ERROR: trove/guestagent/datastore/experimental/postgresql/service/ access.py 80: E1101 no-member, PgSqlAccess.list_access: Instance ofi 'PgSqlAccess' has no '_find_user' member (this is due to the fact that PostgreSQL is spread over multiple files and pylint should cease to complain once https://review.openstack.org/#/c/346082/ lands.) Change-Id: I910c738d3845b7749e57910f76523150ec5a5bff Closes-Bug: #1625158 Closes-Bug: #1625245 Co-Authored-By: Peter Stachowski <peter@tesora.com>
174 lines
5.2 KiB
Plaintext
174 lines
5.2 KiB
Plaintext
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
|
|
|
|
or explicitly as:
|
|
|
|
tox -e pylint check
|
|
|
|
The equivalent result can be obtained by running the command:
|
|
|
|
tools/trove-pylint.py
|
|
|
|
or
|
|
|
|
tools/trove-pylint.py check
|
|
|
|
Running the tool directly may require installing addition pip
|
|
modules on your machine (such as pylint), so using 'tox' is the
|
|
preferred method.
|
|
|
|
|
|
For example, here is the result from such a run.
|
|
|
|
$ tox -e pylint 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.
|
|
|
|
$ tox -e pylint 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()",
|
|
"TroveExtensionMiddleware.__init__"
|
|
],
|
|
[
|
|
"trove/common/extensions.py",
|
|
"bad-super-call",
|
|
"Bad first argument 'ExtensionMiddleware' given to super()",
|
|
"TroveExtensionMiddleware.__init__"
|
|
],
|
|
|
|
With that done, I can recheck as shown below.
|
|
|
|
$ tox -e pylint
|
|
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, remove them 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.
|
|
|