You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by tu...@apache.org on 2020/12/10 22:52:47 UTC
[airflow] branch v1-10-stable updated: Add possibility to check if
upgrade check rule applies (#12981)
This is an automated email from the ASF dual-hosted git repository.
turbaszek pushed a commit to branch v1-10-stable
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/v1-10-stable by this push:
new f666da6 Add possibility to check if upgrade check rule applies (#12981)
f666da6 is described below
commit f666da6123494422d4b6514ba7b474f862b4d3a2
Author: Tomek Urbaszek <tu...@gmail.com>
AuthorDate: Thu Dec 10 22:51:33 2020 +0000
Add possibility to check if upgrade check rule applies (#12981)
closes: #12897
---
airflow/upgrade/formatters.py | 30 ++++++++++++++++---------
airflow/upgrade/problem.py | 10 ++++++---
airflow/upgrade/rules/base_rule.py | 7 ++++++
airflow/upgrade/rules/pod_template_file_rule.py | 5 +++++
tests/upgrade/test_formattes.py | 15 ++++++++++++-
tests/upgrade/test_problem.py | 7 +++---
6 files changed, 56 insertions(+), 18 deletions(-)
diff --git a/airflow/upgrade/formatters.py b/airflow/upgrade/formatters.py
index daeb37f..4b2c5c3 100644
--- a/airflow/upgrade/formatters.py
+++ b/airflow/upgrade/formatters.py
@@ -74,20 +74,23 @@ class ConsoleFormatter(BaseFormatter):
@staticmethod
def display_recommendations(rule_statuses):
for rule_status in rule_statuses:
+ # Show recommendations only if there are any messaged
if not rule_status.messages:
continue
- # Show recommendations only if there are any messaged
rule = rule_status.rule
- lines = [
- rule.title,
- "-" * len(rule.title),
- rule.description,
- "",
- "Problems:",
- "",
- ]
- lines.extend(['{:>3}. {}'.format(i, m) for i, m in enumerate(rule_status.messages, 1)])
+ lines = [rule.title, "-" * len(rule.title)]
+ if rule_status.skipped:
+ lines.extend([rule_status.messages[0]])
+ else:
+ if rule.description:
+ lines.extend([rule.description])
+ lines.extend([
+ "",
+ "Problems:",
+ "",
+ ])
+ lines.extend(['{:>3}. {}'.format(i, m) for i, m in enumerate(rule_status.messages, 1)])
msg = "\n".join(lines)
formatted_msg = pygments.highlight(
@@ -96,7 +99,12 @@ class ConsoleFormatter(BaseFormatter):
print(formatted_msg)
def on_next_rule_status(self, rule_status):
- status = colorize("green", "SUCCESS") if rule_status.is_success else colorize("red", "FAIL")
+ if rule_status.skipped:
+ status = colorize("yellow", "SKIPPED")
+ elif rule_status.is_success:
+ status = colorize("green", "SUCCESS")
+ else:
+ status = colorize("red", "FAIL")
status_line_fmt = self.prepare_status_line_format()
print(status_line_fmt.format(rule_status.rule.title, status))
diff --git a/airflow/upgrade/problem.py b/airflow/upgrade/problem.py
index 38ee0fa..e26b020 100644
--- a/airflow/upgrade/problem.py
+++ b/airflow/upgrade/problem.py
@@ -25,10 +25,10 @@ class RuleStatus(NamedTuple(
'RuleStatus',
[
('rule', BaseRule),
- ('messages', List[str])
+ ('messages', List[str]),
+ ('skipped', bool)
]
)):
-
@property
def is_success(self):
return len(self.messages) == 0
@@ -36,10 +36,14 @@ class RuleStatus(NamedTuple(
@classmethod
def from_rule(cls, rule):
# type: (BaseRule) -> RuleStatus
+ msg = rule.should_skip()
+ if msg:
+ return cls(rule=rule, messages=[msg], skipped=True)
+
messages = [] # type: List[str]
result = rule.check()
if isinstance(result, str):
messages = [result]
elif isinstance(result, Iterable):
messages = list(result)
- return cls(rule=rule, messages=messages)
+ return cls(rule=rule, messages=messages, skipped=False)
diff --git a/airflow/upgrade/rules/base_rule.py b/airflow/upgrade/rules/base_rule.py
index c80ec77..8cf0e0f 100644
--- a/airflow/upgrade/rules/base_rule.py
+++ b/airflow/upgrade/rules/base_rule.py
@@ -33,5 +33,12 @@ class BaseRule(object):
"""A long description explaining the problem in detail. This can be an entry from UPDATING.md file."""
pass
+ def should_skip(self):
+ """
+ Executes a pre check of configuration. If returned value is
+ True then the checking the rule is omitted.
+ """
+ pass
+
def check(self):
pass
diff --git a/airflow/upgrade/rules/pod_template_file_rule.py b/airflow/upgrade/rules/pod_template_file_rule.py
index 8cbbf96..148d34d 100644
--- a/airflow/upgrade/rules/pod_template_file_rule.py
+++ b/airflow/upgrade/rules/pod_template_file_rule.py
@@ -52,6 +52,11 @@ value for all pods launched by the KubernetesExecutor. Many Kubernetes configs a
needed once this pod_template_file has been generated.
"""
+ def should_skip(self):
+ # Check this rule only if users use KubernetesExecutor
+ if conf.get("core", "executor") != "KubernetesExecutor":
+ return "Skipped because this rule applies only to environment using KubernetesExecutor."
+
def check(self):
pod_template_file = conf.get("kubernetes", "pod_template_file", fallback=None)
if not pod_template_file:
diff --git a/tests/upgrade/test_formattes.py b/tests/upgrade/test_formattes.py
index e9b6e85..169c1e4 100644
--- a/tests/upgrade/test_formattes.py
+++ b/tests/upgrade/test_formattes.py
@@ -34,8 +34,16 @@ class MockRule(BaseRule):
return MESSAGES
+class MockRuleToSkip(BaseRule):
+ title = "title"
+ description = "description"
+
+ def should_skip(self):
+ return "Yes, skipp this rule"
+
+
class TestJSONFormatter:
- @mock.patch("airflow.upgrade.checker.ALL_RULES", [MockRule()])
+ @mock.patch("airflow.upgrade.checker.ALL_RULES", [MockRule(), MockRuleToSkip()])
@mock.patch("airflow.upgrade.checker.logging.disable") # mock to avoid side effects
def test_output(self, _):
expected = [
@@ -43,6 +51,11 @@ class TestJSONFormatter:
"rule": MockRule.__name__,
"title": MockRule.title,
"messages": MESSAGES,
+ },
+ {
+ "rule": MockRuleToSkip.__name__,
+ "title": MockRuleToSkip.title,
+ "messages": ["Yes, skipp this rule"],
}
]
parser = cli.CLIFactory.get_parser()
diff --git a/tests/upgrade/test_problem.py b/tests/upgrade/test_problem.py
index 455cdc1..87ea020 100644
--- a/tests/upgrade/test_problem.py
+++ b/tests/upgrade/test_problem.py
@@ -22,14 +22,15 @@ from airflow.upgrade.problem import RuleStatus
class TestRuleStatus:
def test_is_success(self):
- assert RuleStatus(rule=mock.MagicMock(), messages=[]).is_success is True
- assert RuleStatus(rule=mock.MagicMock(), messages=["aaa"]).is_success is False
+ assert RuleStatus(rule=mock.MagicMock(), messages=[], skipped=False).is_success is True
+ assert RuleStatus(rule=mock.MagicMock(), messages=["aaa"], skipped=False).is_success is False
def test_rule_status_from_rule(self):
msgs = ["An interesting problem to solve"]
rule = mock.MagicMock()
rule.check.return_value = msgs
+ rule.should_skip.return_value = None
result = RuleStatus.from_rule(rule)
rule.check.assert_called_once_with()
- assert result == RuleStatus(rule, msgs)
+ assert result == RuleStatus(rule, msgs, skipped=False)