SameerMesiah97 commented on code in PR #69109:
URL: https://github.com/apache/airflow/pull/69109#discussion_r3500012563
##########
providers/openlineage/src/airflow/providers/openlineage/utils/utils.py:
##########
@@ -980,9 +985,12 @@ class DagRunInfo(InfoJsonEncodable):
"dag_bundle_version": lambda dagrun:
DagRunInfo.dag_version_info(dagrun, "bundle_version"),
"dag_version_id": lambda dagrun: DagRunInfo.dag_version_info(dagrun,
"version_id"),
"dag_version_number": lambda dagrun:
DagRunInfo.dag_version_info(dagrun, "version_number"),
+ "dag_team_name": lambda dagrun: DagRunInfo.team_name(dagrun) if
AIRFLOW_V_3_3_PLUS else None,
"deadlines": lambda dagrun: DagRunInfo.deadlines(dagrun),
}
+ _team_name_cache: ClassVar[dict[str, str | None]] = {}
Review Comment:
I think using a regular cache is acceptable for the following reasons:
1) Bundles per team are expected to change very rarely and most certainly
not during the DAG run.
2) This is provider-layer code that runs in the worker in a short-lived
process bounded by the lifetime of the task, so the cache will likely not
remain stale for very long.
3) Is it expected for OpenLineage events to pick up bundle-to-team
relationships that change mid-flight? I don't believe so.
Also, if we were to switch to `DagModel.get_team_name(dag_id)`, there would
be a slight performance hit as we'd have to go through the `DagModel` table to
get the DAG bundle, whereas with `DagBundleModel.get_team_name(bundle_name)`,
we can use the bundle name directly to fetch the team name.
But if you insist on a TTL cache, I think the best option would be to use
`DagModel.get_team_name(dag_id)`. However, I am not sure if it worth it.
--
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]
