Copilot commented on code in PR #63343:
URL: https://github.com/apache/airflow/pull/63343#discussion_r3066481406
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_variables.py:
##########
@@ -254,6 +254,38 @@ def test_get_should_respond_404(self, test_client):
body = response.json()
assert f"The Variable with key: `{TEST_VARIABLE_KEY}` was not found"
== body["detail"]
+ def test_get_variable_with_undecryptable_val(self, test_client, session):
+ """Variable with an undecryptable _val (e.g. migrated with a different
Fernet key) should
+ return 200 with value=None instead of raising a 500 serialization
error."""
+ var = Variable(key="bad_encrypted_var")
+ var._val = "not-a-valid-fernet-token"
+ var.is_encrypted = True
+ var.description = "migrated variable"
+ session.add(var)
+ session.commit()
+
Review Comment:
The setup for creating an undecryptable variable is duplicated in two tests;
consider extracting a small helper/fixture so future changes (e.g. adding
team_name or additional required fields) don't drift between the single-get and
list endpoint cases.
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/variables.py:
##########
@@ -32,7 +32,7 @@ class VariableResponse(BaseModel):
"""Variable serializer for responses."""
key: str
- val: str = Field(alias="value")
+ val: str | None = Field(alias="value")
Review Comment:
The persisted OpenAPI spec file
(`airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml`)
needs to be regenerated and committed after making `VariableResponse.value`
nullable; it currently documents `value` as a required string, which will be
out of sync with the runtime schema and downstream client codegen (e.g.
airflow-ctl).
```suggestion
val: str = Field(alias="value")
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
