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

Always disable cProfile as part of cleanup #699

Merged
merged 2 commits into from
Jan 19, 2024

Conversation

albertyw
Copy link
Member

As a followup to #692 , this diff cleans up two places where cProfile is enabled but not disabled.

In collector.py, the DataCollector()._configure() logic is changed so that the profiler is explicitly stopped before the profiler is set to None. If the profile is set to None, future profiles are blocked even though the old profiler is not accessible and pending garbage collection.

In test_execute_sql.py, the test cases are enabling profiling through DataCollector but the test cases are not explicitly disabling profiling when finished. This causes other test cases to fail when they attempt to profile. (I'm not sure why these test cases explicitly require disabling profiling even though many other test cases also initialize DataCollector and enable profiling)

#692 should have caught this but it turns out that tox.ini is configured to ignore djmain and by extension due to the test version matrix, most python 3.12 failures.

This was referenced Jan 16, 2024
Copy link

codecov bot commented Jan 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7bc0f26) 86.75% compared to head (1832b42) 86.76%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #699   +/-   ##
=======================================
  Coverage   86.75%   86.76%           
=======================================
  Files          52       52           
  Lines        2114     2115    +1     
=======================================
+ Hits         1834     1835    +1     
  Misses        280      280           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@albertyw albertyw merged commit 3a8bebc into jazzband:master Jan 19, 2024
19 checks passed
@albertyw albertyw deleted the profile-cleanup branch January 19, 2024 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant