This is an automated email from the ASF dual-hosted git repository. turbaszek pushed a commit to branch v1-10-test in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/v1-10-test by this push: new 33dd68b Fix rules auto-detection for upgrade check (#11066) 33dd68b is described below commit 33dd68ba23f596850beeb6e7e63d559a8fc0d34b Author: Tomek Urbaszek <turbas...@gmail.com> AuthorDate: Tue Sep 22 14:03:40 2020 +0200 Fix rules auto-detection for upgrade check (#11066) The rules were auto-registered using metaclass but first we would need to load modules that include the classes. So it's simpler to just load all rule classes and avoid metaclass. --- airflow/upgrade/checker.py | 5 +++-- airflow/upgrade/problem.py | 2 +- airflow/upgrade/rules/__init__.py | 18 ++++++++++++++++++ airflow/upgrade/rules/base_rule.py | 16 +--------------- tests/upgrade/rules/test_base_rule.py | 13 +++++++------ tests/upgrade/rules/test_conn_type_is_not_nullable.py | 3 ++- tests/upgrade/test_problem.py | 4 ++-- 7 files changed, 34 insertions(+), 27 deletions(-) diff --git a/airflow/upgrade/checker.py b/airflow/upgrade/checker.py index e8c6837..af01413 100644 --- a/airflow/upgrade/checker.py +++ b/airflow/upgrade/checker.py @@ -20,9 +20,10 @@ from typing import List from airflow.upgrade.formatters import BaseFormatter from airflow.upgrade.problem import RuleStatus -from airflow.upgrade.rules.base_rule import BaseRule, RULES +from airflow.upgrade.rules import get_rules +from airflow.upgrade.rules.base_rule import BaseRule -ALL_RULES = [cls() for cls in RULES] # type: List[BaseRule] +ALL_RULES = [cls() for cls in get_rules()] # type: List[BaseRule] def check_upgrade(formatter): diff --git a/airflow/upgrade/problem.py b/airflow/upgrade/problem.py index d5960e6..e3de490 100644 --- a/airflow/upgrade/problem.py +++ b/airflow/upgrade/problem.py @@ -31,7 +31,7 @@ class RuleStatus(NamedTuple( @property def is_success(self): - return bool(self.messages) + return len(self.messages) == 0 @classmethod def from_rule(cls, rule): diff --git a/airflow/upgrade/rules/__init__.py b/airflow/upgrade/rules/__init__.py index 13a8339..4735c7f 100644 --- a/airflow/upgrade/rules/__init__.py +++ b/airflow/upgrade/rules/__init__.py @@ -14,3 +14,21 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +import os + + +def get_rules(): + """Automatically discover all rules""" + rule_classes = [] + path = os.path.dirname(os.path.abspath(__file__)) + for file in os.listdir(path): + if not file.endswith(".py") or file in ("__init__.py", "base_rule.py"): + continue + py_file = file[:-3] + mod = __import__(".".join([__name__, py_file]), fromlist=[py_file]) + classes = [getattr(mod, x) for x in dir(mod) if isinstance(getattr(mod, x), type)] + for cls in classes: + bases = [b.__name__ for b in cls.__bases__] + if cls.__name__ != "BaseRule" and "BaseRule" in bases: + rule_classes.append(cls) + return rule_classes diff --git a/airflow/upgrade/rules/base_rule.py b/airflow/upgrade/rules/base_rule.py index 75ebe2f..c80ec77 100644 --- a/airflow/upgrade/rules/base_rule.py +++ b/airflow/upgrade/rules/base_rule.py @@ -15,24 +15,10 @@ # specific language governing permissions and limitations # under the License. -from abc import ABCMeta, abstractmethod +from abc import abstractmethod -from six import add_metaclass -RULES = [] - - -class BaseRuleMeta(ABCMeta): - def __new__(cls, clsname, bases, attrs): - clazz = super(BaseRuleMeta, cls).__new__(cls, clsname, bases, attrs) - if clsname != "BaseRule": - RULES.append(clazz) - return clazz - - -@add_metaclass(BaseRuleMeta) class BaseRule(object): - @property @abstractmethod def title(self): diff --git a/tests/upgrade/rules/test_base_rule.py b/tests/upgrade/rules/test_base_rule.py index dbf47e3..5338b91 100644 --- a/tests/upgrade/rules/test_base_rule.py +++ b/tests/upgrade/rules/test_base_rule.py @@ -15,12 +15,13 @@ # specific language governing permissions and limitations # under the License. -from airflow.upgrade.rules.base_rule import BaseRule, RULES +from airflow.upgrade.rules import get_rules +from airflow.upgrade.rules.conn_type_is_not_nullable import ConnTypeIsNotNullableRule +from airflow.upgrade.rules.base_rule import BaseRule class TestBaseRule: - def test_if_custom_rule_is_registered(self): - class CustomRule(BaseRule): - pass - - assert CustomRule in list(RULES) + def test_rules_are_registered(self): + rule_classes = get_rules() + assert BaseRule not in rule_classes + assert ConnTypeIsNotNullableRule in rule_classes diff --git a/tests/upgrade/rules/test_conn_type_is_not_nullable.py b/tests/upgrade/rules/test_conn_type_is_not_nullable.py index 53a14a5..17c230c 100644 --- a/tests/upgrade/rules/test_conn_type_is_not_nullable.py +++ b/tests/upgrade/rules/test_conn_type_is_not_nullable.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from unittest import TestCase from airflow.models import Connection from airflow.upgrade.rules.conn_type_is_not_nullable import ConnTypeIsNotNullableRule @@ -21,7 +22,7 @@ from airflow.utils.db import create_session from tests.test_utils.db import clear_db_connections -class TestConnTypeIsNotNullableRule: +class TestConnTypeIsNotNullableRule(TestCase): def tearDown(self): clear_db_connections() diff --git a/tests/upgrade/test_problem.py b/tests/upgrade/test_problem.py index af2c5b9..455cdc1 100644 --- a/tests/upgrade/test_problem.py +++ b/tests/upgrade/test_problem.py @@ -22,8 +22,8 @@ from airflow.upgrade.problem import RuleStatus class TestRuleStatus: def test_is_success(self): - assert RuleStatus(rule=mock.MagicMock(), messages=[]).is_success is False - assert RuleStatus(rule=mock.MagicMock(), messages=["aaa"]).is_success is True + assert RuleStatus(rule=mock.MagicMock(), messages=[]).is_success is True + assert RuleStatus(rule=mock.MagicMock(), messages=["aaa"]).is_success is False def test_rule_status_from_rule(self): msgs = ["An interesting problem to solve"]