-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
bolkedebruin
merged 1 commit into
apache:main
from
potiuk:use-released-airflow-for-k8s-tests-venv
Oct 21, 2023
Merged
Use airflow released in PyPI for k8s test environment #35099
bolkedebruin
merged 1 commit into
apache:main
from
potiuk:use-released-airflow-for-k8s-tests-venv
Oct 21, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
approved these changes
Oct 21, 2023
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
added
the
changelog:skip
Changes that should be skipped from the changelog (CI, tests, etc..)
label
Oct 29, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.