-
Notifications
You must be signed in to change notification settings - Fork 16.4k
feat: Add helper for OpenLineage version check #47897
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
Conversation
6a3e6dd to
62cdd3e
Compare
| Raises: | ||
| ValueError: If neither `provider_min_version` nor `client_min_version` is provided. | ||
| TypeError: If the decorator is used without parentheses (e.g., `@require_openlineage_version`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes openlineage behave differently from other providers.
We always raise AirflowOptionalProviderFeatureException when there is problem with version.
Even in openlineage itself
airflow/providers/dbt/cloud/src/airflow/providers/dbt/cloud/utils/openlineage.py
Lines 76 to 82 in b2c646a
| try: | |
| from airflow.providers.openlineage.conf import namespace | |
| except ModuleNotFoundError as e: | |
| from airflow.exceptions import AirflowOptionalProviderFeatureException | |
| msg = "Please install `apache-airflow-providers-openlineage>=1.7.0`" | |
| raise AirflowOptionalProviderFeatureException(e, msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i wonder if we can change require_openlineage_version to require_provider_version(provider=...)
and create a more generic interface for all providers that implement the function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So these two errors ValueError and TypeError are related to how this decorator is used. If the OL version if incompatible, we simply log and return None. Do you think we should raise this AirflowOptionalProviderFeatureException in such cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any Exceptions raised during OL's extraction are caught by OL anyway, so the result would be similar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is a provider version incompatibility we should raise AirflowOptionalProviderFeatureException
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed it and added generic checker in this PR: #47909. Could you check if that's satisfying solution?
Some providers, such as Snowflake and DBT Cloud, do not require an OpenLineage provider but may offer optional features that depend on it. These features are generally available starting from a specific version of the OpenLineage provider or client. This decorator helps ensure compatibility, preventing import errors and providing clear logs about version requirements.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.