From e6c2f1fed11dfff56b0f8308bfa00b9f8429475b Mon Sep 17 00:00:00 2001 From: zhouxinyong Date: Tue, 29 Jan 2019 16:17:28 +0800 Subject: [PATCH] Update hacking for Python3 The repo is Python 3 now, so update hacking to version 3.0 which supports Python 3. Fixed related code warnings and updated .pylintrc. Change-Id: I5e82d29e0d6f09b98bebdd5dbcc34b01512b28ef --- .pylintrc | 38 +++++++++++++++++++--- ovsdbapp/backend/ovs_idl/command.py | 16 ++++++--- ovsdbapp/backend/ovs_idl/idlutils.py | 6 ++-- ovsdbapp/backend/ovs_idl/rowview.py | 2 +- ovsdbapp/exceptions.py | 5 ++- ovsdbapp/schema/ovn_northbound/commands.py | 2 +- ovsdbapp/tests/unit/test_event.py | 2 +- ovsdbapp/venv.py | 7 ++-- test-requirements.txt | 4 +-- tox.ini | 16 ++++++--- 10 files changed, 68 insertions(+), 30 deletions(-) diff --git a/.pylintrc b/.pylintrc index 37ff2230..f96576d2 100644 --- a/.pylintrc +++ b/.pylintrc @@ -5,28 +5,44 @@ ignore=.git,tests [MESSAGES CONTROL] -# NOTE: This list is copied from neutron, the options which do not need to be -# suppressed have been removed. +# TODO: This list is copied from neutron, the options which do not need to be +# suppressed have been already removed, some of the remaining options will be +# removed by code adjustment. disable= # "F" Fatal errors that prevent further processing + import-error, # "I" Informational noise # "E" Error for important programming issues (likely bugs) no-member, # "W" Warnings for stylistic problems or minor programming issues + abstract-method, arguments-differ, attribute-defined-outside-init, broad-except, + dangerous-default-value, fixme, + global-statement, + no-init, protected-access, + redefined-builtin, redefined-outer-name, + signature-differs, unused-argument, + unused-import, + unused-variable, useless-super-delegation, # "C" Coding convention violations bad-continuation, invalid-name, + len-as-condition, + misplaced-comparison-constant, missing-docstring, + superfluous-parens, + ungrouped-imports, + wrong-import-order, # "R" Refactor recommendations duplicate-code, + no-else-return, no-self-use, too-few-public-methods, too-many-ancestors, @@ -37,10 +53,20 @@ disable= too-many-locals, too-many-public-methods, too-many-return-statements, + too-many-statements, inconsistent-return-statements, - catching-non-exception, - using-constant-test, - too-many-statements + useless-object-inheritance, + too-many-nested-blocks, + too-many-boolean-expressions, + not-callable, +# new for python3 version of pylint + chained-comparison, + consider-using-dict-comprehension, + consider-using-in, + consider-using-set-comprehension, + unnecessary-pass, + useless-object-inheritance + [BASIC] # Variable names can be 1 to 31 characters long, with lowercase and underscores @@ -75,6 +101,8 @@ ignore-iface-methods= [IMPORTS] # Deprecated modules which should not be used, separated by a comma deprecated-modules= +# should use oslo_serialization.jsonutils + json [TYPECHECK] # List of module names for which member attributes should not be checked diff --git a/ovsdbapp/backend/ovs_idl/command.py b/ovsdbapp/backend/ovs_idl/command.py index 01c093f4..12988b2c 100644 --- a/ovsdbapp/backend/ovs_idl/command.py +++ b/ovsdbapp/backend/ovs_idl/command.py @@ -242,16 +242,23 @@ class DbListCommand(ReadOnlyCommand): rows = table_schema.rows.values() def _match(row): + elem = getattr(row, idx) + return elem in self.records + + def _match_remove(row): elem = getattr(row, idx) found = elem in self.records if found: records_found.remove(elem) return found + def _match_true(row): + return True + records_found = [] if idx and self.records: if self.if_exists: - match = lambda row: getattr(row, idx) in self.records + match = _match else: # If we're using the approach of removing the unwanted # elements, we'll use a helper list to remove elements as we @@ -259,9 +266,9 @@ class DbListCommand(ReadOnlyCommand): # quickly if there's some record missing to raise a RowNotFound # exception later. records_found = list(self.records) - match = _match + match = _match_remove else: - match = lambda row: True + match = _match_true self.result = [ rowview.RowView(row) if self.row else { @@ -332,5 +339,4 @@ class DbRemoveCommand(BaseCommand): except idlutils.RowNotFound: if self.if_exists: return - else: - raise + raise diff --git a/ovsdbapp/backend/ovs_idl/idlutils.py b/ovsdbapp/backend/ovs_idl/idlutils.py index 025f567c..505fa798 100644 --- a/ovsdbapp/backend/ovs_idl/idlutils.py +++ b/ovsdbapp/backend/ovs_idl/idlutils.py @@ -210,7 +210,7 @@ def condition_match(row, condition): # I haven't investigated the reason for the patch that # added this code, but for now I check string_types if type(match) is not type(val) and not all( - isinstance(x, six.string_types) for x in (match, val)): + isinstance(x, six.string_types) for x in (match, val)): # Types of 'val' and 'match' arguments MUST match in all cases with 2 # exceptions: # - 'match' is an empty list and column's type is optional; @@ -297,8 +297,8 @@ def db_replace_record(obj): for k, v in six.iteritems(obj): if isinstance(v, api.Command): obj[k] = v.result - elif (isinstance(obj, collections.Sequence) - and not isinstance(obj, six.string_types)): + elif (isinstance(obj, collections.Sequence) and + not isinstance(obj, six.string_types)): for i, v in enumerate(obj): if isinstance(v, api.Command): try: diff --git a/ovsdbapp/backend/ovs_idl/rowview.py b/ovsdbapp/backend/ovs_idl/rowview.py index e2cf481d..6e143b89 100644 --- a/ovsdbapp/backend/ovs_idl/rowview.py +++ b/ovsdbapp/backend/ovs_idl/rowview.py @@ -22,7 +22,7 @@ class RowView(object): # use other's == since it is likely to be a Row object try: return other == self._row - except NotImplemented: + except NotImplementedError: return other._row == self._row def __hash__(self): diff --git a/ovsdbapp/exceptions.py b/ovsdbapp/exceptions.py index 39665ea5..94de1ff9 100644 --- a/ovsdbapp/exceptions.py +++ b/ovsdbapp/exceptions.py @@ -31,9 +31,8 @@ class OvsdbAppException(RuntimeError): except Exception: if self.use_fatal_exceptions(): raise - else: - # at least get the core message out if something happened - super(OvsdbAppException, self).__init__(self.message) + # at least get the core message out if something happened + super(OvsdbAppException, self).__init__(self.message) if six.PY2: def __unicode__(self): diff --git a/ovsdbapp/schema/ovn_northbound/commands.py b/ovsdbapp/schema/ovn_northbound/commands.py index 537d3ee4..41e9134b 100644 --- a/ovsdbapp/schema/ovn_northbound/commands.py +++ b/ovsdbapp/schema/ovn_northbound/commands.py @@ -727,7 +727,7 @@ class LrpAddCommand(cmd.BaseCommand): msg = "Port %s exists with different networks" % ( self.port) elif (not self.peer) != (not lrp.peer) or ( - self.peer != lrp.peer): + self.peer != lrp.peer): msg = "Port %s exists with different peer" % (self.port) if msg: raise RuntimeError(msg) diff --git a/ovsdbapp/tests/unit/test_event.py b/ovsdbapp/tests/unit/test_event.py index df21fff6..4719a88b 100644 --- a/ovsdbapp/tests/unit/test_event.py +++ b/ovsdbapp/tests/unit/test_event.py @@ -32,4 +32,4 @@ class TestEvent(event.RowEvent): class TestRowEvent(base.TestCase): def test_compare_stop_event(self): r = TestEvent() - self.assertFalse((r, "fake", "fake", "fake") == event.STOP_EVENT) + self.assertNotEqual((r, "fake", "fake", "fake"), event.STOP_EVENT) diff --git a/ovsdbapp/venv.py b/ovsdbapp/venv.py index c9565d29..fc41da43 100644 --- a/ovsdbapp/venv.py +++ b/ovsdbapp/venv.py @@ -221,11 +221,8 @@ class OvsOvnVenvFixture(OvsVenvFixture): 'external_ids:ovn-encap-type=geneve', 'external_ids:ovn-encap-ip=127.0.0.1']) # TODO(twilson) SSL stuff - if False: - pass - else: - self.call(['ovs-vsctl', 'set', 'open', '.', - 'external_ids:ovn-remote=' + self.ovnsb_connection]) + self.call(['ovs-vsctl', 'set', 'open', '.', + 'external_ids:ovn-remote=' + self.ovnsb_connection]) self.call(['ovn-northd', '--detach', '--no-chdir', '--pidfile', '-vconsole:off', '--log-file', '--ovnsb-db=' + self.ovnsb_connection, diff --git a/test-requirements.txt b/test-requirements.txt index 7e2e7ed4..b2773b97 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -2,13 +2,13 @@ # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. -hacking<0.13,>=0.12.0 # Apache-2.0 +hacking>=3.0.0,<3.1.0 # Apache-2.0 coverage!=4.4,>=4.0 # Apache-2.0 python-subunit>=1.0.0 # Apache-2.0/BSD oslotest>=3.2.0 # Apache-2.0 os-testr>=1.0.0 # Apache-2.0 -pylint==1.9.2 # GPLv2 +pylint==2.3.0 # GPLv2 stestr>=2.0.0 # Apache-2.0 testscenarios>=0.4 # Apache-2.0/BSD testtools>=2.2.0 # MIT diff --git a/tox.ini b/tox.ini index 1cd90a61..5b85469d 100644 --- a/tox.ini +++ b/tox.ini @@ -1,7 +1,8 @@ [tox] minversion = 3.1.1 -envlist = py36,py37,pypy,pep8 +envlist = py36,py37,pep8 skipsdist = True +ignore_basepython_conflict = True [testenv] usedevelop = True @@ -17,6 +18,7 @@ deps = commands = stestr run --slowest {posargs} [testenv:pep8] +basepython= python3 commands = flake8 {toxinidir}/tools/coding-checks.sh --all '{posargs}' @@ -63,11 +65,17 @@ commands = {[testenv]commands} [flake8] -# E123, E125 skipped as they are invalid PEP-8. - +# W504 line break after binary operator +ignore = W504 +# H106: Don't put vim configuration in source files +# H203: Use assertIs(Not)None to check for None +# H204: Use assert(Not)Equal to check for equality +# H205: Use assert(Greater|Less)(Equal) for comparison +# H904: Delay string interpolations at logging calls +enable-extensions=H106,H203,H204,H205,H904 show-source = True -ignore = E123,E125 exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build +import-order-style = pep8 [testenv:lower-constraints] deps =