You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "kacpermuda (via GitHub)" <gi...@apache.org> on 2024/05/14 09:01:50 UTC
[PR] feat: Add custom provider runtime checks [airflow]
kacpermuda opened a new pull request, #39609:
URL: https://github.com/apache/airflow/pull/39609
<!--
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/
-->
As discussed [here](https://github.com/apache/airflow/pull/39513#discussion_r1598164584) at pr #39513, changes introduced by #39336 make all previous OpenLineage provider versions incompatible with Airflow >= 2.10.0 (due to changes in try_number behaviour change in core Airflow).
Using previous version of provider is still possible for Airflow >= 2.10.0, but will cause invalid behaviour (yet it will not cause any errors). We can't modify already released versions and provide a max supported Airflow version so we'd like to have a runtime check to ensure that a compatible version of OL provider is installed
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on code in PR #39609:
URL: https://github.com/apache/airflow/pull/39609#discussion_r1599667924
##########
airflow/settings.py:
##########
@@ -207,6 +209,35 @@ def configure_vars():
DONOT_MODIFY_HANDLERS = conf.getboolean("logging", "donot_modify_handlers", fallback=False)
+def _run_openlineage_runtime_check():
+ """
+ Ensure compatibility of OpenLineage provider package and Airflow version.
+
+ Airflow 2.10.0 introduced some core changes (#39336) that made versions <= 1.8.0 of OpenLineage
+ provider incompatible with future Airflow versions (>= 2.10.0).
+ """
+ ol_package = "apache-airflow-providers-openlineage"
+ try:
+ ol_version = metadata.version(ol_package)
+ except metadata.PackageNotFoundError:
+ return
+
+ if (
+ ol_version
+ and Version(ol_version) < Version("1.8.0")
+ and Version(airflow_version) >= Version("2.10.0")
Review Comment:
That's what i though, i figured i'd add it in case it gets cherry picked but yea, it may not make sense. Changing 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] feat: Add custom provider runtime checks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39609:
URL: https://github.com/apache/airflow/pull/39609#discussion_r1599722929
##########
airflow/settings.py:
##########
@@ -207,6 +209,35 @@ def configure_vars():
DONOT_MODIFY_HANDLERS = conf.getboolean("logging", "donot_modify_handlers", fallback=False)
+def _run_openlineage_runtime_check():
+ """
+ Ensure compatibility of OpenLineage provider package and Airflow version.
+
+ Airflow 2.10.0 introduced some core changes (#39336) that made versions <= 1.8.0 of OpenLineage
+ provider incompatible with future Airflow versions (>= 2.10.0).
+ """
+ ol_package = "apache-airflow-providers-openlineage"
+ try:
+ ol_version = metadata.version(ol_package)
Review Comment:
Yeah. Let's just use importlib. Will be simpler.
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39609:
URL: https://github.com/apache/airflow/pull/39609#issuecomment-2109892758
Also - we need to actually bump openlineage to 1.8.0 in provider.yaml to make the Provider tests pass (it failed now correctly detecting wrong version :D).
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39609:
URL: https://github.com/apache/airflow/pull/39609#issuecomment-2124767140
> constraints-main is updated automatically. sometimes it takes time for the job to be completed
Seems that they have not been updated for a while, there were few main failures to prevent it but likely after https://github.com/apache/airflow/pull/39747 we are likely to get them updated
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39609:
URL: https://github.com/apache/airflow/pull/39609#issuecomment-2109923939
Yes. It's OK. But @eladkal needs to know it has been done.
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #39609:
URL: https://github.com/apache/airflow/pull/39609#discussion_r1599870232
##########
airflow/providers/openlineage/CHANGELOG.rst:
##########
@@ -26,10 +26,10 @@
Changelog
---------
-main
+1.8.0
.....
-In Airflow 2.10.0, we fix the way try_number works, so that it no longer returns different values depending on task instance state. Importantly, after the task is done, it no longer shows current_try + 1. Thus in 1.7.2 we patch this provider to fix try_number references so they no longer adjust for the old, bad behavior.
+In Airflow 2.10.0, the way try_number works changed, so that it no longer returns different values depending on task instance state. Importantly, after the task is done, it no longer shows current_try + 1. Thus in 1.8.0 version of this provider we fix try_number references so they no longer adjust for the old, bad behavior. This change makes all previous versions of OpenLineage provider (<1.8.0) incompatible with Airflow >= 2.10.0.
Review Comment:
If I am user that wants to upgrade.
What is the action required from me?
This text explain what was done to the code base. It does not explain what I need to do on upgrade
##########
airflow/providers/openlineage/CHANGELOG.rst:
##########
@@ -26,10 +26,10 @@
Changelog
---------
-main
+1.8.0
.....
-In Airflow 2.10.0, we fix the way try_number works, so that it no longer returns different values depending on task instance state. Importantly, after the task is done, it no longer shows current_try + 1. Thus in 1.7.2 we patch this provider to fix try_number references so they no longer adjust for the old, bad behavior.
+In Airflow 2.10.0, the way try_number works changed, so that it no longer returns different values depending on task instance state. Importantly, after the task is done, it no longer shows current_try + 1. Thus in 1.8.0 version of this provider we fix try_number references so they no longer adjust for the old, bad behavior. This change makes all previous versions of OpenLineage provider (<1.8.0) incompatible with Airflow >= 2.10.0.
Review Comment:
If I am user that want to upgrade.
What is the action required from me?
This text explain what was done to the code base. It does not explain what I need to do on upgrade
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on code in PR #39609:
URL: https://github.com/apache/airflow/pull/39609#discussion_r1599718139
##########
airflow/settings.py:
##########
@@ -207,6 +209,35 @@ def configure_vars():
DONOT_MODIFY_HANDLERS = conf.getboolean("logging", "donot_modify_handlers", fallback=False)
+def _run_openlineage_runtime_check():
+ """
+ Ensure compatibility of OpenLineage provider package and Airflow version.
+
+ Airflow 2.10.0 introduced some core changes (#39336) that made versions <= 1.8.0 of OpenLineage
+ provider incompatible with future Airflow versions (>= 2.10.0).
+ """
+ ol_package = "apache-airflow-providers-openlineage"
+ try:
+ ol_version = metadata.version(ol_package)
Review Comment:
When using `source` installation maybe we should just emit a warning? That's usually a development scenario anyway, right? The one that matters the most here is users installing the package.
Also then the CI would probably pass, now with 2.10.0.dev0 the error is raise as current provider is 1.7.1 (the try_num adjustment has not been released yet, it's only in source)
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39609:
URL: https://github.com/apache/airflow/pull/39609#issuecomment-2109927806
BTW. Both workflows aare fine. It's generally OK to bump minor version of package if we know change is breakin. The `breeze release-management prepare-provider-documentation` will see it and rather than bumping it, it will just tell release manager "hey this package is going to be relased in new major version` .
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on code in PR #39609:
URL: https://github.com/apache/airflow/pull/39609#discussion_r1599688018
##########
airflow/settings.py:
##########
@@ -207,6 +209,35 @@ def configure_vars():
DONOT_MODIFY_HANDLERS = conf.getboolean("logging", "donot_modify_handlers", fallback=False)
+def _run_openlineage_runtime_check():
+ """
+ Ensure compatibility of OpenLineage provider package and Airflow version.
+
+ Airflow 2.10.0 introduced some core changes (#39336) that made versions <= 1.8.0 of OpenLineage
+ provider incompatible with future Airflow versions (>= 2.10.0).
+ """
+ ol_package = "apache-airflow-providers-openlineage"
+ try:
+ ol_version = metadata.version(ol_package)
Review Comment:
Done. That's the first time I'm using it, so not sure if it's correct.
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39609:
URL: https://github.com/apache/airflow/pull/39609#discussion_r1599972861
##########
airflow/providers/openlineage/CHANGELOG.rst:
##########
@@ -26,10 +26,10 @@
Changelog
---------
-main
+1.8.0
.....
-In Airflow 2.10.0, we fix the way try_number works, so that it no longer returns different values depending on task instance state. Importantly, after the task is done, it no longer shows current_try + 1. Thus in 1.7.2 we patch this provider to fix try_number references so they no longer adjust for the old, bad behavior.
+In Airflow 2.10.0, the way try_number works changed, so that it no longer returns different values depending on task instance state. Importantly, after the task is done, it no longer shows current_try + 1. Thus in 1.8.0 version of this provider we fix try_number references so they no longer adjust for the old, bad behavior. This change makes all previous versions of OpenLineage provider (<1.8.0) incompatible with Airflow >= 2.10.0.
Review Comment:
Simple "You need to use openlineage provider version 1.8.0 with Airflow 2.10.0+ if you want to integrate Open Lineage with Airflow". Yes. I agree that our changelog entries should be more empathetic towards the user. Probably good idea - in such case is to explain first what the user should pay attention to (version of the open lineage provider) and when (when they are using openlineage provider) and then why.
BTW. This however should be a neswfragment in arflow newsfragments/ not in provider. I presume (as discussed before) 1.8 version of the provider will add (in the next PR) a way to handle try_number when installed on previous version of Airlfow.
If that's the case, then provider would not even have to technically be breaking change (assuming full backwards compatibility with previous Airflow versions) but I would still leave 1.8.0 to demarkate this "big" change better.
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #39609:
URL: https://github.com/apache/airflow/pull/39609#issuecomment-2124728524
> I think we should also update apache-airflow-providers-openlineage==1.8.0 the constraint in [constraint-main](https://github.com/apache/airflow/blob/constraints-main/constraints-3.11.txt) for all Python versions.
constraints-main is updated automatically. sometimes it takes time for the job to be completed
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on code in PR #39609:
URL: https://github.com/apache/airflow/pull/39609#discussion_r1599788163
##########
airflow/settings.py:
##########
@@ -207,6 +209,35 @@ def configure_vars():
DONOT_MODIFY_HANDLERS = conf.getboolean("logging", "donot_modify_handlers", fallback=False)
+def _run_openlineage_runtime_check():
+ """
+ Ensure compatibility of OpenLineage provider package and Airflow version.
+
+ Airflow 2.10.0 introduced some core changes (#39336) that made versions <= 1.8.0 of OpenLineage
+ provider incompatible with future Airflow versions (>= 2.10.0).
+ """
+ ol_package = "apache-airflow-providers-openlineage"
+ try:
+ ol_version = metadata.version(ol_package)
Review Comment:
Got it, it makes sense to skip the check when installing from source. Reverted the code to the original one, just using importlib.
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on code in PR #39609:
URL: https://github.com/apache/airflow/pull/39609#discussion_r1599979475
##########
airflow/providers/openlineage/CHANGELOG.rst:
##########
@@ -26,10 +26,10 @@
Changelog
---------
-main
+1.8.0
.....
-In Airflow 2.10.0, we fix the way try_number works, so that it no longer returns different values depending on task instance state. Importantly, after the task is done, it no longer shows current_try + 1. Thus in 1.7.2 we patch this provider to fix try_number references so they no longer adjust for the old, bad behavior.
+In Airflow 2.10.0, the way try_number works changed, so that it no longer returns different values depending on task instance state. Importantly, after the task is done, it no longer shows current_try + 1. Thus in 1.8.0 version of this provider we fix try_number references so they no longer adjust for the old, bad behavior. This change makes all previous versions of OpenLineage provider (<1.8.0) incompatible with Airflow >= 2.10.0.
Review Comment:
I just modified the changelog to be more user friendly and simply point to the PR that introduced the change instead of summarizing it.
There is already a `39336.significant.rst` - i can add some information about the influence on OL provider there, if you think it's a good idea.
> I presume (as discussed before) 1.8 version of the provider will add (in the next PR) a way to handle try_number when installed on previous version of Airlfow.
It's already [there](https://github.com/apache/airflow/blob/6bcec908cd27d691e97156a29c7243db9089e0f8/airflow/providers/openlineage/plugins/listener.py#L49), there is a function that determines whether we should subtract one from try_number based on Airflow version
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39609:
URL: https://github.com/apache/airflow/pull/39609#issuecomment-2110453929
@eladkal ?
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39609:
URL: https://github.com/apache/airflow/pull/39609#discussion_r1599780939
##########
airflow/settings.py:
##########
@@ -207,6 +209,35 @@ def configure_vars():
DONOT_MODIFY_HANDLERS = conf.getboolean("logging", "donot_modify_handlers", fallback=False)
+def _run_openlineage_runtime_check():
+ """
+ Ensure compatibility of OpenLineage provider package and Airflow version.
+
+ Airflow 2.10.0 introduced some core changes (#39336) that made versions <= 1.8.0 of OpenLineage
+ provider incompatible with future Airflow versions (>= 2.10.0).
+ """
+ ol_package = "apache-airflow-providers-openlineage"
+ try:
+ ol_version = metadata.version(ol_package)
Review Comment:
It could be done this way, yes. But also there is no need to use ProvidersManager at all. In this case if provider is installed from sources, it's not installed as package at all so when providers are installed from sources this if will return immediately (same as openlineage provider not installed). Which is what we expect - because when we use airflow from sources, we know both airflow and provider are correctly latest versions.
```python
ol_provider = importlib.version("apache-airflow-providers-openlinege")
if not ol_provider:
return
```
##########
airflow/settings.py:
##########
@@ -207,6 +209,35 @@ def configure_vars():
DONOT_MODIFY_HANDLERS = conf.getboolean("logging", "donot_modify_handlers", fallback=False)
+def _run_openlineage_runtime_check():
+ """
+ Ensure compatibility of OpenLineage provider package and Airflow version.
+
+ Airflow 2.10.0 introduced some core changes (#39336) that made versions <= 1.8.0 of OpenLineage
+ provider incompatible with future Airflow versions (>= 2.10.0).
+ """
+ ol_package = "apache-airflow-providers-openlineage"
+ try:
+ ol_version = metadata.version(ol_package)
Review Comment:
So your original code was good.
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on PR #39609:
URL: https://github.com/apache/airflow/pull/39609#issuecomment-2109929735
Also, where are we with Airflow 2.10 on timeline? Do you think there will be time for next providers release before the 2.10. release? We need to make sure that, by accident, we don't get the `apache-airflow-providers-openlineage` < 1.8.0 in Airflow 2.10 constraints 😄
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39609:
URL: https://github.com/apache/airflow/pull/39609#discussion_r1599985346
##########
airflow/providers/openlineage/CHANGELOG.rst:
##########
@@ -26,10 +26,10 @@
Changelog
---------
-main
+1.8.0
.....
-In Airflow 2.10.0, we fix the way try_number works, so that it no longer returns different values depending on task instance state. Importantly, after the task is done, it no longer shows current_try + 1. Thus in 1.7.2 we patch this provider to fix try_number references so they no longer adjust for the old, bad behavior.
+In Airflow 2.10.0, the way try_number works changed, so that it no longer returns different values depending on task instance state. Importantly, after the task is done, it no longer shows current_try + 1. Thus in 1.8.0 version of this provider we fix try_number references so they no longer adjust for the old, bad behavior. This change makes all previous versions of OpenLineage provider (<1.8.0) incompatible with Airflow >= 2.10.0.
Review Comment:
> It's already [there](https://github.com/apache/airflow/blob/6bcec908cd27d691e97156a29c7243db9089e0f8/airflow/providers/openlineage/plugins/listener.py#L49), there is a function that determines whether we should subtract one from try_number based on Airflow version
Right :)
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39609:
URL: https://github.com/apache/airflow/pull/39609#discussion_r1599780939
##########
airflow/settings.py:
##########
@@ -207,6 +209,35 @@ def configure_vars():
DONOT_MODIFY_HANDLERS = conf.getboolean("logging", "donot_modify_handlers", fallback=False)
+def _run_openlineage_runtime_check():
+ """
+ Ensure compatibility of OpenLineage provider package and Airflow version.
+
+ Airflow 2.10.0 introduced some core changes (#39336) that made versions <= 1.8.0 of OpenLineage
+ provider incompatible with future Airflow versions (>= 2.10.0).
+ """
+ ol_package = "apache-airflow-providers-openlineage"
+ try:
+ ol_version = metadata.version(ol_package)
Review Comment:
It could be done this way, yes. But also there is no need to use ProvidersManager at all. In this case if provider is installed from sources, it's not installed as package at all so when providers are installed from sources this if will return immediately (same as openlineage provider not installed). Which is what we expect - because when we use airflow from sources, we know both airflow and provider are correctly latest versions.
```
ol_provider = importlib.verison("apache-airflow-providers-openlinege")
if not ol_provider:
return
```
##########
airflow/settings.py:
##########
@@ -207,6 +209,35 @@ def configure_vars():
DONOT_MODIFY_HANDLERS = conf.getboolean("logging", "donot_modify_handlers", fallback=False)
+def _run_openlineage_runtime_check():
+ """
+ Ensure compatibility of OpenLineage provider package and Airflow version.
+
+ Airflow 2.10.0 introduced some core changes (#39336) that made versions <= 1.8.0 of OpenLineage
+ provider incompatible with future Airflow versions (>= 2.10.0).
+ """
+ ol_package = "apache-airflow-providers-openlineage"
+ try:
+ ol_version = metadata.version(ol_package)
Review Comment:
It could be done this way, yes. But also there is no need to use ProvidersManager at all. In this case if provider is installed from sources, it's not installed as package at all so when providers are installed from sources this if will return immediately (same as openlineage provider not installed). Which is what we expect - because when we use airflow from sources, we know both airflow and provider are correctly latest versions.
```python
ol_provider = importlib.verison("apache-airflow-providers-openlinege")
if not ol_provider:
return
```
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #39609:
URL: https://github.com/apache/airflow/pull/39609#issuecomment-2109958471
> Also, where are we with Airflow 2.10 on timeline? Do you think there will be time for next providers release before the 2.10. release? We need to make sure that, by accident, we don't get the apache-airflow-providers-openlineage < 1.8.0 in Airflow 2.10 constraints 😄
No worries, that will take quite some time (months rather than weeks).
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on PR #39609:
URL: https://github.com/apache/airflow/pull/39609#issuecomment-2109909878
I was wondering how to do it aside the release process not to break anything, as doing it by hand seemed really wrong to me . I modified the changelog, init and provider.yaml, is that the correct approach ? I want to make sure that for the next release it will not get bumped again, and in result the 1.8.0 will not exist.
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39609:
URL: https://github.com/apache/airflow/pull/39609#discussion_r1599704089
##########
airflow/settings.py:
##########
@@ -207,6 +209,35 @@ def configure_vars():
DONOT_MODIFY_HANDLERS = conf.getboolean("logging", "donot_modify_handlers", fallback=False)
+def _run_openlineage_runtime_check():
+ """
+ Ensure compatibility of OpenLineage provider package and Airflow version.
+
+ Airflow 2.10.0 introduced some core changes (#39336) that made versions <= 1.8.0 of OpenLineage
+ provider incompatible with future Airflow versions (>= 2.10.0).
+ """
+ ol_package = "apache-airflow-providers-openlineage"
+ try:
+ ol_version = metadata.version(ol_package)
Review Comment:
**actually** I just realized that it will work just fine as well - because .... provider will not be installed in this case and we just return. :facepalm:
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "utkarsharma2 (via GitHub)" <gi...@apache.org>.
utkarsharma2 commented on PR #39609:
URL: https://github.com/apache/airflow/pull/39609#issuecomment-2124779076
@potiuk @eladkal Thanks :)
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "utkarsharma2 (via GitHub)" <gi...@apache.org>.
utkarsharma2 commented on PR #39609:
URL: https://github.com/apache/airflow/pull/39609#issuecomment-2124714336
I think we should also update `apache-airflow-providers-openlineage==1.8.0` the constraint in [constraint-main](https://github.com/apache/airflow/blob/constraints-main/constraints-3.11.txt) for all Python versions.
cc: @eladkal
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on PR #39609:
URL: https://github.com/apache/airflow/pull/39609#issuecomment-2110348081
@potiuk @eladkal It's all green, so if there are no more things to do here i think it can be merged.
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "kacpermuda (via GitHub)" <gi...@apache.org>.
kacpermuda commented on code in PR #39609:
URL: https://github.com/apache/airflow/pull/39609#discussion_r1599739725
##########
airflow/settings.py:
##########
@@ -207,6 +209,35 @@ def configure_vars():
DONOT_MODIFY_HANDLERS = conf.getboolean("logging", "donot_modify_handlers", fallback=False)
+def _run_openlineage_runtime_check():
+ """
+ Ensure compatibility of OpenLineage provider package and Airflow version.
+
+ Airflow 2.10.0 introduced some core changes (#39336) that made versions <= 1.8.0 of OpenLineage
+ provider incompatible with future Airflow versions (>= 2.10.0).
+ """
+ ol_package = "apache-airflow-providers-openlineage"
+ try:
+ ol_version = metadata.version(ol_package)
Review Comment:
Not sure i understand it tho. I mean, i used the importlib here now, but isn't that what the `ProvidersManager` uses underneath when looking for packages installed (airflow.utils.entry_points._get_grouped_entry_points)? I thought that when `ProvidersManager().providers.get(ol_package).is_source` is False then `ProvidersManager().providers.get(ol_package).version` , it's the same as `importlib.metadata.version(ol_package)` - it simply detects whether or not package has been installed.
So i could just write this, and it should work just fine, right?
```
ol_package = "apache-airflow-providers-openlineage"
ol_provider = ProvidersManager().providers.get(ol_package)
if not ol_provider:
return
err_msg = (
f"You have installed `{ol_package}` == `{ol_provider.version}` that is not compatible with "
f"`apache-airflow` == `{airflow_version}`. "
f"For `apache-airflow` >= `2.10.0` you must use `{ol_package}` >= `1.8.0`."
)
if ol_provider.version and Version(ol_provider.version) < Version("1.8.0"):
if ol_provider.is_source:
log.warning(err_msg)
else:
raise RuntimeError(err_msg)
```
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39609:
URL: https://github.com/apache/airflow/pull/39609#discussion_r1599668717
##########
airflow/settings.py:
##########
@@ -207,6 +209,35 @@ def configure_vars():
DONOT_MODIFY_HANDLERS = conf.getboolean("logging", "donot_modify_handlers", fallback=False)
+def _run_openlineage_runtime_check():
+ """
+ Ensure compatibility of OpenLineage provider package and Airflow version.
+
+ Airflow 2.10.0 introduced some core changes (#39336) that made versions <= 1.8.0 of OpenLineage
+ provider incompatible with future Airflow versions (>= 2.10.0).
+ """
+ ol_package = "apache-airflow-providers-openlineage"
+ try:
+ ol_version = metadata.version(ol_package)
Review Comment:
I think we should have an extra check for breeze. In breeze, there is no provider installed - providers are read directly from sources, so we should rather check version of OpenLineage Provider via ProvidersManager, not directly via importlib
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #39609:
URL: https://github.com/apache/airflow/pull/39609#discussion_r1599666259
##########
airflow/settings.py:
##########
@@ -207,6 +209,35 @@ def configure_vars():
DONOT_MODIFY_HANDLERS = conf.getboolean("logging", "donot_modify_handlers", fallback=False)
+def _run_openlineage_runtime_check():
+ """
+ Ensure compatibility of OpenLineage provider package and Airflow version.
+
+ Airflow 2.10.0 introduced some core changes (#39336) that made versions <= 1.8.0 of OpenLineage
+ provider incompatible with future Airflow versions (>= 2.10.0).
+ """
+ ol_package = "apache-airflow-providers-openlineage"
+ try:
+ ol_version = metadata.version(ol_package)
+ except metadata.PackageNotFoundError:
+ return
+
+ if (
+ ol_version
+ and Version(ol_version) < Version("1.8.0")
+ and Version(airflow_version) >= Version("2.10.0")
Review Comment:
We don't need to check for airflow_verion. By definition it's >= 2.10.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] feat: Add custom provider runtime checks [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #39609:
URL: https://github.com/apache/airflow/pull/39609
--
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] feat: Add custom provider runtime checks [airflow]
Posted by "utkarsharma2 (via GitHub)" <gi...@apache.org>.
utkarsharma2 commented on PR #39609:
URL: https://github.com/apache/airflow/pull/39609#issuecomment-2124710636
I think we should also update the constraint in [constraint-main](https://github.com/apache/airflow/blob/constraints-main/constraints-3.11.txt) for all Python versions.
--
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