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

Use airflow released in PyPI for k8s test environment #35099

Merged

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Oct 21, 2023

So far we've been using ".[cncf.kubernetes]" as requirement for creating kubernetes test virtualenve (in k8s_requirements.txt).

This becomes problematic when we add a new pre-installed provider that is not yet released in PyPI - because it cannot be found as we found out in #82689.

Changing it to "apache-airflow[cncf.kubernetes]" makes it install airflow + k8s requirements from PyPI instead.


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

So far we've been using ".[cncf.kubernetes]" as requirement for
creating kubernetes test virtualenve (in k8s_requirements.txt).

This becomes problematic when we add a new pre-installed provider
that is not yet released in PyPI - because it cannot be found
as we found out in #82689.

Changing it to "apache-airflow[cncf.kubernetes]" makes it install
airflow + k8s requirements from PyPI instead.
@bolkedebruin bolkedebruin merged commit dc2e852 into apache:main Oct 21, 2023
4 checks passed
potiuk added a commit to potiuk/airflow that referenced this pull request Oct 26, 2023
The apache#35099 switched installation of the local venv for Airflow to
released packages, in order to accomodate for case from apache#34729 where
we locally install a new provider that is preinstalled but has not
yet been released.

This however has the side-effect - some k8s tests are using K8S Pod
operator to run the tests and if this operator has been changed, the
local - modified - version of it is not going to be used for tests.

Another side effect is that in case of a new installation, when the
constraint installation does not work (for example constraints in
main are conflicting with released airflow), `pip` starts to
backtrack - and for conflicting requirements it produces strange
results (for example it attempts to install airflow 1.10 and fails
due to "slugify" dependency.

The previous version of it had another problem - once installed,
it had not changed, so in case someone used breeze to run k8s tests
locally and iterated over changes in K8SPod Operator, only the
version from the moment the k8s environment was installed was used.

Both cases are now handled better:

* INSTALL_PROVIDERS_FROM_SOURCES is used as env variable to make
  sure new providers (including preinstalled providers) are found
  in Airfow sources, not in PyPI
* The "." is used back in order to install Airflow but also the
  -e (editable) installation is used for it - so that any changes
  to local version of k8s are used. We are not using constraints
  for installation.
* Dry run and verbose mode of installing the venv are improved
  a bit - they show better what's going on (and dry_run does
  not interact with the installation in unexpected ways - deleting
  the installed venv without recreating it).

We already handled a change that k8s test environment has been
reinstalled after the requirements file changed and caching in CI
includes the hash of the requirements file as content - so we do not
need to handle reinstallation of the venv or caching in CI. The
venv should be appropriately reinstalled as needed.
potiuk added a commit that referenced this pull request Oct 26, 2023
…35191)

The #35099 switched installation of the local venv for Airflow to
released packages, in order to accomodate for case from #34729 where
we locally install a new provider that is preinstalled but has not
yet been released.

This however has the side-effect - some k8s tests are using K8S Pod
operator to run the tests and if this operator has been changed, the
local - modified - version of it is not going to be used for tests.

Another side effect is that in case of a new installation, when the
constraint installation does not work (for example constraints in
main are conflicting with released airflow), `pip` starts to
backtrack - and for conflicting requirements it produces strange
results (for example it attempts to install airflow 1.10 and fails
due to "slugify" dependency.

The previous version of it had another problem - once installed,
it had not changed, so in case someone used breeze to run k8s tests
locally and iterated over changes in K8SPod Operator, only the
version from the moment the k8s environment was installed was used.

Both cases are now handled better:

* INSTALL_PROVIDERS_FROM_SOURCES is used as env variable to make
  sure new providers (including preinstalled providers) are found
  in Airfow sources, not in PyPI
* The "." is used back in order to install Airflow but also the
  -e (editable) installation is used for it - so that any changes
  to local version of k8s are used. We are not using constraints
  for installation.
* Dry run and verbose mode of installing the venv are improved
  a bit - they show better what's going on (and dry_run does
  not interact with the installation in unexpected ways - deleting
  the installed venv without recreating it).

We already handled a change that k8s test environment has been
reinstalled after the requirements file changed and caching in CI
includes the hash of the requirements file as content - so we do not
need to handle reinstallation of the venv or caching in CI. The
venv should be appropriately reinstalled as needed.
potiuk added a commit that referenced this pull request Oct 29, 2023
So far we've been using ".[cncf.kubernetes]" as requirement for
creating kubernetes test virtualenve (in k8s_requirements.txt).

This becomes problematic when we add a new pre-installed provider
that is not yet released in PyPI - because it cannot be found
as we found out in #82689.

Changing it to "apache-airflow[cncf.kubernetes]" makes it install
airflow + k8s requirements from PyPI instead.

(cherry picked from commit dc2e852)
potiuk added a commit that referenced this pull request Oct 29, 2023
…35191)

The #35099 switched installation of the local venv for Airflow to
released packages, in order to accomodate for case from #34729 where
we locally install a new provider that is preinstalled but has not
yet been released.

This however has the side-effect - some k8s tests are using K8S Pod
operator to run the tests and if this operator has been changed, the
local - modified - version of it is not going to be used for tests.

Another side effect is that in case of a new installation, when the
constraint installation does not work (for example constraints in
main are conflicting with released airflow), `pip` starts to
backtrack - and for conflicting requirements it produces strange
results (for example it attempts to install airflow 1.10 and fails
due to "slugify" dependency.

The previous version of it had another problem - once installed,
it had not changed, so in case someone used breeze to run k8s tests
locally and iterated over changes in K8SPod Operator, only the
version from the moment the k8s environment was installed was used.

Both cases are now handled better:

* INSTALL_PROVIDERS_FROM_SOURCES is used as env variable to make
  sure new providers (including preinstalled providers) are found
  in Airfow sources, not in PyPI
* The "." is used back in order to install Airflow but also the
  -e (editable) installation is used for it - so that any changes
  to local version of k8s are used. We are not using constraints
  for installation.
* Dry run and verbose mode of installing the venv are improved
  a bit - they show better what's going on (and dry_run does
  not interact with the installation in unexpected ways - deleting
  the installed venv without recreating it).

We already handled a change that k8s test environment has been
reinstalled after the requirements file changed and caching in CI
includes the hash of the requirements file as content - so we do not
need to handle reinstallation of the venv or caching in CI. The
venv should be appropriately reinstalled as needed.

(cherry picked from commit ba28334)
@potiuk potiuk added this to the Airflow 2.7.3 milestone Oct 29, 2023
@potiuk potiuk added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Oct 29, 2023
@potiuk potiuk deleted the use-released-airflow-for-k8s-tests-venv branch November 17, 2023 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dev-tools changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants