Skip to content

Conversation

@vatsrahul1001
Copy link
Contributor

@vatsrahul1001 vatsrahul1001 commented Apr 15, 2024

PR #37793 introduced KubernetesDeleteJobOperator in the latest release
8.1.0 of the cncf-kubernetes provider, which is also a dependency
for the latest release of the google provider 10.17.0.
This PR bumps the cncf-kubernetes provider dependency version
to a minimum of 8.1.0 for the google provider.

It appears that google provider 10.17.0 won't together with
cncf-kubernetes provider <8.1.0. So we would also need to
discuss what could be done for the released google 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.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added area:providers provider:google Google (including GCP) related issues labels Apr 15, 2024
@vatsrahul1001 vatsrahul1001 marked this pull request as draft April 15, 2024 13:44
@vatsrahul1001
Copy link
Contributor Author

vatsrahul1001 commented Apr 15, 2024

When google provider 10.17.0 is started with CNCF K8s provider less than 8.1.0
image

Error in plain text

Traceback (most recent call last):
  File "/usr/local/airflow/dags/example_kubernetes_engine.py", line 6, in <module>
    from airflow.providers.google.cloud.operators.kubernetes_engine import (
  File "/usr/local/lib/python3.11/site-packages/airflow/providers/google/cloud/operators/kubernetes_engine.py", line 38, in <module>
    from airflow.providers.cncf.kubernetes.operators.job import KubernetesDeleteJobOperator, KubernetesJobOperator
ImportError: cannot import name 'KubernetesDeleteJobOperator' from 'airflow.providers.cncf.kubernetes.operators.job' (/usr/local/lib/python3.11/site-packages/airflow/providers/cncf/kubernetes/operators/job.py)

@pankajkoti pankajkoti changed the title Bump CNCF dependency in Google provider Bump CNCF K8s provider dependency version in Google provider Apr 15, 2024
@pankajkoti pankajkoti changed the title Bump CNCF K8s provider dependency version in Google provider Bump CNCF K8S provider dependency version in Google provider Apr 15, 2024
@vatsrahul1001 vatsrahul1001 marked this pull request as ready for review April 15, 2024 15:04
pankajkoti
pankajkoti previously approved these changes Apr 15, 2024
@pankajkoti
Copy link
Member

It appears that google provider 10.17.0 won't together with
cncf-kubernetes provider <8.1.0. So we would also need to
discuss what could be done for the released google provider.

@eladkal looks like 10.17.0 of the Google provider won't work
with cncf-k8s provider <8.1.0. Do we need to take some care
for the released version of Google provider?

@pankajkoti pankajkoti requested a review from eladkal April 15, 2024 15:27
@potiuk
Copy link
Member

potiuk commented Apr 15, 2024

@eladkal looks like 10.17.0 of the Google provider won't work
with cncf-k8s provider <8.1.0. Do we need to take some care
for the released version of Google provider?

@pankajkoti with your question. Now when you are mentioning it - and since it is only one provider - probably a better approach is to try/catch the specific import error and raise AirflowOptionalProviderFeatureException with explanation that you need to use cncf.kubernetes >=8.1.0 for that one. This is likely better way to handle this than to add whole provider's dependency.

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

As explained in the comment since this is one Operator only - better way is to turn the import error into OptionalProviderFeature with explanation that 8.1.0 of cncf.kubernetes is needed for that

@eladkal
Copy link
Contributor

eladkal commented Apr 15, 2024

@eladkal looks like 10.17.0 of the Google provider won't work
with cncf-k8s provider <8.1.0. Do we need to take some care
for the released version of Google provider?

@pankajkoti with your question. Now when you are mentioning it - and since it is only one provider - probably a better approach is to try/catch the specific import error and raise AirflowOptionalProviderFeatureException with explanation that you need to use cncf.kubernetes >=8.1.0 for that one. This is likely better way to handle this than to add whole provider's dependency.

Yes I also wanted to raise this. We should use AirflowOptionalProviderFeatureException here

@pankajkoti pankajkoti dismissed their stale review April 15, 2024 16:29

Removing stale review based on latest comment from Jarek

@vatsrahul1001
Copy link
Contributor Author

closing in favour of #39036

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants