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 2021/03/14 10:48:36 UTC

[airflow] branch v1-10-stable updated: Add conf not importable from airflow rule (#14400)

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 60991f0  Add conf not importable from airflow rule (#14400)
60991f0 is described below

commit 60991f0fdd23a7a57ce63c10722d93116ee9c20f
Author: Sunday Mgbogu <32...@users.noreply.github.com>
AuthorDate: Sun Mar 14 11:48:19 2021 +0100

    Add conf not importable from airflow rule (#14400)
    
    Closes: #13945
---
 .../rules/fix_conf_not_importable_from_airflow.py  | 60 +++++++++++++++++
 .../test_fix_conf_not_importable_from_airflow.py   | 75 ++++++++++++++++++++++
 2 files changed, 135 insertions(+)

diff --git a/airflow/upgrade/rules/fix_conf_not_importable_from_airflow.py b/airflow/upgrade/rules/fix_conf_not_importable_from_airflow.py
new file mode 100644
index 0000000..88dd41f
--- /dev/null
+++ b/airflow/upgrade/rules/fix_conf_not_importable_from_airflow.py
@@ -0,0 +1,60 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from airflow.upgrade.rules.base_rule import BaseRule
+from airflow import conf
+from airflow.utils.dag_processing import list_py_file_paths
+
+
+class ProperlyImportConfFromAirflow(BaseRule):
+    """
+      ProperlyImportConfFromAirflow class to ensure proper import of conf to work in Airflow 2.0
+      """
+    title = "Ensure Users Properly Import conf from Airflow"
+    description = """\
+    In Airflow-2.0, it's not possible to import `conf` from airflow by using `import conf from airflow`
+    To ensure your code works in Airflow 2.0, you should use `from airflow.configuration import conf`.
+                      """
+
+    wrong_conf_import = "from airflow import conf"
+    proper_conf_import = "from airflow.configuration import conf"
+
+    @staticmethod
+    def _conf_import_info(file_path, line_number):
+        return "Affected file: {} (line {})".format(file_path, line_number)
+
+    def _check_file(self, file_path):
+        problems = []
+        conf_import_check = self.wrong_conf_import
+        with open(file_path, "r") as file_pointer:
+            try:
+                for line_number, line in enumerate(file_pointer, 1):
+                    if conf_import_check in line:
+                        problems.append(self._conf_import_info(file_path, line_number))
+            except UnicodeDecodeError:
+                problems.append("Unable to read python file {}".format(file_path))
+        return problems
+
+    def check(self):
+        dag_folder = conf.get("core", "dags_folder")
+        files = list_py_file_paths(directory=dag_folder, include_examples=False)
+        problems = []
+        for file in files:
+            if not file.endswith(".py"):
+                continue
+            problems.extend(self._check_file(file))
+        return problems
diff --git a/tests/upgrade/rules/test_fix_conf_not_importable_from_airflow.py b/tests/upgrade/rules/test_fix_conf_not_importable_from_airflow.py
new file mode 100644
index 0000000..b65906f
--- /dev/null
+++ b/tests/upgrade/rules/test_fix_conf_not_importable_from_airflow.py
@@ -0,0 +1,75 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from contextlib import contextmanager
+from tempfile import NamedTemporaryFile
+from unittest import TestCase
+
+from tests.compat import mock
+
+from airflow.upgrade.rules.fix_conf_not_importable_from_airflow import ProperlyImportConfFromAirflow
+
+
+@contextmanager
+def create_temp_file(mock_list_files, lines, extension=".py"):
+    with NamedTemporaryFile("w+", suffix=extension) as temp_file:
+        mock_list_files.return_value = [temp_file.name]
+        temp_file.writelines("\n".join(lines))
+        temp_file.flush()
+        yield temp_file
+
+
+@mock.patch("airflow.upgrade.rules.fix_conf_not_importable_from_airflow.list_py_file_paths")
+class TestProperConfImportFromAirflow(TestCase):
+
+    def test_check_conf_import_info(self, mock_list_files):
+        rule = ProperlyImportConfFromAirflow()
+        assert isinstance(rule.title, str)
+        assert isinstance(rule.description, str)
+
+    def test_valid_check(self, mock_list_files):
+        lines = ["import foo.bar.baz"]
+        with create_temp_file(mock_list_files, lines):
+            rule = ProperlyImportConfFromAirflow()
+            msgs = rule.check()
+            assert 0 == len(msgs)
+
+    def test_invalid_check(self, mock_list_files):
+        lines = [
+            "from airflow import conf",
+            "from airflow import conf",
+        ]
+        with create_temp_file(mock_list_files, lines) as temp_file:
+            rule = ProperlyImportConfFromAirflow()
+            msgs = rule.check()
+            assert 2 == len(msgs)
+
+            base_message = "Affected file: {}".format(temp_file.name)
+            expected_messages = ["{} (line {})".format(base_message, line_number) for line_number in [1, 2]]
+            assert expected_messages == msgs
+
+    def test_non_python_file_ignored(self, mock_list_files):
+        lines = [
+            "from airflow import conf",
+            "from airflow import conf",
+        ]
+        with create_temp_file(mock_list_files, lines, extension=".etc"):
+            # Although this file "matches", it shouldn't be flagged because
+            # only python files are checked for dags anyway
+            rule = ProperlyImportConfFromAirflow()
+            msgs = rule.check()
+            assert 0 == len(msgs)