kacpermuda commented on code in PR #74805:
URL: https://github.com/apache/airflow/pull/74805#discussion_r4379812435
##########
providers/openlineage/src/airflow/providers/openlineage/sqlparser.py:
##########
@@ -489,10 +489,8 @@ def get_openlineage_facets_with_sql(
connection = hook.get_connection(conn_id)
if database_info is None:
- try:
- database_info = hook.get_openlineage_database_info(connection)
- except AttributeError:
- database_info = None
+ get_database_info = getattr(hook, "get_openlineage_database_info",
None)
+ database_info = get_database_info(connection) if get_database_info is
not None else None
Review Comment:
Done. All three now use getattr for the missing-method case, and errors
raised inside the method are logged at WARNING and fall back (None / empty
lineage) in both sqlparser.py and SQLExecuteQueryOperator. This also catches
exceptions other than AttributeError, which previously propagated from the
dialect and default schema calls. I think that’s the right call here, but say
if you’d rather keep it narrower.
##########
providers/openlineage/tests/unit/openlineage/utils/test_sql_hook_lineage.py:
##########
@@ -191,6 +191,42 @@ def
test_sql_parsing_failure_falls_back_to_normalized_sql_facet(self):
assert call.kwargs["job_facets"]["sql"].query ==
SQLParser.normalize_sql("SELECT broken(")
assert call.kwargs["run_facets"]["externalQuery"].externalQueryId ==
"qid-1"
+ def test_sql_parsing_failure_is_logged_as_warning(self, caplog):
+ self.mock_ns.return_value = "ns"
+ self.mock_facets_fn.side_effect = Exception("parse error")
+ mock_ti = mock.MagicMock(dag_id="dag_id", task_id="task_id")
+
+ with caplog.at_level(logging.WARNING, logger=_MODULE):
+ emit_lineage_from_sql_extras(
+ task_instance=mock_ti,
+ sql_extras=[_make_extra(sql="SELECT broken(", job_id="qid-1")],
+ )
+
+ assert "Failed to parse SQL for query 1: parse error" in caplog.text
Review Comment:
Replaced. The new and touched tests patch the module log and assert on the
warning call arguments. I dropped the test_manager.py test, since it only
covered existing extractor behavior that this PR no longer depends on. The
older caplog.text assertions elsewhere in the file are untouched.
--
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]
