bramhanandlingala opened a new pull request, #69606:
URL: https://github.com/apache/airflow/pull/69606
## Description
While looking into #69595, I checked `Variable.set()` and `Variable.get()`
in `airflow-core/src/airflow/models/variable.py`, since they're two of the most
commonly used public APIs in Airflow — called directly in DAG code all the time.
**Root cause:**
Neither function validated the `key` argument before doing anything with it.
In `Variable.set()`, the key flowed straight through to a database write. The
`key` column is declared NOT NULL with no `nullable=True`, so
`Variable.set(None, "x")` reached the database before failing, surfacing a raw
`sqlalchemy.exc.IntegrityError` with a stack trace that doesn't say what
actually went wrong. `Variable.set("", "x")` was worse — an empty string
satisfies NOT NULL, so it silently succeeded and created a variable with an
unusable empty-string key.
`Variable.get()` had a milder version of the same gap. `Variable.get(None)`
doesn't crash, but it queries the database for a variable literally named
`None`, finds nothing, and raises `KeyError("Variable None does not exist.")` —
a message that reads like there's a real variable you're missing, not that the
key itself was invalid.
**Fix:**
Added the same check to the top of both methods, before any database or
Task-SDK redirect logic runs:
```python
if not key or not isinstance(key, str):
raise ValueError("Variable key must be a non-empty string")
```
This catches all three cases the issue describes — `None`, empty string, and
non-string values — with one consistent, descriptive error instead of a
database exception or a misleading `KeyError`.
Didn't need to touch `setdefault()` separately since it calls `get()`
internally as its first line, so it inherits the same validation automatically.
Added a test to confirm that rather than just assuming it.
related: #69595
--
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]
