ColtenOuO opened a new pull request, #71015:
URL: https://github.com/apache/airflow/pull/71015
closes: #71010
## Sumarry
`VariableBody.value` is typed `JsonValue`, so the Variables API accepts any
JSON type
— that was deliberate, added in #49844 so the API could take the same files
the CLI
imports. What that PR did not change is the storage path, which still
assumes a
string. Three write paths, three different outcomes:
```console
# create — 201, silently stores a Python repr
$ curl -X POST .../variables -d '{"key":"v","value":["a","b"]}'
201 stored: ['a', 'b'] # json.loads() raises
# patch — 500
$ curl -X PATCH .../variables/v -d '{"key":"v","value":["a","b"]}'
500 TypeError: encoding without a string argument
# bulk (what the UI's "import variables" uses) — dicts and lists are fine,
the rest are not
stored: true -> "True" # Python capital T
```
The create case is the dangerous one: it reports success, the UI shows
something that
looks plausible, and the failure only surfaces days later when a Dag calls
`Variable.get(key, deserialize_json=True)` and hits a `JSONDecodeError` with
nothing
pointing back at the write.
Beyond the report, `bool` and `null` are corrupted too (`True` / `None`
rather than
`true` / `null`), and `int` is only correct by coincidence — `str(7)`
happens to be
valid JSON.
## Fix
`Variable.set` falls back to `str(value)` and `Variable.val`'s setter calls
`bytes(value, "utf-8")`; both already assume a string. So rather than
teaching each
consumer to serialize, the body serializes once and every consumer keeps its
existing
assumption:
```python
@field_validator("value")
@classmethod
def serialize_non_string_value(cls, value: JsonValue) -> JsonValue:
if isinstance(value, str):
return value
return json.dumps(value, indent=2)
```
Strings pass through untouched — they are already the stored form, and
re-encoding
one would add a layer of quotes on every write.
`indent=2` is what the bulk path was already producing for dicts and lists,
so the
bytes it writes are unchanged; only the two broken paths move. That also
makes the
`serialize_json` branch in `BulkVariableService` redundant — it has to go,
or the
value would be encoded twice.
The issue suggests narrowing the field to `str | None` instead. That would
undo
#49844 and reopen #49837, so this keeps JSON values accepted and stores them
properly.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (Opus 5)
--
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]
