You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "JDarDagran (via GitHub)" <gi...@apache.org> on 2024/05/09 18:58:55 UTC
[PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
JDarDagran opened a new pull request, #39530:
URL: https://github.com/apache/airflow/pull/39530
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
In https://github.com/OpenLineage/OpenLineage/pull/2520 released in `openlineage-python==1.13.1` there were introduced V2 classes for facets and events. This PR aims to migrate existing code to use them. PRs related to other providers using deprecated classes will follow this one.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2123741191
> I ran `breeze testing tests --upgrade-boto tests/providers/docker` in main branch with latest CI image - getting same error results.
https://github.com/apache/airflow/pull/39747 should fix the docker test in "Latest Botocore".
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "mobuchowski (via GitHub)" <gi...@apache.org>.
mobuchowski merged PR #39530:
URL: https://github.com/apache/airflow/pull/39530
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39530:
URL: https://github.com/apache/airflow/pull/39530#discussion_r1684245109
##########
airflow/providers/amazon/provider.yaml:
##########
@@ -89,6 +89,7 @@ versions:
dependencies:
- apache-airflow>=2.7.0
+ - apache-airflow-providers-common-compat
Review Comment:
```suggestion
- apache-airflow-providers-common-compat>=1.1.0
```
everywhere, and common.compat version should also be bumpted to 1.1.0
##########
airflow/providers/amazon/provider.yaml:
##########
@@ -89,6 +89,7 @@ versions:
dependencies:
- apache-airflow>=2.7.0
+ - apache-airflow-providers-common-compat
Review Comment:
```suggestion
- apache-airflow-providers-common-compat>=1.1.0
```
everywhere, and common.compat version should also be bumped to 1.1.0
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #39530:
URL: https://github.com/apache/airflow/pull/39530#discussion_r1687170456
##########
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:
Added.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2236945057
Last one seems flaky?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #39530:
URL: https://github.com/apache/airflow/pull/39530#discussion_r1686556372
##########
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:
That is for any other integrations that did not already migrate to v2. It's only for static analysis but it felt safer to do it this way.
##########
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:
That is for any other providers that did not already migrate to v2. It's only for static analysis but it felt safer to do it this way.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #39530:
URL: https://github.com/apache/airflow/pull/39530#discussion_r1686582997
##########
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:
I followed the path how these packages are generated - it comes from `cross-providers-deps` that does not version limits into account.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2208496034
@eladkal I'll be working on moving part of the code to `common.compat` provider
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on code in PR #39530:
URL: https://github.com/apache/airflow/pull/39530#discussion_r1684284845
##########
airflow/providers/amazon/provider.yaml:
##########
@@ -89,6 +89,7 @@ versions:
dependencies:
- apache-airflow>=2.7.0
+ - apache-airflow-providers-common-compat
Review Comment:
FYI, you might bump into the same CI error as Maciej did: https://github.com/apache/airflow/pull/40819#discussion_r1684277036
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2236275910
almost :)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on code in PR #39530:
URL: https://github.com/apache/airflow/pull/39530#discussion_r1686591772
##########
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:
Got it now, makes sense. Feel free to resolve this one.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2123454942
I ran `breeze testing tests --upgrade-boto tests/providers/docker` in main branch with latest CI image - getting same error results.
Celery integration test seems flaky, no?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2236786821
Let's see -rebased to take latest deps into account :crossed_fingers:
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2145670785
I've added weird-looking:
```python
if TYPE_CHECKING:
# v2 imports
else:
try:
# v2 imports
except ImportError:
# v1 imports
```
This satisfies mypy and works with previous OL provider versions.
I'll be adding backwards-compatibility cross-provider checks in separate PR soon.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "mobuchowski (via GitHub)" <gi...@apache.org>.
mobuchowski commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2124976010
@JDarDagran we probably need to bump cross-provider version dependencies. If someone has older OL provider version, and bumps some of the other provider versions, the openlineage methods will fail.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2245140771
@potiuk looks like I'm on green path finally consistently 🟢 Would love to get this merged soon as new features arrive and I still need to rebase and apply new imports :)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2236756517
> > I think I need to drop tests checking what is imported with different availability of OL client. mocking imports breaks other tests when running in parallel.
>
> The OTEL "lowest dependencies" test is - quite surprisingly caused by packaging version - I have a PR for that #40865 - but it might have some side-effects so I am waiting for it to complete.
That's great to have it fixed. Still, I meant different way of tests breaking caused by mocking builtin import - which I removed eventually. Now you can see it's _almost_ green.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2172772750
Consensus thread started ...
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2182972360
PR to add `common.compat` here https://github.com/apache/airflow/pull/40374 @kacpermuda
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2236703263
I found it by bisecting downgraded packages BTW.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2166627546
yep, waiting for better times :)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2236701714
> I think I need to drop tests checking what is imported with different availability of OL client. mocking imports breaks other tests when running in parallel.
The OTEL "lowest dependencies" test is - quite surprisingly caused by packaging version - I have a PR for that https://github.com/apache/airflow/pull/40865 - but it might have some side-effects so I am waiting for it to complete.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #39530:
URL: https://github.com/apache/airflow/pull/39530#discussion_r1686583461
##########
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:
Added the comment.
##########
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:
Sure, will add 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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2156153913
Should we re-start discussion about common provider for that one? I have an idea..
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2156164939
Discussion at devlist https://lists.apache.org/thread/hjmrv1wc2rxhkvozpvccs4mhgwg5sw2f
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2125412172
As discussed with @mobuchowski -> we need to add back-compatibility `try/import` to past versions of the openlineage provider. We could even potentially add back-compatibility test suite for older version of of openlineage client < 1.13 to test it (symiliar to what we have in Pydantic/Botocore cases.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2165802148
My proposal is to wait for LAZY CONSENSUS on compat provider and implement it (See https://lists.apache.org/thread/hjmrv1wc2rxhkvozpvccs4mhgwg5sw2f)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2236427967
I think I need to drop tests checking what is imported with different availability of OL client. mocking imports breaks other tests when running in parallel.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "JDarDagran (via GitHub)" <gi...@apache.org>.
JDarDagran commented on code in PR #39530:
URL: https://github.com/apache/airflow/pull/39530#discussion_r1684423933
##########
airflow/providers/amazon/provider.yaml:
##########
@@ -89,6 +89,7 @@ versions:
dependencies:
- apache-airflow>=2.7.0
+ - apache-airflow-providers-common-compat
Review Comment:
I think I added it in all proper places. Also followed advice how to avoid CI error from Kacper's link.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] openlineage: migrate OpenLineage provider to V2 facets. [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39530:
URL: https://github.com/apache/airflow/pull/39530#issuecomment-2208552183
> @eladkal I'll be working on moving part of the code to common.compat provider
Cool :)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org