Newer pylint added some checks that trigger a lot for this
code base. Turn them off until we are ready to apply the
new rules. They are:
- c-extension-no-member (Informational)
- keyword-arg-before-vararg (Warning)
- inconsistent-return-statements (Refactor recommendation)
Change-Id: I8c1f72bb334c87d62d47e8296d13c660d6bb5576
Signed-off-by: Doug Hellmann <doug@doughellmann.com>
Recenly pylint version was bumped to 1.7.1 in global-requirements.txt.
Pylint 1.7.1 adds more checks and breaks the neutron pep8 job.
This commit adds new checks to the disabled list to avoid the gate
breakage. Individual disabled tests can be fixed later.
The new disabled list is compatible with pylint 1.4.5,
so we can merge this regardless of pylint bump is reverted or not.
Closes-Bug: #1696403
Change-Id: If6228d0626413049b82f9d75bcdf3bea7ddacde5
This is a community goal for Ocata. We're already not using any of
the code, but there are some other files and references left over.
Partial-Bug: #1639103
Change-Id: I2afd263ddc480e2512e3be77a03ffe5ae16e17a5
Debtcollector warnings are enabled by an env var for now, to avoid blasting
everyone's unit tests at once.
Partially-Implements: blueprint neutron-lib
Depends-On: I6991464acc3aef99f6ec5eff56a893deaaefe40b
Change-Id: If183b7a6797834e29c377937fc06261aa3b00249
This check attempts to detect dictionary literals with duplicate keys.
The rest of the Neutron tree has already had the few instances of this
cleaned[1] or moved into external vendor repos. Enabling the pylint
check will stop future occurrences.
[1] I29cd2b843a7905986de13a1ecfba0cb5797ccaf8
(Original patch I1aa221d2019853f905f2b8421dd45b0a3102baf0 by zhiyuan_cai)
Change-Id: If4fed9714cd7fa586845f21f8f56dde2645cc5e0
Co-Authored-By: zhiyuan_cai <luckyvega.g@gmail.com>
This change proposes to move pylint violation checks to the pep8
testenv. This changes make pylint gating within Neutron as it would
participate in the vote. Having pylint executed on a separate job makes
it difficult to handle potential unexpected breakages, because we need
to get infra involved. When we need to renable the job, it is equally
painful.
Furthermore, it also causes us to spin an extra node, when the checks
can easily be handled by the node for the pep8 job.
Finally, having pylint running with tox -epep8 "helps" developers
become aware of pylint violations sooner rather than later, if they
"forget" to run the pylint testenv too before submitting the change.
In order to make this patch succeed, a couple of pylint violation
checks were skipped, as they slipped in, whilst the job was non-voting.
We'll tackle them in due course.
Change-Id: Ida6ae44a837d1761f5ce3e8a92f8850407f5cd16
.. and disable the check locally for two midonet and Ryu classes that
fail since the midonet/ryu libraries are unavailable in our toxenv.
Change-Id: Idca46f853e6a116e8b2c246b4c6cae159894f887
Pylint last version(1.4.1), at least, reports an
unbalanced-tuple-unpacking warning[1] in keepalived[2] module because
self.authentication is defined as an empty tuple in __init__ method and
unpacked in build_config method as if it was a 2-tuple.
self.authentication references an empty tuple (defined in __init__
method) or a 2-tuple (updated in set_authentication method). Such
warning is a false positive because the unpacking is only performed if
self.authentication is not evaluated to false which only appends if
self.authentication is a 2-tuple.
Defining self.authentication as None in __init__ avoids such warning
without disabling unbalanced-tuple-unpacking warning check.
[1]
W:252,12: Possible unbalanced tuple unpacking with sequence defined at
line 153: left side has 2 label(s), right side has 0 value(s)
(unbalanced-tuple-unpacking)
[2] neutron.agent.linux.keepalived
Change-Id: Ifcdf08e574ef44a65c6d121323cbe31d9af2f921
Closes-Bug: #1411865
This check seems to have changed and started to break in Neutron.
Disabling it is one way we could get back to good.
Change-Id: I6d06d43cb2e78501cd69fcb3940694d04d936fa8
Closes-bug: #1411865
This check catches attempts to call variables that pylint believes are
not functions. A trivial example would be:
# Trivial example caught by this check:
foo = dict()
print foo('bar') # <- oops, meant foo['bar']
This change enables the "not-callable" pylint check, after disabling a
few cases where the alert triggers but the usage was intended (defining
decorators).
Change-Id: I09ad929902509018fe7183a15b784601c36b6196
Related-Bug: #1356224
Escapes in python string literals are well defined, but can be
confusing. These pylint checks look for backslash escapes in strings
that might be mistakes. Two code refactors were required to satisfy
these tests:
1. midonet_lib.py used \**kwargs in docstrings.
There doesn't seem to be a sphinx standard for kwargs, so this change
simply replaces them with "kwargs".
2. Regex literals containing escapes replaced with r''.
The assumption with this change (and the underlying pylint
check) is that r'' literals are more straightforward for regular
expressions, where every backslash is important.
While looking at these regexes, this change also removes a few
unnecessary "\-" escapes.
Change-Id: I01528b2482f78b9e851685ebbf6fded4e58355f1
This change replaces a few no-op string statements with regular
comments. While there was no harm in the previous use of strings for
comments, this allows us to re-enable the corresponding pylint check
which may catch genuinely unintended cases.
Change-Id: I796a059292e26c4df75c54f095d9e20e99187c98
This required a trivial refactor of two existing cases in the codebase.
These two cases were perfectly correct, but the check uncovered a 3rd
case which was a real bug (fixed separately). The new versions also
make it clear that if the loop fails to break early then the 'result' is
None (and thus an error) and not simply the last element. On balance,
it's probably worth enforcing this small inconvenience to coding style.
Change-Id: I780a95241f1454c6886d91f980eb9ada7678a119
Related-Bug: #1362466
_execute_request has a list of exception handlers to log various types
of errors with more specific error messages. Unfortunately, it catches
requests.exceptions.ConnectionError before requests.exceptions.SSLError,
but ConnectionError is a superclass of SSLError so the latter is never
invoked.
This change corrects the exception handling order, and enables the
bad-except-order pylint check now that the check passes.
Change-Id: I92bacd6088de5cbc170bc5c081a1db1baeec69e7
Closes-Bug: #1360970
"else" on for loops is only important if the loop contains a "break"
statement. Without a "break", the else block is _always_ executed and
it is clearer just to omit "else".
This change also enables the corresponding pylint warning, now that the
only offending case has been fixed.
Change-Id: Ibe8761cb40a7d2d564aa718d62c9f383b5ad711e
Add _MovedItems (from six.moves) to pylintrc ignored-modules, and adjust
one import of sqlalchemy.orm.properties.RelationshipProperty.
s.o.p.RelationshipProperty is created at import-time in a rather
exciting manner - rearranging the import in this way forces the
import-time code to be executed and seems sufficient to satisfy the
pylint static check.
Change-Id: Ic99dc2b7dfac75930a5c446ae899eaae09ee6174
.. and enable the cyclic-import pylint check, now that this particular
import is invisible to pylint.
Change-Id: I9bfe7f77742b0db3ebead6a6767ade9b91e54c22
The @versioning.versioned decorator used (only) in
plugins.vmware.nsxlib.router completely confuses this check, so add a
file-local pylint disable.
Change-Id: I2a79a643a982f49faaf22b88764cb170ef89ce21
Returning within a finally block can be surprising since the finally
block will "hijack" the regular return flow. In this case, pylint is
trying to warn us that the return-within-finally would discard the
earlier caught exception. For this particular function we don't care
that the exception is lost, so the existing code is correct but possibly
confusing.
Our options are:
1. Disable the lost-exception warning for this function
2. Rewrite the function to avoid return-within-finally
This change takes approach (2), since the required change is trivial.
This change also enables the corresponding pylint check now that the
only offending case has been removed.
Change-Id: If1706851b4bd67ebdbbdb3485984773598efdf7a
pylintrc update disables all warnings that currently trigger on neutron
code. The rough plan is to slowly re-enable warning categories as we
clean up code in question.
This change also includes a few ultra-trivial syntax cleanups where it
allowed the check to be immediately enabled for the rest of the
codebase:
- Added missing trailing newlines in several files
(db/migration/__init__.py, nuage/{nuagedb,syncmanager,common/config}.py)
- Renamed self to cls in @classmethods
(cisco/db/l3/device_handling_db.py)
- Removed whitespace around '=' in a kwarg
(cisco/db/l3/device_handling_db.py, cisco/db/n1kv_db_v2.py)
- Updated deprecated pylint 'disable-msg' directive to newer 'disable'
(cisco/extensions/qos.py)
- File-specific disable for too-many-format-args pending further
investigation of alternatives
(ml2/drivers/arista/arista_l3_driver.py)
- Import module rather than object and avoid long line
(services/l3_router/l3_arista.py)
Change-Id: Ifb0a1a38e33f9073a78658ca578fbd2a42747724
The correct neutron configuration will no longer be overwritten by
the old quantum configuration.
Change-Id: I4923ad4e35a5053966edb307587c72c0b684d149
Closes-Bug: 1316334
This change renames everything to Neutron while providing backwards
compatible adjustments for Grizzly configuration files.
implements blueprint: remove-use-of-quantum
Change-Id: Ie7d07ba7c89857e13d4ddc8f0e9b68de020a3d19
Change run_tests.sh for running pep8/pylint validation only
(also adds .pylintrc file)
Resubmitting this time making sure we run pylint for Quantum!
Also run just with -l for total number of messages
Run with -l -v for detailed pylint messages
Change-Id: I593c8aed4e0e6b06204c6c4308934da198778fd6