refactor: [AIP-94] airflowctl variables: add list command

Signed-off-by: PoAn Yang <payang@apache.org>

Author: FrankYang0529

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

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"),
 ]
 
 

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,101 +228,90 @@ 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"
 
-        args = self.parser.parse_args(
-            ["variables", "list", "--output", "json", "--show-values", "--hide-sensitive"]
-        )
-        with redirect_stdout(StringIO()) as stdout_io:
-            variable_command.variables_list(args)
-            output = stdout_io.getvalue()
-
-        data = json.loads(output)
-        for item in data:
-            if item["key"] in ["empty_var", "none_var", "normal_var"]:
-                assert item["val"] == "***"
-
     def test_variables_delete(self):
         """Test variable_delete command"""
         variable_command.variables_set(self.parser.parse_args(["variables", "set", "foo", "bar"]))