Skip to content

Commit 38a9bf0

Browse files
committed
Run fab provider tests in v2-11-test
Since we are releasing fab provider from v2-11-test we should also run fab provider tests there. This chang consists of: * migrating to FAB 4.5.3 (includes scrypt algorithm fix) * monkeypatching flask-session so that it works well with new flask-sqlalchey * caching initial$aization of Session class from flask-session and AirflowDatabaseSessionInterface so that DB connection is not created multiple times during tests * fixing imports from tests
1 parent 1d9cf9e commit 38a9bf0

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+253
-518
lines changed

airflow/api_connexion/security.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,14 @@
4545

4646
def check_authentication() -> None:
4747
"""Check that the request has valid authorization information."""
48-
for auth in get_airflow_app().api_auth:
48+
airflow_app = get_airflow_app()
49+
for auth in airflow_app.api_auth:
4950
response = auth.requires_authentication(Response)()
5051
if response.status_code == 200:
5152
return
5253

5354
# Even if the current_user is anonymous, the AUTH_ROLE_PUBLIC might still have permission.
54-
appbuilder = get_airflow_app().appbuilder
55+
appbuilder = airflow_app.appbuilder
5556
if appbuilder.get_app.config.get("AUTH_ROLE_PUBLIC", None):
5657
return
5758

airflow/providers/fab/auth_manager/cli_commands/utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ def _return_appbuilder(app: Flask) -> AirflowAppBuilder:
4242
"""Return an appbuilder instance for the given app."""
4343
init_appbuilder(app)
4444
init_plugins(app)
45-
init_airflow_session_interface(app)
45+
init_airflow_session_interface(app, None)
4646
return app.appbuilder # type: ignore[attr-defined]
4747

4848

airflow/providers/fab/auth_manager/fab_auth_manager.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@
9595
)
9696
from airflow.providers.common.compat.assets import AssetDetails
9797
from airflow.providers.fab.auth_manager.security_manager.override import FabAirflowSecurityManagerOverride
98-
from airflow.security.permissions import RESOURCE_ASSET
98+
from airflow.security.permissions import RESOURCE_ASSET # type: ignore[attr-defined]
9999
else:
100100
from airflow.providers.common.compat.security.permissions import RESOURCE_ASSET
101101

@@ -403,9 +403,7 @@ def get_url_logout(self):
403403

404404
def get_url_user_profile(self) -> str | None:
405405
"""Return the url to a page displaying info about the current user."""
406-
if not self.security_manager.user_view or self.appbuilder.get_app.config.get(
407-
"AUTH_ROLE_PUBLIC", None
408-
):
406+
if not self.security_manager.user_view:
409407
return None
410408
return url_for(f"{self.security_manager.user_view.endpoint}.userinfo")
411409

airflow/providers/fab/auth_manager/security_manager/override.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@
6969
from flask_login import LoginManager
7070
from itsdangerous import want_bytes
7171
from markupsafe import Markup
72-
from packaging.version import Version
7372
from sqlalchemy import and_, func, inspect, literal, or_, select
7473
from sqlalchemy.exc import MultipleResultsFound
7574
from sqlalchemy.orm import Session, joinedload
@@ -850,7 +849,8 @@ def _init_config(self):
850849
app.config.setdefault("AUTH_ROLES_SYNC_AT_LOGIN", False)
851850
app.config.setdefault("AUTH_API_LOGIN_ALLOW_MULTIPLE_PROVIDERS", False)
852851

853-
# Werkzeug prior to 3.0.0 does not support scrypt
852+
from packaging.version import Version
853+
854854
parsed_werkzeug_version = Version(importlib.metadata.version("werkzeug"))
855855
if parsed_werkzeug_version < Version("3.0.0"):
856856
app.config.setdefault(
@@ -861,9 +861,10 @@ def _init_config(self):
861861
else:
862862
app.config.setdefault(
863863
"AUTH_DB_FAKE_PASSWORD_HASH_CHECK",
864-
"scrypt:32768:8:1$wiDa0ruWlIPhp9LM$6e409d093e62ad54df2af895d0e125b05ff6cf6414"
865-
"8350189ffc4bcc71286edf1b8ad94a442c00f890224bf2b32153d0750c89ee9"
866-
"401e62f9dcee5399065e4e5",
864+
"scrypt:32768:8:1$wiDa0ruWlIPhp9LM$6e40"
865+
"9d093e62ad54df2af895d0e125b05ff6cf6414"
866+
"8350189ffc4bcc71286edf1b8ad94a442c00f8"
867+
"90224bf2b32153d0750c89ee9401e62f9dcee5399065e4e5",
867868
)
868869

869870
# LDAP Config

airflow/providers/fab/provider.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ dependencies:
6262
# Every time we update FAB version here, please make sure that you review the classes and models in
6363
# `airflow/providers/fab/auth_manager/security_manager/override.py` with their upstream counterparts.
6464
# In particular, make sure any breaking changes, for example any new methods, are accounted for.
65-
- flask-appbuilder==4.5.3
65+
- flask-appbuilder==4.5.4
6666
- google-re2>=1.0
6767
- jmespath>=0.7.0
6868

airflow/www/app.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ def create_app(config=None, testing=False):
187187
init_jinja_globals(flask_app)
188188
init_xframe_protection(flask_app)
189189
init_cache_control(flask_app)
190-
init_airflow_session_interface(flask_app)
190+
init_airflow_session_interface(flask_app, db)
191191
init_check_user_active(flask_app)
192192
return flask_app
193193

airflow/www/extensions/init_session.py

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,52 @@
2323
from airflow.www.session import AirflowDatabaseSessionInterface, AirflowSecureCookieSessionInterface
2424

2525

26-
def init_airflow_session_interface(app):
26+
# Monkey patch flask-session's create_session_model to fix compatibility with Flask-SQLAlchemy 2.5.1
27+
# The issue is that dynamically created Session models don't inherit query_class from db.Model,
28+
# which causes AttributeError when flask-session tries to use .query property.
29+
# This patch ensures query_class is set on the Session model class.
30+
def _patch_flask_session_create_session_model():
31+
"""
32+
Patch flask-session's create_session_model to ensure query_class compatibility.
33+
34+
This fixes the issue where flask-session's Session model doesn't have the query_class
35+
attribute required by Flask-SQLAlchemy's _QueryProperty.
36+
"""
37+
try:
38+
from flask_session.sqlalchemy import sqlalchemy as flask_session_module
39+
40+
_original_create_session_model = flask_session_module.create_session_model
41+
_session_model = None
42+
43+
def patched_create_session_model(db, table_name, schema=None, bind_key=None, sequence=None):
44+
nonlocal _session_model
45+
if _session_model:
46+
return _session_model
47+
48+
# Create new model
49+
Session = _original_create_session_model(db, table_name, schema, bind_key, sequence)
50+
51+
# Ensure query_class is set for compatibility with Flask-SQLAlchemy
52+
# Use db.Query which is always available on the SQLAlchemy instance
53+
if not hasattr(Session, "query_class"):
54+
Session.query_class = getattr(db, "Query", None) or getattr(db.Model, "query_class", None)
55+
56+
_session_model = Session
57+
return Session
58+
59+
flask_session_module.create_session_model = patched_create_session_model
60+
except ImportError:
61+
# flask-session not installed, no need to patch
62+
pass
63+
64+
65+
# Apply the patch immediately when this module is imported
66+
_patch_flask_session_create_session_model()
67+
68+
_session_interface = None
69+
70+
71+
def init_airflow_session_interface(app, sqlalchemy_client):
2772
"""Set airflow session interface."""
2873
config = app.config.copy()
2974
selected_backend = conf.get("webserver", "SESSION_BACKEND")
@@ -42,9 +87,13 @@ def make_session_permanent():
4287

4388
app.before_request(make_session_permanent)
4489
elif selected_backend == "database":
45-
app.session_interface = AirflowDatabaseSessionInterface(
90+
global _session_interface
91+
if _session_interface:
92+
app.session_interface = _session_interface
93+
return
94+
_session_interface = AirflowDatabaseSessionInterface(
4695
app=app,
47-
client=None,
96+
client=sqlalchemy_client,
4897
permanent=permanent_cookie,
4998
# Typically these would be configurable with Flask-Session,
5099
# but we will set them explicitly instead as they don't make
@@ -53,6 +102,7 @@ def make_session_permanent():
53102
key_prefix="",
54103
use_signer=True,
55104
)
105+
app.session_interface = _session_interface
56106
else:
57107
raise AirflowConfigException(
58108
"Unrecognized session backend specified in "

dev/breeze/src/airflow_breeze/utils/selective_checks.py

Lines changed: 73 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@
5555
SelectiveProvidersTestType,
5656
all_helm_test_packages,
5757
all_selective_core_test_types,
58-
providers_test_type,
5958
)
6059
from airflow_breeze.utils.console import get_console
6160
from airflow_breeze.utils.exclude_from_matrix import excluded_combos
@@ -841,49 +840,51 @@ def _get_core_test_types_to_run(self) -> list[str]:
841840
return sorted_candidate_test_types
842841

843842
def _get_providers_test_types_to_run(self, split_to_individual_providers: bool = False) -> list[str]:
844-
if self._default_branch != "main":
845-
return []
846-
if self.full_tests_needed:
847-
if split_to_individual_providers:
848-
return list(providers_test_type())
849-
else:
850-
return ["Providers"]
851-
else:
852-
all_providers_source_files = self._matching_files(
853-
FileGroupForCi.ALL_PROVIDERS_PYTHON_FILES, CI_FILE_GROUP_MATCHES, CI_FILE_GROUP_EXCLUDES
854-
)
855-
assets_source_files = self._matching_files(
856-
FileGroupForCi.ASSET_FILES, CI_FILE_GROUP_MATCHES, CI_FILE_GROUP_EXCLUDES
857-
)
858-
859-
if (
860-
len(all_providers_source_files) == 0
861-
and len(assets_source_files) == 0
862-
and not self.needs_api_tests
863-
):
864-
# IF API tests are needed, that will trigger extra provider checks
865-
return []
866-
else:
867-
affected_providers = self._find_all_providers_affected(
868-
include_docs=False,
869-
)
870-
candidate_test_types: set[str] = set()
871-
if isinstance(affected_providers, AllProvidersSentinel):
872-
if split_to_individual_providers:
873-
for provider in get_available_packages():
874-
candidate_test_types.add(f"Providers[{provider}]")
875-
else:
876-
candidate_test_types.add("Providers")
877-
elif affected_providers:
878-
if split_to_individual_providers:
879-
for provider in affected_providers:
880-
candidate_test_types.add(f"Providers[{provider}]")
881-
else:
882-
candidate_test_types.add(f"Providers[{','.join(sorted(affected_providers))}]")
883-
sorted_candidate_test_types = sorted(candidate_test_types)
884-
get_console().print("[warning]Selected providers test type candidates to run:[/]")
885-
get_console().print(sorted_candidate_test_types)
886-
return sorted_candidate_test_types
843+
# For v2-11-test branch we always run airflow + fab provider tests
844+
return ["Providers[fab]"]
845+
# if self._default_branch != "main":
846+
# return []
847+
# if self.full_tests_needed:
848+
# if split_to_individual_providers:
849+
# return list(providers_test_type())
850+
# else:
851+
# return ["Providers"]
852+
# else:
853+
# all_providers_source_files = self._matching_files(
854+
# FileGroupForCi.ALL_PROVIDERS_PYTHON_FILES, CI_FILE_GROUP_MATCHES, CI_FILE_GROUP_EXCLUDES
855+
# )
856+
# assets_source_files = self._matching_files(
857+
# FileGroupForCi.ASSET_FILES, CI_FILE_GROUP_MATCHES, CI_FILE_GROUP_EXCLUDES
858+
# )
859+
#
860+
# if (
861+
# len(all_providers_source_files) == 0
862+
# and len(assets_source_files) == 0
863+
# and not self.needs_api_tests
864+
# ):
865+
# # IF API tests are needed, that will trigger extra provider checks
866+
# return []
867+
# else:
868+
# affected_providers = self._find_all_providers_affected(
869+
# include_docs=False,
870+
# )
871+
# candidate_test_types: set[str] = set()
872+
# if isinstance(affected_providers, AllProvidersSentinel):
873+
# if split_to_individual_providers:
874+
# for provider in get_available_packages():
875+
# candidate_test_types.add(f"Providers[{provider}]")
876+
# else:
877+
# candidate_test_types.add("Providers")
878+
# elif affected_providers:
879+
# if split_to_individual_providers:
880+
# for provider in affected_providers:
881+
# candidate_test_types.add(f"Providers[{provider}]")
882+
# else:
883+
# candidate_test_types.add(f"Providers[{','.join(sorted(affected_providers))}]")
884+
# sorted_candidate_test_types = sorted(candidate_test_types)
885+
# get_console().print("[warning]Selected providers test type candidates to run:[/]")
886+
# get_console().print(sorted_candidate_test_types)
887+
# return sorted_candidate_test_types
887888

888889
@staticmethod
889890
def _extract_long_provider_tests(current_test_types: set[str]):
@@ -930,7 +931,8 @@ def providers_test_types_list_as_string(self) -> str | None:
930931
if self._default_branch != "main":
931932
test_types_to_remove: set[str] = set()
932933
for test_type in current_test_types:
933-
if test_type.startswith("Providers"):
934+
# For v2-11-test branch we always run airflow + fab provider tests
935+
if test_type.startswith("Providers") and not test_type.startswith("Providers[fab]"):
934936
get_console().print(
935937
f"[warning]Removing {test_type} because the target branch "
936938
f"is {self._default_branch} and not main[/]"
@@ -1072,7 +1074,8 @@ def docs_list_as_string(self) -> str | None:
10721074
if not self.docs_build:
10731075
return None
10741076
if self._default_branch != "main":
1075-
return "apache-airflow docker-stack"
1077+
# For v2-11-test branch we always run airflow + fab provider tests
1078+
return "apache-airflow docker-stack apache-airflow-providers-fab"
10761079
if self.full_tests_needed:
10771080
return _ALL_DOCS_LIST
10781081
providers_affected = self._find_all_providers_affected(
@@ -1164,13 +1167,15 @@ def skip_pre_commits(self) -> str:
11641167

11651168
@cached_property
11661169
def skip_providers_tests(self) -> bool:
1167-
if self._default_branch != "main":
1168-
return True
1169-
if self.full_tests_needed:
1170-
return False
1171-
if self._get_providers_test_types_to_run():
1172-
return False
1173-
return True
1170+
# For v2-11-test branch we always run airflow + fab provider tests
1171+
return False
1172+
# if self._default_branch != "main":
1173+
# return True
1174+
# if self.full_tests_needed:
1175+
# return False
1176+
# if self._get_providers_test_types_to_run():
1177+
# return False
1178+
# return True
11741179

11751180
@cached_property
11761181
def test_groups(self):
@@ -1200,18 +1205,21 @@ def helm_test_packages(self) -> str:
12001205

12011206
@cached_property
12021207
def selected_providers_list_as_string(self) -> str | None:
1203-
if self._default_branch != "main":
1204-
return None
1205-
if self.full_tests_needed:
1206-
return ""
1207-
if self._are_all_providers_affected():
1208-
return ""
1209-
affected_providers = self._find_all_providers_affected(include_docs=True)
1210-
if not affected_providers:
1211-
return None
1212-
if isinstance(affected_providers, AllProvidersSentinel):
1213-
return ""
1214-
return " ".join(sorted(affected_providers))
1208+
# For v2-11-test branch we always test airflow + fab provider
1209+
return "fab"
1210+
1211+
# if self._default_branch != "main":
1212+
# return None
1213+
# if self.full_tests_needed:
1214+
# return ""
1215+
# if self._are_all_providers_affected():
1216+
# return ""
1217+
# affected_providers = self._find_all_providers_affected(include_docs=True)
1218+
# if not affected_providers:
1219+
# return None
1220+
# if isinstance(affected_providers, AllProvidersSentinel):
1221+
# return ""
1222+
# return " ".join(sorted(affected_providers))
12151223

12161224
@cached_property
12171225
def runs_on_as_json_default(self) -> str:

generated/provider_dependencies.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,7 @@
550550
"deps": [
551551
"apache-airflow-providers-common-compat>=1.2.1",
552552
"apache-airflow>=2.9.0",
553-
"flask-appbuilder==4.5.3",
553+
"flask-appbuilder==4.5.4",
554554
"flask-login>=0.6.3",
555555
"flask-session>=0.8.0",
556556
"flask>=2.2,<3",

scripts/ci/pre_commit/check_system_tests.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,21 +38,21 @@
3838
WATCHER_APPEND_INSTRUCTION_SHORT = " >> watcher()"
3939

4040
PYTEST_FUNCTION = """
41-
from tests_common.test_utils.system_tests import get_test_run # noqa: E402
41+
from tests.test_utils.system_tests import get_test_run # noqa: E402
4242
4343
# Needed to run the example DAG with pytest (see: tests/system/README.md#run_via_pytest)
4444
test_run = get_test_run(dag)
4545
"""
4646
PYTEST_FUNCTION_PATTERN = re.compile(
47-
r"from tests_common\.test_utils\.system_tests import get_test_run(?: # noqa: E402)?\s+"
47+
r"from tests\.test_utils\.system_tests import get_test_run(?: # noqa: E402)?\s+"
4848
r"(?:# .+\))?\s+"
4949
r"test_run = get_test_run\(dag\)"
5050
)
5151

5252

5353
def _check_file(file: Path):
5454
content = file.read_text()
55-
if "from tests_common.test_utils.watcher import watcher" in content:
55+
if "from tests.test_utils.watcher import watcher" in content:
5656
index = content.find(WATCHER_APPEND_INSTRUCTION_SHORT)
5757
if index == -1:
5858
errors.append(

0 commit comments

Comments
 (0)