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

Bump minimum Airflow version in providers to Airflow 2.6.0 #36017

Merged
merged 5 commits into from
Dec 7, 2023

Conversation

eladkal
Copy link
Contributor

@eladkal eladkal commented Dec 1, 2023

Note: suspended providers are not included in bump min airflow version (daskexecutor provider)

@eladkal eladkal requested a review from kaxil as a code owner December 1, 2023 19:54
@potiuk
Copy link
Member

potiuk commented Dec 1, 2023

Interesting error with providers. It looks like (Can't remember that) Airflow 2.6.0 had "apache-airflow-providers-hive" dependency instead of "apache-airflow-providers-apache-hive". But it seems that 2.6.1 has it fixed already - so just setting

--use-airflow-version 2.6.1

should fix the issue.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@potiuk
Copy link
Member

potiuk commented Dec 2, 2023

Ok. Two fixes are needed:

  1. rm -rf dist/apache_airflow_providers_cohere*.whl - added here:
    rm -rf dist/apache_airflow_providers_common_io*.whl

With the comment that is should be removed when we bump to 2.7.1

  1. add min_airflow_version = 2.7.1 for cohere provider - seems that importlib limitation in airflow (<5) is incompatible with cohere's (>=6) - and it was only removed in 2.7.1 (Fix dependencies for celery and opentelemetry for Python 3.8 #33579)

So looks ike Cohere provider is only actually installable for Airflow 2.7.1+ - we should add in their provider.yaml

@eladkal
Copy link
Contributor Author

eladkal commented Dec 2, 2023

add min_airflow_version = 2.7.1 for cohere provider - seems that importlib limitation in airflow (<5) is incompatible with cohere's (>=6) - and it was only removed in 2.7.1 (#3357)

I think the PR you linked is wrong?
Why do you claim that cohere provider needs Airflow>=2.7.1?
I don't see anything binding that provider to newer core versions (unlike common.fs)

@eladkal
Copy link
Contributor Author

eladkal commented Dec 2, 2023

The cohere provider can't work with Airflow 2.6 unless using Python 3.9 due to version of importlib

OK thanks to @potiuk we may have workaround for this

8ad4e9b

@potiuk
Copy link
Member

potiuk commented Dec 3, 2023

Seems like we are havin issue with just released 0.23.1 of Pytest-asyncio, which was supposed to fix bugs in (also just released but 10 hours earlier). I opened an issue pytest-dev/pytest-asyncio#703 to ask what's going on.

@potiuk
Copy link
Member

potiuk commented Dec 3, 2023

And PR here to temporarily limit asyncio until we know whats going on #36037

@eladkal
Copy link
Contributor Author

eladkal commented Dec 6, 2023

rebased after #36061
lets see if CI passes

@potiuk
Copy link
Member

potiuk commented Dec 6, 2023

rebased after #36061 lets see if CI passes

Close enough - you need to also remove 2.5.0 from the list :

https://github.com/apache/airflow/blob/main/dev/README_RELEASE_PROVIDER_PACKAGES.md#bump-min-airflow-version-for-providers

Update BASE_PROVIDERS_COMPATIBILITY_CHECKS in src/airflow_breeze/global_constants.py to remove the versions of Airflow that are not applicable any more.

@eladkal
Copy link
Contributor Author

eladkal commented Dec 6, 2023

Close enough - you need to also remove 2.5.0 from the list

Thanks! I new i forgot something

@potiuk
Copy link
Member

potiuk commented Dec 6, 2023

Looks good :)

@eladkal eladkal merged commit d0918d7 into apache:main Dec 7, 2023
74 checks passed
@eladkal eladkal deleted the bump branch December 7, 2023 03:25
@ephraimbuddy
Copy link
Contributor

This should be in 2.8 right @eladkal ?

@eladkal
Copy link
Contributor Author

eladkal commented Dec 7, 2023

This should be in 2.8 right @eladkal ?

If the breeze changes are relevant yes.

@eladkal eladkal added this to the Airflow 2.8.0 milestone Dec 7, 2023
@eladkal eladkal added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Dec 7, 2023
@ephraimbuddy
Copy link
Contributor

This should be in 2.8 right @eladkal ?

If the breeze changes are relevant yes.

I think it popped up because of the generated/ folder. I will try cherry-picking it

@potiuk
Copy link
Member

potiuk commented Dec 7, 2023

I think it's not strictly necessary for 2.8 - I will cherry-pick it eventually anyway (if @ephraimbuddy will not do it ) - but for 2.8.0 (i will eventually make it completely uneccessary when we move to more modern tooling for building airflow package ~ couple of months or so.

potiuk pushed a commit that referenced this pull request Dec 15, 2023
* Bump minimum Airflow version in providers to Airflow 2.6.0

* Fix breeze unit tests

* Handle cohere provider

* enhance CI matrix for provider-airflow-compatibility-check

* remove 2.5.0  from BASE_PROVIDERS_COMPATIBILITY_CHECKS

(cherry picked from commit d0918d7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants