From f96f3e8b7fe37d35eda820f91e2d2c347658133d Mon Sep 17 00:00:00 2001 From: PoAn Yang Date: Sat, 13 Jun 2026 16:53:58 +0900 Subject: [PATCH] refactor: [AIP-94] airflowctl variables: add list command Signed-off-by: PoAn Yang --- .../airflow/cli/commands/variable_command.py | 50 +++------ .../cli/commands/test_command_deprecations.py | 3 +- .../cli/commands/test_variable_command.py | 106 ++++++++++-------- 3 files changed, 77 insertions(+), 82 deletions(-) diff --git a/airflow-core/src/airflow/cli/commands/variable_command.py b/airflow-core/src/airflow/cli/commands/variable_command.py index 5216aabb446c6..9f201e573a559 100644 --- a/airflow-core/src/airflow/cli/commands/variable_command.py +++ b/airflow-core/src/airflow/cli/commands/variable_command.py @@ -25,8 +25,9 @@ from sqlalchemy import select +from airflow.cli.api_client import NEW_API_CLIENT, Client, provide_api_client from airflow.cli.simple_table import AirflowConsole -from airflow.cli.utils import SENSITIVE_PLACEHOLDER, print_export_output +from airflow.cli.utils import SENSITIVE_PLACEHOLDER, deprecated_for_airflowctl, print_export_output from airflow.exceptions import ( AirflowFileParseException, AirflowUnsupportedFileTypeException, @@ -43,29 +44,11 @@ from sqlalchemy.orm.session import Session -class VariableDisplayMapper: - """Mapper class for formatting variable data for CLI display.""" - - @staticmethod - def keys_only(var) -> dict[str, str]: - """Return only variable keys. Accepts Variable model or dict with 'key'.""" - key = var.key if hasattr(var, "key") else var["key"] - return {"key": key} - - @staticmethod - def with_values(var, hide_sensitive: bool = False) -> dict[str, str]: - """Return variable with value, optionally masked.""" - key = var.key if hasattr(var, "key") else var["key"] - raw = var.val if hasattr(var, "val") else var.get("val", var.get("_val")) - val = "" if raw is None else str(raw) - if hide_sensitive: - val = SENSITIVE_PLACEHOLDER - return {"key": key, "val": val} - - +@deprecated_for_airflowctl("airflowctl variables list") @suppress_logs_and_warning @providers_configuration_loaded -def variables_list(args): +@provide_api_client +def variables_list(args, api_client: Client = NEW_API_CLIENT): """ Display all the variables. @@ -79,17 +62,20 @@ def variables_list(args): if hide_sensitive and not show_values: raise SystemExit("--hide-sensitive can only be used with --show-values") - def _mapper(var): - return VariableDisplayMapper.with_values(var, hide_sensitive) + variables = api_client.variables.list().variables - with create_session() as session: - if show_values: - variables = session.scalars(select(Variable)).all() - AirflowConsole().print_as(data=variables, output=args.output, mapper=_mapper) - else: - keys = session.scalars(select(Variable.key).distinct()).all() - variables = [{"key": key} for key in keys] - AirflowConsole().print_as(data=variables, output=args.output, mapper=None) + if show_values: + + def _mapper(var): + val = "" if var.value is None else str(var.value) + if hide_sensitive: + val = SENSITIVE_PLACEHOLDER + return {"key": var.key, "val": val} + + AirflowConsole().print_as(data=variables, output=args.output, mapper=_mapper) + else: + data = [{"key": var.key} for var in variables] + AirflowConsole().print_as(data=data, output=args.output, mapper=None) @suppress_logs_and_warning diff --git a/airflow-core/tests/unit/cli/commands/test_command_deprecations.py b/airflow-core/tests/unit/cli/commands/test_command_deprecations.py index b4eb6840c9069..9e5cd5266d87d 100644 --- a/airflow-core/tests/unit/cli/commands/test_command_deprecations.py +++ b/airflow-core/tests/unit/cli/commands/test_command_deprecations.py @@ -30,7 +30,7 @@ import pytest -from airflow.cli.commands import asset_command, dag_command, pool_command +from airflow.cli.commands import asset_command, dag_command, pool_command, variable_command from airflow.exceptions import RemovedInAirflow4Warning # (command callable, argv to parse, expected airflowctl replacement named in the warning) @@ -52,6 +52,7 @@ ["assets", "materialize", "--name=foo"], "airflowctl assets materialize", ), + (variable_command.variables_list, ["variables", "list"], "airflowctl variables list"), ] diff --git a/airflow-core/tests/unit/cli/commands/test_variable_command.py b/airflow-core/tests/unit/cli/commands/test_variable_command.py index a02c95aa31722..dd2a0a64ff976 100644 --- a/airflow-core/tests/unit/cli/commands/test_variable_command.py +++ b/airflow-core/tests/unit/cli/commands/test_variable_command.py @@ -19,11 +19,10 @@ import json import os -from contextlib import redirect_stdout -from io import StringIO import pytest import yaml +from airflowctl.api.datamodels.generated import VariableResponse from sqlalchemy import select from airflow import models @@ -229,100 +228,109 @@ def test_variables_set_different_types(self): os.remove("variables_types.json") - def test_variables_list(self): + def test_variables_list(self, mock_cli_api_client): """Test variable_list command""" - # Test command is received + mock_cli_api_client.variables.list.return_value.variables = [] variable_command.variables_list(self.parser.parse_args(["variables", "list"])) + mock_cli_api_client.variables.list.assert_called_once() - def test_variables_list_show_values(self): + def test_variables_list_show_values(self, mock_cli_api_client, stdout_capture): """Test variables list with --show-values flag shows actual values.""" - # Create test variables - Variable.set("test_key1", "test_value1") - Variable.set("test_key2", "test_value2") + mock_cli_api_client.variables.list.return_value.variables = [ + VariableResponse(key="test_key1", value="test_value1", is_encrypted=False), + VariableResponse(key="test_key2", value="test_value2", is_encrypted=False), + ] args = self.parser.parse_args(["variables", "list", "--output", "json", "--show-values"]) - with redirect_stdout(StringIO()) as stdout_io: + with stdout_capture as stdout: variable_command.variables_list(args) - output = stdout_io.getvalue() - # Parse JSON output and verify values are shown - data = json.loads(output) - assert len(data) >= 2 + data = json.loads(stdout.getvalue()) key_value_map = {item["key"]: item["val"] for item in data} - assert "test_value1" in key_value_map["test_key1"] - assert "test_value2" in key_value_map["test_key2"] + assert key_value_map["test_key1"] == "test_value1" + assert key_value_map["test_key2"] == "test_value2" - def test_variables_list_hide_sensitive(self): + def test_variables_list_hide_sensitive(self, mock_cli_api_client, stdout_capture): """Test variables list with --hide-sensitive masks all values.""" - # Create test variables - Variable.set("test_key1", "test_value1") - Variable.set("test_key2", "test_value2") + mock_cli_api_client.variables.list.return_value.variables = [ + VariableResponse(key="test_key1", value="test_value1", is_encrypted=False), + VariableResponse(key="test_key2", value="test_value2", is_encrypted=False), + ] args = self.parser.parse_args( ["variables", "list", "--output", "json", "--show-values", "--hide-sensitive"] ) - with redirect_stdout(StringIO()) as stdout_io: + with stdout_capture as stdout: variable_command.variables_list(args) - output = stdout_io.getvalue() - # Parse JSON output and verify values are masked - data = json.loads(output) - assert len(data) >= 2 + data = json.loads(stdout.getvalue()) + assert len(data) == 2 for item in data: - if "test_key" in item["key"]: - assert item["val"] == "***" + assert item["val"] == "***" - def test_variables_list_hide_sensitive_without_show_values_fails(self): + def test_variables_list_hide_sensitive_without_show_values_fails(self, mock_cli_api_client): """--hide-sensitive without --show-values should fail.""" args = self.parser.parse_args(["variables", "list", "--hide-sensitive"]) with pytest.raises(SystemExit, match="--hide-sensitive can only be used with --show-values"): variable_command.variables_list(args) + mock_cli_api_client.variables.list.assert_not_called() - def test_variables_list_default_hides_values(self): + def test_variables_list_default_hides_values(self, mock_cli_api_client, stdout_capture): """By default, variables list should only show keys, not values.""" - Variable.set("test_key1", "test_value1") - Variable.set("test_key2", "test_value2") + mock_cli_api_client.variables.list.return_value.variables = [ + VariableResponse(key="test_key1", value="test_value1", is_encrypted=False), + VariableResponse(key="test_key2", value="test_value2", is_encrypted=False), + ] args = self.parser.parse_args(["variables", "list", "--output", "json"]) - with redirect_stdout(StringIO()) as stdout_io: + with stdout_capture as stdout: variable_command.variables_list(args) - output = stdout_io.getvalue() - data = json.loads(output) - assert len(data) >= 2 + data = json.loads(stdout.getvalue()) + assert len(data) == 2 for item in data: - if "test_key" in item["key"]: - assert "val" not in item + assert "val" not in item - def test_variables_list_edge_cases(self): + def test_variables_list_edge_cases(self, mock_cli_api_client, stdout_capture): """Test variables list with None and empty values.""" - Variable.set("empty_var", "") - Variable.set("none_var", None) - Variable.set("normal_var", "normal_value") + mock_cli_api_client.variables.list.return_value.variables = [ + VariableResponse(key="empty_var", value="", is_encrypted=False), + VariableResponse(key="none_str_var", value="None", is_encrypted=False), + VariableResponse(key="none_var", value=None, is_encrypted=False), + VariableResponse(key="normal_var", value="normal_value", is_encrypted=False), + ] args = self.parser.parse_args(["variables", "list", "--output", "json", "--show-values"]) - with redirect_stdout(StringIO()) as stdout_io: + with stdout_capture as stdout: variable_command.variables_list(args) - output = stdout_io.getvalue() - data = json.loads(output) + data = json.loads(stdout.getvalue()) key_value_map = {item["key"]: item["val"] for item in data} assert key_value_map["empty_var"] == "" - assert key_value_map["none_var"] == "None" + assert key_value_map["none_str_var"] == "None" + assert key_value_map["none_var"] == "" assert key_value_map["normal_var"] == "normal_value" + def test_variables_list_hide_sensitive_masks_edge_case_values(self, mock_cli_api_client, stdout_capture): + """--hide-sensitive masks empty, "None" and null values alike.""" + mock_cli_api_client.variables.list.return_value.variables = [ + VariableResponse(key="empty_var", value="", is_encrypted=False), + VariableResponse(key="none_str_var", value="None", is_encrypted=False), + VariableResponse(key="none_var", value=None, is_encrypted=False), + VariableResponse(key="normal_var", value="normal_value", is_encrypted=False), + ] + args = self.parser.parse_args( ["variables", "list", "--output", "json", "--show-values", "--hide-sensitive"] ) - with redirect_stdout(StringIO()) as stdout_io: + with stdout_capture as stdout: variable_command.variables_list(args) - output = stdout_io.getvalue() - data = json.loads(output) + data = json.loads(stdout.getvalue()) + assert len(data) == 4 for item in data: - if item["key"] in ["empty_var", "none_var", "normal_var"]: - assert item["val"] == "***" + assert item["val"] == "***" def test_variables_delete(self): """Test variable_delete command"""