Skip to content

Commit

Permalink
Remove "from __future__" from airflow/providers __init__.py
Browse files Browse the repository at this point in the history
Cleans-up airflow and providers `__init__.py" files in order to
get providers import work again.

This is done by excluding the two `__init__.py` files from
automated ruff isort rules adding `from __future__ import annotations`.

Also removed the `__init__.py` file from "providers" directory,
it is not needed there, because "providers" is just a folder where
we keep provider files, it's not a Python package.

That should finally get rid of the Intellij teething import
problem that has been introduced in #42505.

There were earlier - unsuccesful - attempts to fix it in
the #43116 and #43081 that followed #42951 - but the key is that Pycharm
requires the namespace's extend_path to be first "real" line
of code in the `__init__.py` to understand that the package
is an "explicit" namespace package - and it conflicts with
the requirement of "from __future__ import annotations" to be
the first line of Python code.

Also this PR fixes a few other teething problems with setup of
tests that were introcuded in #42505 and #43802 "masked" by having
`__init__.py` added in providers package:

* openlineage extractor test that should not expect "providers.tests.*"
  but "tests.*" package
* common_sql_api_stubs wrongly calculating generated path for
  stub-generated files
* pytest_plugin expecting .asf.yml in "airflow" sources - even during
  compatibility tests with older version of airflow (where the
  .asf.yml is not present)
  • Loading branch information
potiuk committed Oct 18, 2024
1 parent 4edcfc8 commit a732655
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 45 deletions.
5 changes: 4 additions & 1 deletion airflow/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from __future__ import annotations

# We do not use "from __future__ import annotations" here because it is not supported
# by Pycharm when we want to make sure all imports in airflow work from namespace packages
# Adding it automatically is excluded in pyproject.toml via I002 ruff rule exclusion

# Make `airflow` a namespace package, supporting installing
# airflow.providers.* in different locations (i.e. one in site, and one in user
Expand Down
23 changes: 0 additions & 23 deletions providers/__init__.py

This file was deleted.

4 changes: 3 additions & 1 deletion providers/src/airflow/providers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
# specific language governing permissions and limitations
# under the License.

from __future__ import annotations
# We do not use "from __future__ import annotations" here because it is not supported
# by Pycharm when we want to make sure all imports in airflow work from namespace packages
# Adding it automatically is excluded in pyproject.toml via I002 ruff rule exclusion

# Make `airflow` a namespace package, supporting installing
# airflow.providers.* in different locations (i.e. one in site, and one in user
Expand Down
2 changes: 1 addition & 1 deletion providers/tests/openlineage/extractors/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ def test_extract_on_failure(task_state, is_airflow_2_10_or_higher, should_call_o

@mock.patch("airflow.providers.openlineage.conf.custom_extractors")
def test_extractors_env_var(custom_extractors):
custom_extractors.return_value = {"providers.tests.openlineage.extractors.test_base.ExampleExtractor"}
custom_extractors.return_value = {"tests.openlineage.extractors.test_base.ExampleExtractor"}
extractor = ExtractorManager().get_extractor_class(ExampleOperator(task_id="example"))
assert extractor is ExampleExtractor

Expand Down
3 changes: 2 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -346,9 +346,10 @@ section-order = [
testing = ["dev", "providers.tests", "task_sdk.tests", "tests_common", "tests"]

[tool.ruff.lint.extend-per-file-ignores]
"airflow/__init__.py" = ["F401", "TCH004"]
"airflow/__init__.py" = ["F401", "TCH004", "I002"]
"airflow/models/__init__.py" = ["F401", "TCH004"]
"airflow/models/sqla_models.py" = ["F401"]
"providers/src/airflow/providers/__init__.py" = ["I002"]

# The test_python.py is needed because adding __future__.annotations breaks runtime checks that are
# needed for the test to work
Expand Down
7 changes: 4 additions & 3 deletions scripts/ci/pre_commit/update_common_sql_api_stubs.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
)
COMMON_SQL_ROOT = (PROVIDERS_ROOT / "common" / "sql").resolve(strict=True)
OUT_DIR = AIRFLOW_SOURCES_ROOT_PATH / "out"
OUT_DIR_PROVIDERS = OUT_DIR / PROVIDERS_ROOT.relative_to(AIRFLOW_SOURCES_ROOT_PATH)
OUT_DIR_PROVIDERS = OUT_DIR / "providers"

COMMON_SQL_PACKAGE_PREFIX = "airflow.providers.common.sql."

Expand Down Expand Up @@ -334,10 +334,11 @@ def compare_stub_files(generated_stub_path: Path, force_override: bool) -> tuple
total_removals += _new_removals
total_additions += _new_additions
for target_path in COMMON_SQL_ROOT.rglob("*.pyi"):
generated_path = OUT_DIR_PROVIDERS / target_path.relative_to(PROVIDERS_ROOT)
provider_path = target_path.relative_to(PROVIDERS_ROOT)
generated_path = OUT_DIR_PROVIDERS / provider_path
if not generated_path.exists():
console.print(
f"[red]The {target_path} file is missing in generated files:. "
f"[red]The {generated_path} file is missing in generated files:. "
f"This is treated as breaking change."
)
total_removals += 1
Expand Down
33 changes: 18 additions & 15 deletions tests_common/pytest_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,21 +371,24 @@ def initialize_airflow_tests(request):
def pytest_configure(config: pytest.Config) -> None:
# Ensure that the airflow sources dir is at the end of the sys path if it's not already there. Needed to
# run import from `providers/tests/`
desired = AIRFLOW_SOURCES_ROOT_DIR.as_posix()
for path in sys.path:
if path == desired:
break
else:
# This "desired" path should be the Airflow source directory (repo root)
assert (AIRFLOW_SOURCES_ROOT_DIR / ".asf.yaml").exists(), f"Path {desired} is not Airflow root"
sys.path.append(desired)

if (backend := config.getoption("backend", default=None)) and backend not in SUPPORTED_DB_BACKENDS:
msg = (
f"Provided DB backend {backend!r} not supported, "
f"expected one of: {', '.join(map(repr, SUPPORTED_DB_BACKENDS))}"
)
pytest.exit(msg, returncode=6)
if os.environ.get("USE_AIRFLOW_VERSION") == "":
# if USE_AIRFLOW_VERSION is not empty, we are running tests against the installed version of Airflow
# and providers so there is no need to add the sources directory to the path
desired = AIRFLOW_SOURCES_ROOT_DIR.as_posix()
for path in sys.path:
if path == desired:
break
else:
# This "desired" path should be the Airflow source directory (repo root)
assert (AIRFLOW_SOURCES_ROOT_DIR / ".asf.yaml").exists(), f"Path {desired} is not Airflow root"
sys.path.append(desired)

if (backend := config.getoption("backend", default=None)) and backend not in SUPPORTED_DB_BACKENDS:
msg = (
f"Provided DB backend {backend!r} not supported, "
f"expected one of: {', '.join(map(repr, SUPPORTED_DB_BACKENDS))}"
)
pytest.exit(msg, returncode=6)

config.addinivalue_line("markers", "integration(name): mark test to run with named integration")
config.addinivalue_line("markers", "backend(name): mark test to run with named backend")
Expand Down

0 comments on commit a732655

Please sign in to comment.