From 6ae1b6d1a0c8cd7435c355bafae9744e063e2778 Mon Sep 17 00:00:00 2001 From: PoAn Yang Date: Sat, 13 Jun 2026 17:55:46 +0900 Subject: [PATCH] refactor: [AIP-94] airflowctl variables: add delete command Signed-off-by: PoAn Yang --- .../airflow/cli/commands/variable_command.py | 16 ++++++++++--- .../cli/commands/test_command_deprecations.py | 3 ++- .../cli/commands/test_variable_command.py | 23 +++++++++++++++---- 3 files changed, 33 insertions(+), 9 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..b10d46dda958a 100644 --- a/airflow-core/src/airflow/cli/commands/variable_command.py +++ b/airflow-core/src/airflow/cli/commands/variable_command.py @@ -23,10 +23,12 @@ import os from typing import TYPE_CHECKING +from airflowctl.api.operations import ServerResponseError 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, @@ -116,10 +118,18 @@ def variables_set(args): @cli_utils.action_cli +@deprecated_for_airflowctl("airflowctl variables delete") +@suppress_logs_and_warning @providers_configuration_loaded -def variables_delete(args): +@provide_api_client +def variables_delete(args, api_client: Client = NEW_API_CLIENT): """Delete variable by a given name.""" - Variable.delete(args.key) + try: + api_client.variables.delete(variable_key=args.key) + except ServerResponseError as e: + if e.response.status_code == 404: + raise SystemExit(f"Variable {args.key} does not exist") + raise print(f"Variable {args.key} deleted") 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..e7b5b71920d5b 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_delete, ["variables", "delete", "foo"], "airflowctl variables delete"), ] 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..4db723dc72d01 100644 --- a/airflow-core/tests/unit/cli/commands/test_variable_command.py +++ b/airflow-core/tests/unit/cli/commands/test_variable_command.py @@ -22,8 +22,10 @@ from contextlib import redirect_stdout from io import StringIO +import httpx import pytest import yaml +from airflowctl.api.operations import ServerResponseError from sqlalchemy import select from airflow import models @@ -37,6 +39,12 @@ pytestmark = pytest.mark.db_test +def _server_error(status_code: int) -> ServerResponseError: + request = httpx.Request("GET", "http://testserver/api/v2/variables/foo") + response = httpx.Response(status_code, request=request, json={"detail": "boom"}) + return ServerResponseError(message="boom", request=request, response=response) + + # Test data fixtures @pytest.fixture def simple_variable_data(): @@ -324,12 +332,16 @@ def test_variables_list_edge_cases(self): if item["key"] in ["empty_var", "none_var", "normal_var"]: assert item["val"] == "***" - def test_variables_delete(self): + def test_variables_delete(self, mock_cli_api_client): """Test variable_delete command""" - variable_command.variables_set(self.parser.parse_args(["variables", "set", "foo", "bar"])) variable_command.variables_delete(self.parser.parse_args(["variables", "delete", "foo"])) - with pytest.raises(KeyError): - Variable.get("foo") + mock_cli_api_client.variables.delete.assert_called_once_with(variable_key="foo") + + def test_variables_delete_missing(self, mock_cli_api_client): + """Deleting a missing variable exits with an error.""" + mock_cli_api_client.variables.delete.side_effect = _server_error(404) + with pytest.raises(SystemExit): + variable_command.variables_delete(self.parser.parse_args(["variables", "delete", "foo"])) @pytest.mark.parametrize( "filename", @@ -372,7 +384,8 @@ def test_variables_isolation(self, tmp_path): variable_command.variables_set(self.parser.parse_args(["variables", "set", "bar", "updated"])) variable_command.variables_set(self.parser.parse_args(["variables", "set", "foo", '{"foo":"oops"}'])) - variable_command.variables_delete(self.parser.parse_args(["variables", "delete", "foo"])) + # ``delete`` is migrated to airflowctl, so remove via the model here + Variable.delete("foo") with create_session() as session: variable_command.variables_import( self.parser.parse_args(["variables", "import", os.fspath(path1)]), session=session