From ecdb51c65f7c52f5e1379bc57b5045f5c030fa3b Mon Sep 17 00:00:00 2001 From: pierrejeambrun Date: Mon, 7 Oct 2024 17:30:37 +0200 Subject: [PATCH] AIP-84 Delete Variable --- .../endpoints/variable_endpoint.py | 2 + airflow/api_fastapi/openapi/v1-generated.yaml | 41 ++++++++++++ airflow/api_fastapi/views/public/__init__.py | 2 + airflow/api_fastapi/views/public/variables.py | 42 ++++++++++++ airflow/ui/openapi-gen/queries/common.ts | 4 ++ airflow/ui/openapi-gen/queries/queries.ts | 40 +++++++++++ .../ui/openapi-gen/requests/services.gen.ts | 30 +++++++++ airflow/ui/openapi-gen/requests/types.gen.ts | 33 ++++++++++ .../views/public/test_variables.py | 66 +++++++++++++++++++ 9 files changed, 260 insertions(+) create mode 100644 airflow/api_fastapi/views/public/variables.py create mode 100644 tests/api_fastapi/views/public/test_variables.py diff --git a/airflow/api_connexion/endpoints/variable_endpoint.py b/airflow/api_connexion/endpoints/variable_endpoint.py index 1375484a422fc..9413f9158652d 100644 --- a/airflow/api_connexion/endpoints/variable_endpoint.py +++ b/airflow/api_connexion/endpoints/variable_endpoint.py @@ -31,6 +31,7 @@ from airflow.api_connexion.schemas.variable_schema import variable_collection_schema, variable_schema from airflow.models import Variable from airflow.security import permissions +from airflow.utils.api_migration import mark_fastapi_migration_done from airflow.utils.log.action_logger import action_event_from_permission from airflow.utils.session import NEW_SESSION, provide_session from airflow.www.decorators import action_logging @@ -43,6 +44,7 @@ RESOURCE_EVENT_PREFIX = "variable" +@mark_fastapi_migration_done @security.requires_access_variable("DELETE") @action_logging( event=action_event_from_permission( diff --git a/airflow/api_fastapi/openapi/v1-generated.yaml b/airflow/api_fastapi/openapi/v1-generated.yaml index d272dd03d9302..8cbed11db61ae 100644 --- a/airflow/api_fastapi/openapi/v1-generated.yaml +++ b/airflow/api_fastapi/openapi/v1-generated.yaml @@ -455,6 +455,47 @@ paths: application/json: schema: $ref: '#/components/schemas/HTTPValidationError' + /public/variables/{variable_key}: + delete: + tags: + - Variable + summary: Delete Variable + description: Delete a variable entry. + operationId: delete_variable + parameters: + - name: variable_key + in: path + required: true + schema: + type: string + title: Variable Key + responses: + '204': + description: Successful Response + '401': + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPExceptionResponse' + description: Unauthorized + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPExceptionResponse' + description: Forbidden + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPExceptionResponse' + description: Not Found + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' components: schemas: ConnectionResponse: diff --git a/airflow/api_fastapi/views/public/__init__.py b/airflow/api_fastapi/views/public/__init__.py index 9c0eefebb875e..4e02d9ab43bcf 100644 --- a/airflow/api_fastapi/views/public/__init__.py +++ b/airflow/api_fastapi/views/public/__init__.py @@ -19,6 +19,7 @@ from airflow.api_fastapi.views.public.connections import connections_router from airflow.api_fastapi.views.public.dags import dags_router +from airflow.api_fastapi.views.public.variables import variables_router from airflow.api_fastapi.views.router import AirflowRouter public_router = AirflowRouter(prefix="/public") @@ -26,3 +27,4 @@ public_router.include_router(dags_router) public_router.include_router(connections_router) +public_router.include_router(variables_router) diff --git a/airflow/api_fastapi/views/public/variables.py b/airflow/api_fastapi/views/public/variables.py new file mode 100644 index 0000000000000..e4edb8601fd09 --- /dev/null +++ b/airflow/api_fastapi/views/public/variables.py @@ -0,0 +1,42 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +from fastapi import Depends, HTTPException +from sqlalchemy.orm import Session +from typing_extensions import Annotated + +from airflow.api_fastapi.db.common import get_session +from airflow.api_fastapi.openapi.exceptions import create_openapi_http_exception_doc +from airflow.api_fastapi.views.router import AirflowRouter +from airflow.models.variable import Variable + +variables_router = AirflowRouter(tags=["Variable"], prefix="/variables") + + +@variables_router.delete( + "/{variable_key}", + status_code=204, + responses=create_openapi_http_exception_doc([401, 403, 404]), +) +async def delete_variable( + variable_key: str, + session: Annotated[Session, Depends(get_session)], +): + """Delete a variable entry.""" + if Variable.delete(variable_key, session) == 0: + raise HTTPException(404, f"The Variable with key: `{variable_key}` was not found") diff --git a/airflow/ui/openapi-gen/queries/common.ts b/airflow/ui/openapi-gen/queries/common.ts index ff7bb3995cb49..afd7afd5bcc06 100644 --- a/airflow/ui/openapi-gen/queries/common.ts +++ b/airflow/ui/openapi-gen/queries/common.ts @@ -5,6 +5,7 @@ import { AssetService, ConnectionService, DagService, + VariableService, } from "../requests/services.gen"; import { DagRunState } from "../requests/types.gen"; @@ -119,3 +120,6 @@ export type DagServicePatchDagMutationResult = Awaited< export type ConnectionServiceDeleteConnectionMutationResult = Awaited< ReturnType >; +export type VariableServiceDeleteVariableMutationResult = Awaited< + ReturnType +>; diff --git a/airflow/ui/openapi-gen/queries/queries.ts b/airflow/ui/openapi-gen/queries/queries.ts index 4aa627d74fd0c..22d58eadda0cb 100644 --- a/airflow/ui/openapi-gen/queries/queries.ts +++ b/airflow/ui/openapi-gen/queries/queries.ts @@ -10,6 +10,7 @@ import { AssetService, ConnectionService, DagService, + VariableService, } from "../requests/services.gen"; import { DAGPatchBody, DagRunState } from "../requests/types.gen"; import * as Common from "./common"; @@ -345,3 +346,42 @@ export const useConnectionServiceDeleteConnection = < }) as unknown as Promise, ...options, }); +/** + * Delete Variable + * Delete a variable entry. + * @param data The data for the request. + * @param data.variableKey + * @returns void Successful Response + * @throws ApiError + */ +export const useVariableServiceDeleteVariable = < + TData = Common.VariableServiceDeleteVariableMutationResult, + TError = unknown, + TContext = unknown, +>( + options?: Omit< + UseMutationOptions< + TData, + TError, + { + variableKey: string; + }, + TContext + >, + "mutationFn" + >, +) => + useMutation< + TData, + TError, + { + variableKey: string; + }, + TContext + >({ + mutationFn: ({ variableKey }) => + VariableService.deleteVariable({ + variableKey, + }) as unknown as Promise, + ...options, + }); diff --git a/airflow/ui/openapi-gen/requests/services.gen.ts b/airflow/ui/openapi-gen/requests/services.gen.ts index 023a2a458dd7b..8da678e019232 100644 --- a/airflow/ui/openapi-gen/requests/services.gen.ts +++ b/airflow/ui/openapi-gen/requests/services.gen.ts @@ -17,6 +17,8 @@ import type { DeleteConnectionResponse, GetConnectionData, GetConnectionResponse, + DeleteVariableData, + DeleteVariableResponse, } from "./types.gen"; export class AssetService { @@ -246,3 +248,31 @@ export class ConnectionService { }); } } + +export class VariableService { + /** + * Delete Variable + * Delete a variable entry. + * @param data The data for the request. + * @param data.variableKey + * @returns void Successful Response + * @throws ApiError + */ + public static deleteVariable( + data: DeleteVariableData, + ): CancelablePromise { + return __request(OpenAPI, { + method: "DELETE", + url: "/public/variables/{variable_key}", + path: { + variable_key: data.variableKey, + }, + errors: { + 401: "Unauthorized", + 403: "Forbidden", + 404: "Not Found", + 422: "Validation Error", + }, + }); + } +} diff --git a/airflow/ui/openapi-gen/requests/types.gen.ts b/airflow/ui/openapi-gen/requests/types.gen.ts index 65a2db8926510..85fb8030c6f7e 100644 --- a/airflow/ui/openapi-gen/requests/types.gen.ts +++ b/airflow/ui/openapi-gen/requests/types.gen.ts @@ -222,6 +222,12 @@ export type GetConnectionData = { export type GetConnectionResponse = ConnectionResponse; +export type DeleteVariableData = { + variableKey: string; +}; + +export type DeleteVariableResponse = void; + export type $OpenApiTs = { "/ui/next_run_assets/{dag_id}": { get: { @@ -398,4 +404,31 @@ export type $OpenApiTs = { }; }; }; + "/public/variables/{variable_key}": { + delete: { + req: DeleteVariableData; + res: { + /** + * Successful Response + */ + 204: void; + /** + * Unauthorized + */ + 401: HTTPExceptionResponse; + /** + * Forbidden + */ + 403: HTTPExceptionResponse; + /** + * Not Found + */ + 404: HTTPExceptionResponse; + /** + * Validation Error + */ + 422: HTTPValidationError; + }; + }; + }; }; diff --git a/tests/api_fastapi/views/public/test_variables.py b/tests/api_fastapi/views/public/test_variables.py new file mode 100644 index 0000000000000..56d65b98b6589 --- /dev/null +++ b/tests/api_fastapi/views/public/test_variables.py @@ -0,0 +1,66 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import pytest + +from airflow.models.variable import Variable +from airflow.utils.session import provide_session +from tests.test_utils.db import clear_db_variables + +pytestmark = pytest.mark.db_test + +TEST_VARIABLE_KEY = "test_variable_key" +TEST_VARIABLE_VAL = 3 +TEST_VARIABLE_DESCRIPTION = "Some description for the variable" +TEST_CONN_TYPE = "test_type" + + +@provide_session +def _create_variable(session) -> None: + Variable.set( + key=TEST_VARIABLE_KEY, value=TEST_VARIABLE_VAL, description=TEST_VARIABLE_DESCRIPTION, session=session + ) + + +class TestVariableEndpoint: + @pytest.fixture(autouse=True) + def setup(self) -> None: + clear_db_variables() + + def teardown_method(self) -> None: + clear_db_variables() + + def create_variable(self): + _create_variable() + + +class TestDeleteVariable(TestVariableEndpoint): + def test_delete_should_respond_204(self, test_client, session): + self.create_variable() + variables = session.query(Variable).all() + assert len(variables) == 1 + response = test_client.delete(f"/public/variables/{TEST_VARIABLE_KEY}") + assert response.status_code == 204 + variables = session.query(Variable).all() + assert len(variables) == 0 + + def test_delete_should_respond_404(self, test_client): + response = test_client.delete(f"/public/variables/{TEST_VARIABLE_KEY}") + assert response.status_code == 404 + body = response.json() + assert f"The Variable with key: `{TEST_VARIABLE_KEY}` was not found" == body["detail"]