Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

openlineage: remove openlineage.common dependencies in Google and Snowflake providers. #39614

Merged

Conversation

JDarDagran
Copy link
Contributor

This PR removes usage of openlineage.common dependencies in Google (BigQuery) and Snowflake providers. In both cases it externally contains logic that should be moved directly into providers.

The only dependency from openlineage.common should be related to OpenLineage SQLParser libraries.

@boring-cyborg boring-cyborg bot added area:providers provider:google Google (including GCP) related issues provider:openlineage AIP-53 provider:snowflake Issues related to Snowflake provider labels May 14, 2024
@JDarDagran JDarDagran force-pushed the openlineage/remove-common-package-usage branch 2 times, most recently from 464aec3 to c0b5ae7 Compare May 14, 2024 10:57
@JDarDagran JDarDagran force-pushed the openlineage/remove-common-package-usage branch 2 times, most recently from 9f27a05 to 50b3cd9 Compare May 14, 2024 11:30
Copy link
Contributor

@kacpermuda kacpermuda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I love the changes - the code is much more readable now. Leaving some suggestions.

airflow/providers/google/cloud/utils/openlineage.py Outdated Show resolved Hide resolved
airflow/providers/google/cloud/utils/openlineage.py Outdated Show resolved Hide resolved
airflow/providers/google/cloud/utils/openlineage.py Outdated Show resolved Hide resolved
airflow/providers/google/cloud/utils/openlineage.py Outdated Show resolved Hide resolved
airflow/providers/google/cloud/utils/openlineage.py Outdated Show resolved Hide resolved
airflow/providers/google/cloud/utils/openlineage.py Outdated Show resolved Hide resolved
airflow/providers/google/cloud/utils/openlineage.py Outdated Show resolved Hide resolved
airflow/providers/google/cloud/utils/openlineage.py Outdated Show resolved Hide resolved
airflow/providers/google/cloud/utils/openlineage.py Outdated Show resolved Hide resolved
airflow/providers/google/cloud/utils/openlineage.py Outdated Show resolved Hide resolved
@JDarDagran JDarDagran force-pushed the openlineage/remove-common-package-usage branch 3 times, most recently from 659d4bf to 8e8aca0 Compare May 15, 2024 19:08
@mobuchowski
Copy link
Contributor

@JDarDagran can we go with this but without removing old facets? We can notify they are deprecated and will be removed in future release. However we need better strategy to do that, and let's not block this PR on that problem.

@JDarDagran JDarDagran force-pushed the openlineage/remove-common-package-usage branch from 8e8aca0 to d86cc04 Compare May 16, 2024 22:35
Copy link
Contributor

@kacpermuda kacpermuda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All perfect now 🥇

@JDarDagran JDarDagran force-pushed the openlineage/remove-common-package-usage branch 4 times, most recently from da03d86 to 543aa8c Compare May 18, 2024 21:46
…iders.

Signed-off-by: Jakub Dardzinski <kuba0221@gmail.com>
@JDarDagran JDarDagran force-pushed the openlineage/remove-common-package-usage branch from 543aa8c to 3cd280c Compare May 19, 2024 17:42
@mobuchowski mobuchowski merged commit 4ee46b9 into apache:main May 20, 2024
41 checks passed
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Jul 26, 2024
…iders. (apache#39614)

Signed-off-by: Jakub Dardzinski <kuba0221@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:google Google (including GCP) related issues provider:openlineage AIP-53 provider:snowflake Issues related to Snowflake provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants