This is an automated email from the ASF dual-hosted git repository.
shahar1 pushed a commit to branch airflow-ctl/v0-1-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/airflow-ctl/v0-1-test by this
push:
new 071c5171443 [airflow-ctl/v0-1-test] Report invalid pools in airflowctl
pools import as readable errors (#76173) (#76204)
071c5171443 is described below
commit 071c5171443ca9f5ad27baef81301979bb74e746
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Sun Nov 1 13:18:49 2026 +0200
[airflow-ctl/v0-1-test] Report invalid pools in airflowctl pools import as
readable errors (#76173) (#76204)
* Report invalid pools in airflowctl pools import as readable errors
A pool entry with a bad value, such as non-integer slots or slots below
-1, crashed `airflowctl pools import` with a raw pydantic traceback that
did not say which pool was wrong. Report the pool name, the file and the
reason instead, as `airflowctl connections import` already does.
* Test more invalid pool cases in airflowctl pools import
A pool file with a valid pool followed by an invalid one must not send
the valid pool, and a pool with several invalid values must report all
of them. Neither case was covered by the new error handling.
---------
(cherry picked from commit 4eba3515cee9cd8a625def72d4aff851fc8e9657)
Co-authored-by: Rishabh <[email protected]>
Co-authored-by: Shahar Epstein <[email protected]>
---
.../src/airflowctl/ctl/commands/pool_command.py | 31 ++++++++----
.../airflow_ctl/ctl/commands/test_pool_command.py | 56 ++++++++++++++++++++++
2 files changed, 78 insertions(+), 9 deletions(-)
diff --git a/airflow-ctl/src/airflowctl/ctl/commands/pool_command.py
b/airflow-ctl/src/airflowctl/ctl/commands/pool_command.py
index 8815827ab6f..588656a7bf8 100644
--- a/airflow-ctl/src/airflowctl/ctl/commands/pool_command.py
+++ b/airflow-ctl/src/airflowctl/ctl/commands/pool_command.py
@@ -23,6 +23,7 @@ from json import JSONDecodeError
from pathlib import Path
import rich
+from pydantic import ValidationError
from airflowctl.api.client import NEW_API_CLIENT, Client, ClientKind,
provide_api_client
from airflowctl.api.datamodels.generated import (
@@ -32,6 +33,7 @@ from airflowctl.api.datamodels.generated import (
PoolBody,
)
from airflowctl.ctl.console_formatting import AirflowConsole
+from airflowctl.ctl.utils.validation import split_validation_errors
@provide_api_client(kind=ClientKind.CLI)
@@ -84,6 +86,25 @@ def export(args, api_client: Client = NEW_API_CLIENT) ->
None:
raise SystemExit(f"Failed to export pools: {e}")
+def _build_pool_body(pool_config: dict, filepath: Path) -> PoolBody:
+ """Build a ``PoolBody``, exiting with a readable message naming the pool
if it is invalid."""
+ try:
+ return PoolBody(
+ name=pool_config["name"],
+ slots=pool_config["slots"],
+ description=pool_config.get("description", ""),
+ include_deferred=pool_config.get("include_deferred", False),
+ team_name=pool_config.get("team_name"),
+ )
+ except ValidationError as e:
+ missing, invalid = split_validation_errors(PoolBody, e, str)
+ messages = []
+ if missing:
+ messages.append(f"missing required field(s): {', '.join(missing)}")
+ messages.extend(invalid)
+ raise SystemExit(f"Invalid pool {pool_config['name']!r} in {filepath}:
" + "; ".join(messages))
+
+
def _import_helper(api_client: Client, filepath: Path, action_on_existence:
BulkActionOnExistence):
"""Help import pools from the json file."""
try:
@@ -100,15 +121,7 @@ def _import_helper(api_client: Client, filepath: Path,
action_on_existence: Bulk
if not isinstance(pool_config, dict) or "name" not in pool_config or
"slots" not in pool_config:
raise SystemExit(f"Invalid pool configuration: {pool_config}")
- pools_to_update.append(
- PoolBody(
- name=pool_config["name"],
- slots=pool_config["slots"],
- description=pool_config.get("description", ""),
- include_deferred=pool_config.get("include_deferred", False),
- team_name=pool_config.get("team_name"),
- )
- )
+ pools_to_update.append(_build_pool_body(pool_config, filepath))
bulk_body = BulkBodyPoolBody(
actions=[
diff --git a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_pool_command.py
b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_pool_command.py
index 15ce83e7af1..f66350067a1 100644
--- a/airflow-ctl/tests/airflow_ctl/ctl/commands/test_pool_command.py
+++ b/airflow-ctl/tests/airflow_ctl/ctl/commands/test_pool_command.py
@@ -64,6 +64,62 @@ class TestPoolImportCommand:
with pytest.raises(SystemExit, match="Invalid pool configuration:
{'invalid': 'config'}"):
pool_command.import_(mock.MagicMock(file=invalid_pool,
action_on_existing_key="fail"))
+ @pytest.mark.parametrize(
+ ("pool_config", "expected_reason"),
+ [
+ (
+ {"name": "bad_pool", "slots": "many"},
+ "slots: Input should be a valid integer, unable to parse
string as an integer",
+ ),
+ ({"name": "bad_pool", "slots": -5}, "slots: Input should be
greater than or equal to -1"),
+ (
+ {"name": "bad_pool", "slots": 1, "include_deferred": "maybe"},
+ "include_deferred: Input should be a valid boolean, unable to
interpret input",
+ ),
+ (
+ {"name": "bad_pool", "slots": "many", "include_deferred":
"maybe"},
+ "slots: Input should be a valid integer, unable to parse
string as an integer; "
+ "include_deferred: Input should be a valid boolean, unable to
interpret input",
+ ),
+ ({"name": None, "slots": 1}, "missing required field(s): name"),
+ ],
+ ids=[
+ "non-integer-slots",
+ "slots-below-minimum",
+ "non-boolean-include-deferred",
+ "non-integer-slots-and-non-boolean-include-deferred",
+ "null-name",
+ ],
+ )
+ def test_import_reports_invalid_pool(self, mock_client, tmp_path,
pool_config, expected_reason):
+ """Test that an invalid pool value is reported as a readable error
instead of a traceback."""
+ pools_file = tmp_path / "pools.json"
+ pools_file.write_text(json.dumps([pool_config]))
+
+ with pytest.raises(SystemExit) as exc_info:
+ pool_command.import_(mock.MagicMock(file=pools_file,
action_on_existing_key="fail"))
+
+ assert (
+ str(exc_info.value) == f"Invalid pool {pool_config['name']!r} in
{pools_file}: {expected_reason}"
+ )
+ mock_client.pools.bulk.assert_not_called()
+
+ def test_import_reports_invalid_pool_after_valid_one(self, mock_client,
tmp_path):
+ """Test that an invalid pool stops the whole import, so the valid pool
before it is not sent."""
+ pools_file = tmp_path / "pools.json"
+ pools_file.write_text(
+ json.dumps([{"name": "good_pool", "slots": 1}, {"name":
"bad_pool", "slots": "many"}])
+ )
+
+ with pytest.raises(SystemExit) as exc_info:
+ pool_command.import_(mock.MagicMock(file=pools_file,
action_on_existing_key="fail"))
+
+ assert str(exc_info.value) == (
+ f"Invalid pool 'bad_pool' in {pools_file}: "
+ "slots: Input should be a valid integer, unable to parse string as
an integer"
+ )
+ mock_client.pools.bulk.assert_not_called()
+
def test_import_success(self, mock_client, tmp_path, capsys):
"""Test successful pool import."""
pools_file = tmp_path / "pools.json"
