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

fix: profiler reactor initialization #1058

Merged
merged 1 commit into from
Jul 5, 2024

Conversation

glevco
Copy link
Contributor

@glevco glevco commented Jun 10, 2024

Motivation

The get_cpu_profiler() function which is called at the module-level in multiple places indirectly installed the Twisted reactor by calling LoopingCall(). This meant that when a module with a profiler was imported before reactor initialization (in initialize_global_reactor), a ReactorAlreadyInstalledError could be thrown.

This PR prevents the profiler from indirectly installing a reactor before it has been initialized.

Acceptance Criteria

  • Change SimpleCPUProfiler so it does not create a LoopingCall in its __init__.
  • Remove some unused calls to get_cpu_profiler().

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged

@glevco glevco self-assigned this Jun 10, 2024
@glevco glevco marked this pull request as ready for review June 10, 2024 22:02
@glevco glevco requested review from jansegre and msbrogli as code owners June 10, 2024 22:02
Copy link

codecov bot commented Jun 10, 2024

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 84.83%. Comparing base (14289d5) to head (8a733dd).

Files Patch % Lines
hathor/profiler/cpu.py 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1058      +/-   ##
==========================================
- Coverage   84.92%   84.83%   -0.10%     
==========================================
  Files         300      300              
  Lines       22944    22936       -8     
  Branches     3471     3471              
==========================================
- Hits        19486    19458      -28     
- Misses       2769     2784      +15     
- Partials      689      694       +5     

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

@glevco glevco force-pushed the fix/profiler-reactor-initialization branch from ac3f0be to 8a733dd Compare July 5, 2024 17:54
@glevco glevco merged commit 255fd58 into master Jul 5, 2024
12 checks passed
@glevco glevco deleted the fix/profiler-reactor-initialization branch July 5, 2024 18:50
msbrogli pushed a commit that referenced this pull request Jul 5, 2024
This was referenced Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants