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

Improve speed and stability of Python 3.12 tests in canary build #38194

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Mar 15, 2024

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.

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 potiuk added the canary When set on PR running from apache repo - behave as canary run label Mar 15, 2024
@potiuk
Copy link
Member Author

potiuk commented Mar 15, 2024

🤞

Copy link
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

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

Interesting

@potiuk potiuk merged commit dffc9e3 into main Mar 15, 2024
95 of 96 checks passed
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
@eladkal eladkal deleted the improve-speed-and-stability-of-3.12-tests branch March 16, 2024 10:41
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 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.
utkarsharma2 pushed a commit to astronomer/airflow that referenced this pull request Apr 22, 2024
…che#38194)

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.
utkarsharma2 pushed a commit to astronomer/airflow that referenced this pull request Apr 22, 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
utkarsharma2 pushed a commit to astronomer/airflow that referenced this pull request Apr 22, 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dev-tools canary When set on PR running from apache repo - behave as canary run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants