-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
Improve speed and stability of Python 3.12 tests in canary build #38194
Merged
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
Python 3.12 introduced a new (much faster) way of tracking and monitoring execution of python code by tools like coverage tracking using sysmon (PEP 669). This however also apparently heavily impacted performance of coverage tracking for Python 3.12 when PEP 669 is not used. The coverage library since 7.4.0 has an experimental support for PEP 669 that can be enabled with COVERAGE_CORE=sysmon env variable and a number of users confirmed it fixes the problem. We are using 7.4.4 coverage already so we should enable this mode to speed up our coverage tracking. That should also allow us to remove databricks from excluded providers. See databricks/databricks-sql-python#369 for databricks case and nedbat/coveragepy#1665 for coverage bug.
potiuk
added
the
canary
When set on PR running from apache repo - behave as canary run
label
Mar 15, 2024
jscheffl
approved these changes
Mar 15, 2024
🤞 |
hussein-awala
approved these changes
Mar 15, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting
potiuk
added a commit
to potiuk/airflow
that referenced
this pull request
Mar 16, 2024
We excluded Python 3.12 from Databricks provider, because it was failing our Python 3.12 tests intermittently (but often enough to make a difference). It turned out that this was caused by running the tests with coverage enabled and PEP 669 implementation in Python 3.12 impacting intermittently performance of tests run with coverage. However seems that experimenetal PEP 669 support implemented in coverage 7.4.0 is nicely handling the performance issues and after apache#38194 we shoudl be able to enable Python 3.12 for Databricks without impacting our tests. Related: databricks/databricks-sql-python#369
potiuk
added a commit
that referenced
this pull request
Mar 16, 2024
We excluded Python 3.12 from Databricks provider, because it was failing our Python 3.12 tests intermittently (but often enough to make a difference). It turned out that this was caused by running the tests with coverage enabled and PEP 669 implementation in Python 3.12 impacting intermittently performance of tests run with coverage. However seems that experimenetal PEP 669 support implemented in coverage 7.4.0 is nicely handling the performance issues and after #38194 we shoudl be able to enable Python 3.12 for Databricks without impacting our tests. Related: databricks/databricks-sql-python#369
potiuk
added a commit
to potiuk/airflow
that referenced
this pull request
Mar 16, 2024
The coverage fix in apache#38194 was wrongly applied to Helm tests, not to the unit tests 🤦. This PR sets it in the right place.
potiuk
added a commit
that referenced
this pull request
Mar 16, 2024
The coverage fix in #38194 was wrongly applied to Helm tests, not to the unit tests 🤦. This PR sets it in the right place.
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.
Python 3.12 introduced a new (much faster) way of tracking and monitoring execution of python code by tools like coverage tracking using sysmon (PEP 669). This however also apparently heavily impacted performance of coverage tracking for Python 3.12 when PEP 669 is not used. The coverage library since 7.4.0 has an experimental support for PEP 669 that can be enabled with COVERAGE_CORE=sysmon env variable and a number of users confirmed it fixes the problem.
We are using 7.4.4 coverage already so we should enable this mode to speed up our coverage tracking. That should also allow us to remove databricks from excluded providers.
See databricks/databricks-sql-python#369 for databricks case and nedbat/coveragepy#1665 for coverage bug.
^ 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.