ferruzzi commented on code in PR #69109:
URL: https://github.com/apache/airflow/pull/69109#discussion_r3531462760
##########
providers/openlineage/src/airflow/providers/openlineage/plugins/listener.py:
##########
@@ -291,10 +304,23 @@ def on_running():
},
)
event_size = len(Serde.to_json(redacted_event).encode("utf-8"))
+
+ airflow_facet = redacted_event.run.facets.get("airflow")
+ team_name = None
Review Comment:
You should still have team_name from line 247, no? I don't think it's
possible for these not to result in the same value intentionally.
##########
providers/openlineage/tests/unit/openlineage/utils/test_utils.py:
##########
@@ -331,6 +333,57 @@ def test_dag_run_version(key):
assert result == key
[email protected]_test
[email protected](not AIRFLOW_V_3_3_PLUS, reason="multi-team requires
Airflow 3.3+")
+@patch("airflow.providers.openlineage.utils.utils.DagBundleModel.get_team_name")
+@patch("airflow.providers.openlineage.utils.utils.airflow_conf.getboolean",
return_value=True)
+def test_dag_run_team_name(
+ mock_getboolean,
+ mock_get_team_name,
+):
+ DagRunInfo._team_name_cache.clear()
+
+ dagrun_mock = MagicMock(DagRun)
+ dagrun_mock.dag_versions = [
+ MagicMock(
+ bundle_name="bundle_name",
+ bundle_version="bundle_version",
+ id="version_id",
+ version_number="version_number",
+ )
+ ]
+
+ mock_get_team_name.return_value = "team_a"
+
+ assert DagRunInfo.team_name(dagrun_mock) == "team_a"
+ assert DagRunInfo.team_name(dagrun_mock) == "team_a"
+
+ mock_get_team_name.assert_called_once_with("bundle_name")
+
+
[email protected]_test
[email protected](not AIRFLOW_V_3_3_PLUS, reason="multi-team requires
Airflow 3.1+")
Review Comment:
Here and below, copypasta typo? Pretty sure those should be 3.3+ to match
the skipif, not 3.1+.
##########
providers/openlineage/src/airflow/providers/openlineage/plugins/listener.py:
##########
@@ -241,9 +243,19 @@ def on_running():
if not doc:
doc, doc_type = get_dag_documentation(dag)
+ team_name = None
+ team_name = DagRunInfo.team_name(dagrun)
Review Comment:
Looks like an unnecessary double assignment here and a couple times below?
--
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]
