-
Notifications
You must be signed in to change notification settings - Fork 16.3k
fix: add explicit requirement for OpenLineage provider>=2.0.0 on OL function #47999
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
fix: add explicit requirement for OpenLineage provider>=2.0.0 on OL function #47999
Conversation
531adb9 to
8595198
Compare
|
Nice. But I think we should make an explicit bump as a separate PR in I think #47909 was a bit wrong in the sense that it did not bump common.compat version to 1.6.0 - such bump should happen there, because that's where the functionality was added, not here where the functionality is "used". We can rectify by extracting We have no good mechanisms for full automation here (yet - maybe we should introduce it at some point of time), but for now I think it could be relatively easy thing to follow a process similar to this:
WDYT. @kacpermuda @mobuchowski @eladkal ? Maybe you have some other ideas here and maybe we should write-down a small description on how we approach common.compat changes? |
|
@potiuk your suggestions make sense to me.
I think for now we need to rely on contributors (and possibly reviewers) checking it manually... maybe this should live in |
8595198 to
587881f
Compare
|
@potiuk The general idea sounds good to me, I'm not sure I follow on how exactly you'd expect it to be done, but I'm eager to make it work. I think this time Elad already bumped the version and prepared the rc1, so I have no need to bum,p the common.compat anymore, but for the next feature I'll make sure to follow this approach. If this makes sense, let me know if we should merge this one then? |
potiuk
left a comment
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.
Sure!
generate_openlineage_events_from_dbt_cloud_runassumed the OL provider has some features that were introduced in 2.0.0 version. Added an explicit requirement on the function, so that it throwsAirflowOptionalProviderFeatureExceptionif the OL provider version is not enough.As the function checking the provider version is in common.compat, I needed to add compat provider as requirement for DBT cloud provider.
^ 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.