kacpermuda commented on code in PR #39530:
URL: https://github.com/apache/airflow/pull/39530#discussion_r1686256814
##########
dev/breeze/tests/test_packages.py:
##########
@@ -236,6 +236,7 @@ def test_get_install_requirements(provider: str,
version_suffix: str, expected:
"apache.beam": ["apache-airflow-providers-apache-beam",
"apache-beam[gcp]"],
"apache.cassandra":
["apache-airflow-providers-apache-cassandra"],
"cncf.kubernetes":
["apache-airflow-providers-cncf-kubernetes>=7.2.0"],
+ "common.compat": ["apache-airflow-providers-common-compat"],
Review Comment:
Should we also include `>=1.1.0` here? Just making sure it's a explicit
decision, resolve this if it is.
##########
tests/providers/google/cloud/transfers/test_bigquery_to_gcs.py:
##########
@@ -23,20 +23,20 @@
import pytest
from google.cloud.bigquery.retry import DEFAULT_RETRY
from google.cloud.bigquery.table import Table
-from openlineage.client.facet import (
+
+from airflow.exceptions import TaskDeferred
+from airflow.providers.common.compat.openlineage.facet import (
ColumnLineageDatasetFacet,
- ColumnLineageDatasetFacetFieldsAdditional,
- ColumnLineageDatasetFacetFieldsAdditionalInputFields,
+ Dataset,
DocumentationDatasetFacet,
ExternalQueryRunFacet,
+ Fields,
+ Identifier,
+ InputField,
SchemaDatasetFacet,
- SchemaField,
+ SchemaDatasetFacetFields,
SymlinksDatasetFacet,
- SymlinksDatasetFacetIdentifiers,
)
-from openlineage.client.run import Dataset
Review Comment:
That's a general note for all the files, maybe we should do a pre-commit
that will check if there are any imports from `openlineage.client.run` or
`openlineage.client.facet` and point the users to airflow.providers.compat or
the v2 facets. OL support can be implemented in all providers so this kind of
global pre-commit, just simple grep based one, would make sense.
##########
airflow/providers/openlineage/extractors/base.py:
##########
@@ -17,27 +17,34 @@
from __future__ import annotations
+import warnings
from abc import ABC, abstractmethod
-from typing import TYPE_CHECKING
+from typing import Generic, TypeVar, Union
from attrs import Factory, define
+from openlineage.client.event_v2 import Dataset as OLDataset
+
+with warnings.catch_warnings():
+ warnings.simplefilter("ignore", DeprecationWarning)
+ from openlineage.client.facet import BaseFacet as BaseFacet_V1
Review Comment:
I'm not i understand this import from v1 and the logic that follows it, why
do we still need it? If somebody uses the new provider version, they must use
the OL client that support v2 facets right? I must be missing something here 😄
##########
tests/providers/dbt/cloud/utils/test_openlineage.py:
##########
@@ -42,18 +44,12 @@ def json(self):
def emit_event(event):
- run_id = TASK_UUID
- name = f"{DAG_ID}.{TASK_ID}"
- run_obj = event.run.facets["parent"].run
- job_obj = event.run.facets["parent"].job
- if isinstance(run_obj, dict):
- assert run_obj["runId"] == run_id
+ if parse(__version__) >= parse("1.15.0"):
Review Comment:
It's obvious now, but maybe we can leave some inline comment that this
version introduced V2 facets?
--
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]
