Allow empty value while variable creation (apache#45402)

* fix * fix backend * added tests
PolideaInternal · Jan 4, 2025 · 8a3d0f4 · 8a3d0f4
1 parent de6d83a
commit 8a3d0f4
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 34 deletions.
diff --git a/airflow/api_fastapi/core_api/datamodels/variables.py b/airflow/api_fastapi/core_api/datamodels/variables.py
@@ -33,7 +33,7 @@ class VariableResponse(BaseModel):
     model_config = ConfigDict(populate_by_name=True, from_attributes=True)
 
     key: str
-    val: str | None = Field(alias="value")
+    val: str = Field(alias="value")
     description: str | None
     is_encrypted: bool
 
@@ -56,7 +56,7 @@ class VariableBody(BaseModel):
     """Variable serializer for bodies."""
 
     key: str = Field(max_length=ID_LEN)
-    value: str | None = Field(serialization_alias="val")
+    value: str = Field(serialization_alias="val")
     description: str | None = Field(default=None)
 
 

diff --git a/airflow/api_fastapi/core_api/openapi/v1-generated.yaml b/airflow/api_fastapi/core_api/openapi/v1-generated.yaml
@@ -9736,9 +9736,7 @@ components:
           maxLength: 250
           title: Key
         value:
-          anyOf:
-          - type: string
-          - type: 'null'
+          type: string
           title: Value
         description:
           anyOf:
@@ -9773,9 +9771,7 @@ components:
           type: string
           title: Key
         value:
-          anyOf:
-          - type: string
-          - type: 'null'
+          type: string
           title: Value
         description:
           anyOf:

diff --git a/airflow/ui/openapi-gen/requests/schemas.gen.ts b/airflow/ui/openapi-gen/requests/schemas.gen.ts
@@ -5485,14 +5485,7 @@ export const $VariableBody = {
       title: "Key",
     },
     value: {
-      anyOf: [
-        {
-          type: "string",
-        },
-        {
-          type: "null",
-        },
-      ],
+      type: "string",
       title: "Value",
     },
     description: {
@@ -5540,14 +5533,7 @@ export const $VariableResponse = {
       title: "Key",
     },
     value: {
-      anyOf: [
-        {
-          type: "string",
-        },
-        {
-          type: "null",
-        },
-      ],
+      type: "string",
       title: "Value",
     },
     description: {

diff --git a/airflow/ui/openapi-gen/requests/types.gen.ts b/airflow/ui/openapi-gen/requests/types.gen.ts
@@ -1276,7 +1276,7 @@ export type ValidationError = {
  */
 export type VariableBody = {
   key: string;
-  value: string | null;
+  value: string;
   description?: string | null;
 };
 
@@ -1293,7 +1293,7 @@ export type VariableCollectionResponse = {
  */
 export type VariableResponse = {
   key: string;
-  value: string | null;
+  value: string;
   description: string | null;
   is_encrypted: boolean;
 };

diff --git a/airflow/ui/src/pages/Variables/ManageVariable/EditVariableButton.tsx b/airflow/ui/src/pages/Variables/ManageVariable/EditVariableButton.tsx
@@ -36,7 +36,7 @@ const EditVariableButton = ({ variable }: Props) => {
   const initialVariableValue: VariableBody = {
     description: variable.description ?? "",
     key: variable.key,
-    value: variable.value ?? "",
+    value: variable.value,
   };
   const { editVariable, error, isPending, setError } = useEditVariable(initialVariableValue, {
     onSuccessConfirm: onClose,

diff --git a/airflow/ui/src/pages/Variables/ManageVariable/VariableForm.tsx b/airflow/ui/src/pages/Variables/ManageVariable/VariableForm.tsx
@@ -80,18 +80,14 @@ const VariableForm = ({ error, initialVariable, isPending, manageMutate, setErro
       <Controller
         control={control}
         name="value"
-        render={({ field, fieldState }) => (
-          <Field.Root invalid={Boolean(fieldState.error)} mt={4} required>
+        render={({ field }) => (
+          <Field.Root mt={4}>
             <Field.Label fontSize="md">
               Value <Field.RequiredIndicator />
             </Field.Label>
-            <Textarea {...field} required size="sm" />
-            {fieldState.error ? <Field.ErrorText>{fieldState.error.message}</Field.ErrorText> : undefined}
+            <Textarea {...field} size="sm" />
           </Field.Root>
         )}
-        rules={{
-          required: "Value is required",
-        }}
       />
 
       <Controller

diff --git a/tests/api_fastapi/core_api/routes/public/test_variables.py b/tests/api_fastapi/core_api/routes/public/test_variables.py
@@ -356,6 +356,19 @@ class TestPostVariable(TestVariableEndpoint):
                     "is_encrypted": True,
                 },
             ),
+            (
+                {
+                    "key": "empty value variable",
+                    "value": "",
+                    "description": "some description",
+                },
+                {
+                    "key": "empty value variable",
+                    "value": "",
+                    "description": "some description",
+                    "is_encrypted": True,
+                },
+            ),
         ],
     )
     def test_post_should_respond_201(self, test_client, session, body, expected_response):
@@ -400,6 +413,25 @@ def test_post_should_respond_422_when_key_too_large(self, test_client):
             ]
         }
 
+    def test_post_should_respond_422_when_value_is_null(self, test_client):
+        body = {
+            "key": "null value key",
+            "value": None,
+            "description": "key too large",
+        }
+        response = test_client.post("/public/variables", json=body)
+        assert response.status_code == 422
+        assert response.json() == {
+            "detail": [
+                {
+                    "type": "string_type",
+                    "loc": ["body", "value"],
+                    "msg": "Input should be a valid string",
+                    "input": None,
+                }
+            ]
+        }
+
 
 class TestImportVariables(TestVariableEndpoint):
     @pytest.mark.enable_redact