Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Oct 31, 2024

Standard provider should have virtualenv as dependency set as it has PythonVirtualenvOperator that needs it. Also it should have min airflow version 2.8.0 same as all other providers.


^ 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.

Standard provider should have virtualenv as dependency set as it
has PythonVirtualenvOperator that needs it. Also it should have
min airflow version 2.8.0 same as all other providers.
@potiuk potiuk added the full tests needed We need to run full set of tests for this PR to merge label Oct 31, 2024
@gopidesupavan
Copy link
Member

gopidesupavan commented Oct 31, 2024

I believe changes required here also BASE_PROVIDERS_COMPATIBILITY_CHECKS

https://github.com/apache/airflow/blob/main/dev/breeze/src/airflow_breeze/global_constants.py#L563 ?

remove-providers: standard from 2.8.4 and 2.9.3

@potiuk
Copy link
Member Author

potiuk commented Oct 31, 2024

I believe changes required here also BASE_PROVIDERS_COMPATIBILITY_CHECKS

https://github.com/apache/airflow/blob/main/dev/breeze/src/airflow_breeze/global_constants.py#L563 ?

remove-providers: standard from 2.8.4 and 2.9.3

GREAT POINT

@potiuk
Copy link
Member Author

potiuk commented Oct 31, 2024

I will merge it first to fix main though, and then wil make a follow up PR @gopidesupavan

@potiuk potiuk merged commit 0c9e159 into apache:main Oct 31, 2024
@potiuk potiuk deleted the fix-dependencies-of-standard-provider branch October 31, 2024 18:21
potiuk added a commit to potiuk/airflow that referenced this pull request Oct 31, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
@potiuk
Copy link
Member Author

potiuk commented Oct 31, 2024

Follow up PR in #43556

- apache-airflow>=2.10.0
- apache-airflow>=2.8.0
- apache-airflow-providers-common-sql>=1.18.0
- virtualenv>=20.26.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Mhm, I am thinking about this as dependency... so in new Airflow Standard provider will be a standard install dependency, this has a standard dependency to virtualenv...
So far ths was always optional, even the examples excluded parts if virtualenv was not available as dependency. Do we really want to make this a pre-requisite?

Personally I think it is not harming but we change the default dependencies here. (Compared to 2.x line where no dependecy was modelled)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. Thought about it too. No harm I think. It's been always installed anyway in most cases. And the problem is that if we add it as provider's optional dependency - then almost never will actually install it, while PythonVirtualEnv operator is one that is pretty used.

So I think we should eventually remove the virtualenv extra from airflow and have it always installed.

Another option is - we migh change PythonVirtualenv to use the built-in venv.

Which actually might be a very good idea. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

COmpared to the authors of virtualenv I don't see a real benefir over the included venv. Might be a clever small change.

Otherwise - much cooler would be if UV is supported. We had some discussions in slack... not that pre-commit gets uv support on main-line bewfore we have Airflow 3 out :-D

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd be for both - venv (only) and uv (optional) support in the new PythonVirtualenv in provider.

Copy link
Member Author

Choose a reason for hiding this comment

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

First part fixed in #43568

potiuk added a commit to potiuk/airflow that referenced this pull request Oct 31, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Oct 31, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 1, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 1, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit that referenced this pull request Nov 1, 2024
The standard provider has now min version of Airflow = 2.8
since #43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 3, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 3, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 3, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 3, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 3, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 3, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 3, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 3, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 3, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 4, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 4, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 4, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 5, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 6, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 7, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 8, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 8, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 8, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 8, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 9, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit to potiuk/airflow that referenced this pull request Nov 9, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
potiuk added a commit that referenced this pull request Nov 9, 2024
The standard provider has now min version of Airflow = 2.8
since #43553, but we have not tested it for Airflow 2.8 and 2.9.
ellisms pushed a commit to ellisms/airflow that referenced this pull request Nov 13, 2024
Standard provider should have virtualenv as dependency set as it
has PythonVirtualenvOperator that needs it. Also it should have
min airflow version 2.8.0 same as all other providers.
ellisms pushed a commit to ellisms/airflow that referenced this pull request Nov 13, 2024
The standard provider has now min version of Airflow = 2.8
since apache#43553, but we have not tested it for Airflow 2.8 and 2.9.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers full tests needed We need to run full set of tests for this PR to merge provider:standard

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants