From 1bb2aefec39b0d8f69cf89c67a8443a498cf2829 Mon Sep 17 00:00:00 2001 From: Andreas Jaeger Date: Thu, 2 Apr 2020 07:36:46 +0200 Subject: [PATCH] Update hacking for Python3 The repo is Python 3 now, so update hacking to version 3.0 which supports Python 3. Fix problems found. Update local hacking checks for new flake8. Remove hacking and friends from lower-constraints, they are not needed to be installed at run-time. Change-Id: Ia6af344ec8441dc98a0820176373dcff3a8c80d5 --- lower-constraints.txt | 4 --- test-requirements.txt | 2 +- tox.ini | 24 +++++++++++-- watcher/api/controllers/v1/types.py | 2 +- watcher/api/controllers/v1/versions.py | 1 + watcher/api/middleware/auth_token.py | 2 +- watcher/applier/actions/base.py | 2 +- watcher/applier/actions/resize.py | 34 +++++++++---------- watcher/applier/workflow_engine/default.py | 2 +- watcher/common/scheduling.py | 1 + watcher/common/utils.py | 1 + watcher/db/sqlalchemy/api.py | 4 +-- .../decision_engine/datasources/ceilometer.py | 2 +- .../decision_engine/datasources/gnocchi.py | 2 +- .../decision_engine/datasources/grafana.py | 2 +- .../decision_engine/datasources/manager.py | 14 ++++---- .../strategies/basic_consolidation.py | 2 +- .../strategies/workload_stabilization.py | 2 +- watcher/hacking/checks.py | 24 +++---------- 19 files changed, 65 insertions(+), 62 deletions(-) diff --git a/lower-constraints.txt b/lower-constraints.txt index 46163fee0..510b2b3d6 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -30,7 +30,6 @@ eventlet==0.20.0 extras==1.0.0 fasteners==0.14.1 fixtures==3.0.0 -flake8==2.5.5 freezegun==0.3.10 future==0.16.0 futurist==1.8.0 @@ -38,7 +37,6 @@ gitdb2==2.0.3 GitPython==2.1.8 gnocchiclient==7.0.1 greenlet==0.4.13 -hacking==0.12.0 idna==2.6 imagesize==1.0.0 iso8601==0.1.12 @@ -95,14 +93,12 @@ Paste==2.0.3 PasteDeploy==1.5.2 pbr==3.1.1 pecan==1.3.2 -pep8==1.5.7 pika==0.10.0 pika-pool==0.1.3 prettytable==0.7.2 psutil==5.4.3 pycadf==2.7.0 pycparser==2.18 -pyflakes==0.8.1 Pygments==2.2.0 pyinotify==0.9.6 pyOpenSSL==17.5.0 diff --git a/test-requirements.txt b/test-requirements.txt index 1a5854f69..69ba46a64 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -5,7 +5,7 @@ coverage>=4.5.1 # Apache-2.0 doc8>=0.8.0 # Apache-2.0 freezegun>=0.3.10 # Apache-2.0 -hacking>=1.1.0,<1.2.0 # Apache-2.0 +hacking>=3.0,<3.1.0 # Apache-2.0 mock>=2.0.0 # BSD oslotest>=3.3.0 # Apache-2.0 os-testr>=1.0.0 # Apache-2.0 diff --git a/tox.ini b/tox.ini index bbe8a27d9..9c84cf529 100644 --- a/tox.ini +++ b/tox.ini @@ -75,7 +75,8 @@ commands = [flake8] filename = *.py,app.wsgi show-source=True -ignore= H105,E123,E226,N320,H202 +# W504 line break after binary operator +ignore= H105,E123,E226,N320,H202,W504 builtins= _ enable-extensions = H106,H203,H904 exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build,*sqlalchemy/alembic/versions/*,demo/,releasenotes @@ -85,7 +86,26 @@ commands = python setup.py bdist_wheel [hacking] import_exceptions = watcher._i18n -local-check-factory = watcher.hacking.checks.factory + +[flake8:local-plugins] +extension = + N319 = checks:no_translate_debug_logs + N321 = checks:use_jsonutils + N322 = checks:check_assert_called_once_with + N325 = checks:check_python3_xrange + N326 = checks:check_no_basestring + N327 = checks:check_python3_no_iteritems + N328 = checks:check_asserttrue + N329 = checks:check_assertfalse + N330 = checks:check_assertempty + N331 = checks:check_assertisinstance + N332 = checks:check_assertequal_for_httpcode + N333 = checks:check_log_warn_deprecated + N340 = checks:check_oslo_i18n_wrapper + N341 = checks:check_builtins_gettext + N342 = checks:no_redundant_import_alias +paths = ./watcher/hacking + [doc8] extension=.rst diff --git a/watcher/api/controllers/v1/types.py b/watcher/api/controllers/v1/types.py index 77d41b633..fa148113b 100644 --- a/watcher/api/controllers/v1/types.py +++ b/watcher/api/controllers/v1/types.py @@ -184,7 +184,7 @@ class MultiType(wtypes.UserType): class JsonPatchType(wtypes.Base): """A complex type that represents a single json-patch operation.""" - path = wtypes.wsattr(wtypes.StringType(pattern='^(/[\w-]+)+$'), + path = wtypes.wsattr(wtypes.StringType(pattern=r'^(/[\w-]+)+$'), mandatory=True) op = wtypes.wsattr(wtypes.Enum(str, 'add', 'replace', 'remove'), mandatory=True) diff --git a/watcher/api/controllers/v1/versions.py b/watcher/api/controllers/v1/versions.py index bff91bb66..369c40064 100644 --- a/watcher/api/controllers/v1/versions.py +++ b/watcher/api/controllers/v1/versions.py @@ -25,6 +25,7 @@ class VERSIONS(enum.Enum): MINOR_4_WEBHOOK_API = 4 # v1.4: Add webhook trigger API MINOR_MAX_VERSION = 4 + # This is the version 1 API BASE_VERSION = 1 # String representations of the minor and maximum versions diff --git a/watcher/api/middleware/auth_token.py b/watcher/api/middleware/auth_token.py index b426c04a8..fd01b793d 100644 --- a/watcher/api/middleware/auth_token.py +++ b/watcher/api/middleware/auth_token.py @@ -34,7 +34,7 @@ class AuthTokenMiddleware(auth_token.AuthProtocol): """ def __init__(self, app, conf, public_api_routes=()): - route_pattern_tpl = '%s(\.json|\.xml)?$' + route_pattern_tpl = r'%s(\.json|\.xml)?$' try: self.public_api_routes = [re.compile(route_pattern_tpl % route_tpl) diff --git a/watcher/applier/actions/base.py b/watcher/applier/actions/base.py index 6252c1ebc..6518f17c9 100644 --- a/watcher/applier/actions/base.py +++ b/watcher/applier/actions/base.py @@ -140,7 +140,7 @@ class BaseAction(loadable.Loadable): raise NotImplementedError() def check_abort(self): - if self.__class__.__name__ is 'Migrate': + if self.__class__.__name__ == 'Migrate': if self.migration_type == self.LIVE_MIGRATION: return True else: diff --git a/watcher/applier/actions/resize.py b/watcher/applier/actions/resize.py index 561e545ec..935dbd0fa 100644 --- a/watcher/applier/actions/resize.py +++ b/watcher/applier/actions/resize.py @@ -47,24 +47,24 @@ class Resize(base.BaseAction): @property def schema(self): - return { - 'type': 'object', - 'properties': { - 'resource_id': { - 'type': 'string', - 'minlength': 1, - 'pattern': ('^([a-fA-F0-9]){8}-([a-fA-F0-9]){4}-' - '([a-fA-F0-9]){4}-([a-fA-F0-9]){4}-' - '([a-fA-F0-9]){12}$') - }, - 'flavor': { - 'type': 'string', - 'minlength': 1, - }, + return { + 'type': 'object', + 'properties': { + 'resource_id': { + 'type': 'string', + 'minlength': 1, + 'pattern': ('^([a-fA-F0-9]){8}-([a-fA-F0-9]){4}-' + '([a-fA-F0-9]){4}-([a-fA-F0-9]){4}-' + '([a-fA-F0-9]){12}$') }, - 'required': ['resource_id', 'flavor'], - 'additionalProperties': False, - } + 'flavor': { + 'type': 'string', + 'minlength': 1, + }, + }, + 'required': ['resource_id', 'flavor'], + 'additionalProperties': False, + } @property def instance_uuid(self): diff --git a/watcher/applier/workflow_engine/default.py b/watcher/applier/workflow_engine/default.py index 4f69b14a5..66f717e9b 100644 --- a/watcher/applier/workflow_engine/default.py +++ b/watcher/applier/workflow_engine/default.py @@ -112,7 +112,7 @@ class DefaultWorkFlowEngine(base.BaseWorkFlowEngine): return flow - except exception.ActionPlanCancelled as e: + except exception.ActionPlanCancelled: raise except tf_exception.WrappedFailure as e: diff --git a/watcher/common/scheduling.py b/watcher/common/scheduling.py index 6d42d0883..072dd12fa 100644 --- a/watcher/common/scheduling.py +++ b/watcher/common/scheduling.py @@ -37,6 +37,7 @@ class GreenThreadPoolExecutor(BasePoolExecutor): pool = futurist.GreenThreadPoolExecutor(int(max_workers)) super(GreenThreadPoolExecutor, self).__init__(pool) + executors = { 'default': GreenThreadPoolExecutor(), } diff --git a/watcher/common/utils.py b/watcher/common/utils.py index ad2f7cf7e..8c1b5d0d5 100644 --- a/watcher/common/utils.py +++ b/watcher/common/utils.py @@ -153,6 +153,7 @@ def extend_with_strict_schema(validator_class): return validators.extend(validator_class, {"properties": strict_schema}) + StrictDefaultValidatingDraft4Validator = extend_with_default( extend_with_strict_schema(validators.Draft4Validator)) diff --git a/watcher/db/sqlalchemy/api.py b/watcher/db/sqlalchemy/api.py index ee641d34b..73e82d745 100644 --- a/watcher/db/sqlalchemy/api.py +++ b/watcher/db/sqlalchemy/api.py @@ -1125,8 +1125,8 @@ class Connection(api.BaseConnection): def get_action_description_by_id(self, context, action_id, eager=False): - return self._get_action_description( - context, fieldname="id", value=action_id, eager=eager) + return self._get_action_description( + context, fieldname="id", value=action_id, eager=eager) def get_action_description_by_type(self, context, action_type, eager=False): diff --git a/watcher/decision_engine/datasources/ceilometer.py b/watcher/decision_engine/datasources/ceilometer.py index d6f294aae..316e8edd9 100644 --- a/watcher/decision_engine/datasources/ceilometer.py +++ b/watcher/decision_engine/datasources/ceilometer.py @@ -188,7 +188,7 @@ class CeilometerHelper(base.DataSourceBase): item_value = None if statistic: item_value = statistic[-1]._info.get('aggregate').get(aggregate) - if meter_name is 'host_airflow': + if meter_name == 'host_airflow': # Airflow from hardware.ipmi.node.airflow is reported as # 1/10 th of actual CFM item_value *= 10 diff --git a/watcher/decision_engine/datasources/gnocchi.py b/watcher/decision_engine/datasources/gnocchi.py index 26e275432..74100238c 100644 --- a/watcher/decision_engine/datasources/gnocchi.py +++ b/watcher/decision_engine/datasources/gnocchi.py @@ -116,7 +116,7 @@ class GnocchiHelper(base.DataSourceBase): # measure has structure [time, granularity, value] return_value = statistics[-1][2] - if meter_name is 'host_airflow': + if meter_name == 'host_airflow': # Airflow from hardware.ipmi.node.airflow is reported as # 1/10 th of actual CFM return_value *= 10 diff --git a/watcher/decision_engine/datasources/grafana.py b/watcher/decision_engine/datasources/grafana.py index af301ae7a..b194d59d4 100644 --- a/watcher/decision_engine/datasources/grafana.py +++ b/watcher/decision_engine/datasources/grafana.py @@ -72,7 +72,7 @@ class GrafanaHelper(base.DataSourceBase): # Very basic url parsing parse = urlparse.urlparse(self._base_url) - if parse.scheme is '' or parse.netloc is '' or parse.path is '': + if parse.scheme == '' or parse.netloc == '' or parse.path == '': LOG.critical("GrafanaHelper url not properly configured, " "check base_url and project_id") return diff --git a/watcher/decision_engine/datasources/manager.py b/watcher/decision_engine/datasources/manager.py index dfde0d9f6..f19e72e01 100644 --- a/watcher/decision_engine/datasources/manager.py +++ b/watcher/decision_engine/datasources/manager.py @@ -112,10 +112,10 @@ class DataSourceManager(object): datasource is attempted. """ - if not self.datasources or len(self.datasources) is 0: + if not self.datasources or len(self.datasources) == 0: raise exception.NoDatasourceAvailable - if not metrics or len(metrics) is 0: + if not metrics or len(metrics) == 0: LOG.critical("Can not retrieve datasource without specifying " "list of required metrics.") raise exception.InvalidParameter(parameter='metrics', @@ -125,11 +125,11 @@ class DataSourceManager(object): no_metric = False for metric in metrics: if (metric not in self.metric_map[datasource] or - self.metric_map[datasource].get(metric) is None): - no_metric = True - LOG.warning("Datasource: {0} could not be used due to " - "metric: {1}".format(datasource, metric)) - break + self.metric_map[datasource].get(metric) is None): + no_metric = True + LOG.warning("Datasource: {0} could not be used due to " + "metric: {1}".format(datasource, metric)) + break if not no_metric: # Try to use a specific datasource but attempt additional # datasources upon exceptions (if config has more datasources) diff --git a/watcher/decision_engine/strategy/strategies/basic_consolidation.py b/watcher/decision_engine/strategy/strategies/basic_consolidation.py index a3e1e53e2..5b621cd31 100644 --- a/watcher/decision_engine/strategy/strategies/basic_consolidation.py +++ b/watcher/decision_engine/strategy/strategies/basic_consolidation.py @@ -401,7 +401,7 @@ class BasicConsolidation(base.ServerConsolidationBaseStrategy): self._pre_execute() # backwards compatibility for node parameter. - if self.aggregation_method['node'] is not '': + if self.aggregation_method['node'] != '': LOG.warning('Parameter node has been renamed to compute_node and ' 'will be removed in next release.') self.aggregation_method['compute_node'] = \ diff --git a/watcher/decision_engine/strategy/strategies/workload_stabilization.py b/watcher/decision_engine/strategy/strategies/workload_stabilization.py index f1c4e89ec..c083370ff 100644 --- a/watcher/decision_engine/strategy/strategies/workload_stabilization.py +++ b/watcher/decision_engine/strategy/strategies/workload_stabilization.py @@ -514,7 +514,7 @@ class WorkloadStabilization(base.WorkloadStabilizationBaseStrategy): self.aggregation_method['node'] # backwards compatibility for node parameter with period. - if self.periods['node'] is not 0: + if self.periods['node'] != 0: LOG.warning('Parameter node has been renamed to compute_node and ' 'will be removed in next release.') self.periods['compute_node'] = self.periods['node'] diff --git a/watcher/hacking/checks.py b/watcher/hacking/checks.py index 4d880f512..c4bfb85f3 100644 --- a/watcher/hacking/checks.py +++ b/watcher/hacking/checks.py @@ -23,6 +23,8 @@ def flake8ext(f): only for unit tests to know which are watcher flake8 extensions. """ f.name = __name__ + f.version = '0.0.1' + f.skip_on_py3 = False return f @@ -162,11 +164,11 @@ def check_asserttrue(logical_line, filename): def check_assertfalse(logical_line, filename): if 'watcher/tests/' in filename: if re.search(r"assertEqual\(\s*False,[^,]*(,[^,]*)?\)", logical_line): - msg = ("N328: Use assertFalse(observed) instead of " + msg = ("N329: Use assertFalse(observed) instead of " "assertEqual(False, observed)") yield (0, msg) if re.search(r"assertEqual\([^,]*,\s*False(,[^,]*)?\)", logical_line): - msg = ("N328: Use assertFalse(observed) instead of " + msg = ("N329: Use assertFalse(observed) instead of " "assertEqual(False, observed)") yield (0, msg) @@ -283,21 +285,3 @@ def no_redundant_import_alias(logical_line): """ if re.match(re_redundant_import_alias, logical_line): yield(0, "N342: No redundant import alias.") - - -def factory(register): - register(use_jsonutils) - register(check_assert_called_once_with) - register(no_translate_debug_logs) - register(check_python3_xrange) - register(check_no_basestring) - register(check_python3_no_iteritems) - register(check_asserttrue) - register(check_assertfalse) - register(check_assertempty) - register(check_assertisinstance) - register(check_assertequal_for_httpcode) - register(check_log_warn_deprecated) - register(check_oslo_i18n_wrapper) - register(check_builtins_gettext) - register(no_redundant_import_alias)