jazzband / django-silk

Silky smooth profiling for Django
MIT License
4.42k stars 333 forks source link

Always disable cProfile as part of cleanup #699

Closed albertyw closed 8 months ago

albertyw commented 8 months ago

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.

codecov[bot] commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

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

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.