Skip to content

Commit 2d16ba4

Browse files
authored
Don't verify team existence during cli parser loading (#58067)
This blocks the migration for adding the Teams table, because the executor loader will try to verify that teams exist before the table is added. Also, whether teams exist or not is not relevant when compiling the list of cli commands, causing excess latency with DB queries.
1 parent 19ebc85 commit 2d16ba4

File tree

4 files changed

+50
-7
lines changed

4 files changed

+50
-7
lines changed

airflow-core/src/airflow/cli/cli_parser.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@
6060
log = logging.getLogger(__name__)
6161

6262

63-
for executor_name in ExecutorLoader.get_executor_names():
63+
for executor_name in ExecutorLoader.get_executor_names(validate_teams=False):
6464
try:
6565
executor, _ = ExecutorLoader.import_executor_cls(executor_name)
6666
airflow_commands.extend(executor.get_cli_commands())

airflow-core/src/airflow/executors/executor_loader.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,16 +63,19 @@ class ExecutorLoader:
6363
}
6464

6565
@classmethod
66-
def _get_executor_names(cls) -> list[ExecutorName]:
66+
def _get_executor_names(cls, validate_teams: bool = True) -> list[ExecutorName]:
6767
"""
6868
Return the executor names from Airflow configuration.
6969
70+
:param validate_teams: Whether to validate that team names exist in database
7071
:return: List of executor names from Airflow configuration
7172
"""
7273
if _executor_names:
7374
return _executor_names
7475

75-
all_executor_names: list[tuple[str | None, list[str]]] = cls._get_team_executor_configs()
76+
all_executor_names: list[tuple[str | None, list[str]]] = cls._get_team_executor_configs(
77+
validate_teams=validate_teams
78+
)
7679

7780
executor_names: list[ExecutorName] = []
7881
for team_name, executor_names_config in all_executor_names:
@@ -181,12 +184,14 @@ def _validate_teams_exist_in_database(cls, team_names: set[str]) -> None:
181184
)
182185

183186
@classmethod
184-
def _get_team_executor_configs(cls) -> list[tuple[str | None, list[str]]]:
187+
def _get_team_executor_configs(cls, validate_teams: bool = True) -> list[tuple[str | None, list[str]]]:
185188
"""
186189
Return a list of executor configs to be loaded.
187190
188191
Each tuple contains the team id as the first element and the second element is the executor config
189192
for that team (a list of executor names/modules/aliases).
193+
194+
:param validate_teams: Whether to validate that team names exist in database
190195
"""
191196
from airflow.configuration import conf
192197

@@ -245,19 +250,20 @@ def _get_team_executor_configs(cls) -> list[tuple[str | None, list[str]]]:
245250

246251
# Validate that all team names exist in the database (excluding None for global configs)
247252
team_names_to_validate = {team_name for team_name in seen_teams if team_name is not None}
248-
if team_names_to_validate:
253+
if team_names_to_validate and validate_teams:
249254
cls._validate_teams_exist_in_database(team_names_to_validate)
250255

251256
return configs
252257

253258
@classmethod
254-
def get_executor_names(cls) -> list[ExecutorName]:
259+
def get_executor_names(cls, validate_teams: bool = True) -> list[ExecutorName]:
255260
"""
256261
Return the executor names from Airflow configuration.
257262
263+
:param validate_teams: Whether to validate that team names exist in database
258264
:return: List of executor names from Airflow configuration
259265
"""
260-
return cls._get_executor_names()
266+
return cls._get_executor_names(validate_teams=validate_teams)
261267

262268
@classmethod
263269
def get_default_executor_name(cls, team_name: str | None = None) -> ExecutorName:

airflow-core/tests/unit/cli/test_cli_parser.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,3 +558,13 @@ def test_airflow_config_output_does_not_contain_providers_when_excluded(self):
558558
)
559559
assert result.returncode == 0
560560
assert "celery_config_options" not in result.stdout
561+
562+
def test_cli_parser_skips_team_validation(self):
563+
"""Test that CLI parser calls get_executor_names with validate_teams=False to prevent database dependency during CLI loading."""
564+
with patch.object(executor_loader.ExecutorLoader, "get_executor_names") as mock_get_executor_names:
565+
mock_get_executor_names.return_value = []
566+
# Force reload of cli_parser to trigger the executor loading
567+
reload(cli_parser)
568+
569+
# Verify get_executor_names was called with validate_teams=False
570+
mock_get_executor_names.assert_called_with(validate_teams=False)

airflow-core/tests/unit/executors/test_executor_loader.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,3 +630,30 @@ def test_team_validation_skips_global_teams(self):
630630

631631
# No team validation should occur since only global teams are configured
632632
mock_get_team_names.assert_not_called()
633+
634+
def test_get_executor_names_skip_team_validation(self):
635+
"""Test that get_executor_names can skip team validation."""
636+
with (
637+
patch.object(executor_loader.Team, "get_all_team_names") as mock_get_team_names,
638+
mock.patch.object(executor_loader.ExecutorLoader, "block_use_of_multi_team"),
639+
):
640+
with conf_vars(
641+
{("core", "executor"): "=CeleryExecutor;team_a=LocalExecutor", ("core", "multi_team"): "True"}
642+
):
643+
# Should not call team validation when validate_teams=False
644+
executor_loader.ExecutorLoader.get_executor_names(validate_teams=False)
645+
mock_get_team_names.assert_not_called()
646+
647+
def test_get_executor_names_default_validates_teams(self):
648+
"""Test that get_executor_names validates teams by default."""
649+
with (
650+
patch.object(executor_loader.Team, "get_all_team_names") as mock_get_team_names,
651+
mock.patch.object(executor_loader.ExecutorLoader, "block_use_of_multi_team"),
652+
):
653+
with conf_vars(
654+
{("core", "executor"): "=CeleryExecutor;team_a=LocalExecutor", ("core", "multi_team"): "True"}
655+
):
656+
# Default behavior should validate teams
657+
mock_get_team_names.return_value = {"team_a"}
658+
executor_loader.ExecutorLoader.get_executor_names()
659+
mock_get_team_names.assert_called_once()

0 commit comments

Comments
 (0)