fat-catTW commented on PR #72499:
URL: https://github.com/apache/airflow/pull/72499#issuecomment-5556628051
Thanks for the update!
One thing I noticed in the API masking path:
`_build_masked_dag_run_responses()` caches the resolved Dag by `dag_id` only:
```python
dag_cache: dict[str, object] = {}
for dag_run in dag_runs:
response = DAGRunResponse.model_validate(dag_run)
if dag_run.dag_id not in dag_cache:
dag_cache[dag_run.dag_id] = get_dag_for_run(dag_bag, dag_run,
session=session)
response.conf = _mask_password_conf(dag_cache[dag_run.dag_id],
response.conf)
```
However, `get_dag_for_run()` resolves the Dag for that specific DagRun
version, not just for the `dag_id`. Two DagRuns can have the same `dag_id` but
different `created_dag_version_id`s.
For example:
- `run_v1`: `dag_id="my_dag"`, `created_dag_version_id=v1`, where
`api_token` is a normal string Param.
- `run_v2`: `dag_id="my_dag"`, `created_dag_version_id=v2`, where
`api_token` is `Param(..., type="string", format="password")`.
If the list response processes `run_v1` first, the cache stores the v1 Dag
under `dag_cache["my_dag"]`. When it later processes `run_v2`, it reuses the v1
Dag because the `dag_id` is the same, so `run_v2.conf["api_token"]` may not be
masked even though it is a password Param in v2.
Could we either avoid caching here, or key the cache by the Dag version used
for that DagRun instead of only `dag_id`? A regression test with two runs of
the same Dag but different Dag versions, where only one version declares
`format="password"`, would make this behavior clear.
--
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]
