ferruzzi commented on PR #71307:
URL: https://github.com/apache/airflow/pull/71307#issuecomment-5610564977
First, I'm sorry this sat over a month without any reviews, we really got
swarmed with AI-generated contributions and didn't (still don't, TBH) have a
good way to filter them out.
That said, nice troubleshooting. A permanent 404 for a secret that exists
is a real bug, and the `None` caching makes it worse. But I think the retry is
a bandaid rather than the fix. Once you exhaust `max_db_retries`, you still
get the same 404 and it is still cached for `cache_ttl_seconds`.
The root cause looks like it is that both callers accept a database failure
as missing data. `get_connection_from_secrets` already distinguishes one class
of error:
```python
except AirflowSecretsBackendAccessDenied:
# Authoritative deny — must NOT fall through to a less-restrictive
backend.
raise
except Exception:
...fall through...
```
...and then, after the loop, `raise AirflowNotFoundException(f"The conn_id
{conn_id} isn't defined")`. `(DBAPIError, StaleDataError)` belongs in that
carve-out for exactly the same reason: it isn't an answer about whether the
secret exists.
Here's what I'd suggest: keep trying the remaining backends since a
metastore blip shouldn't stop Vault from answering, but track that a backend
errored. Then, if nothing is found, surface a database error rather than
not-found, and skip the `save_variable(key, None)` in that case.
One more reason to prefer that over the decorator: `retry_db_transaction`
calls `session.rollback()`, and `MetastoreBackend.get_variable` takes a
`session` argument that callers will be supplying once #71968 lands. That
means it can roll back a transaction it doesn't own, which is the problem
#71968 exists to avoid. Fixing the classification sidesteps the session
question entirely.
Happy to be argued out of it if you think the retry is the better solution.
It's an improvement either way, just not one that closes the 404 bug.
--
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]
