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

Use Concurrent.usable_processor_count when it is available #2339

Merged
merged 2 commits into from
Jul 9, 2024

Conversation

st0012
Copy link
Collaborator

@st0012 st0012 commented Jul 6, 2024

It's a new API introduced in concurrent-ruby 1.3.1, which works better in the container environment.

ruby-concurrency/concurrent-ruby#1038

Since there are gems like sorbet-runtime that still use older versions of concurrent-ruby, we can't directly bump concurrent-ruby's requirement, but need to check if the method is available before calling it.

Big thanks to @trevorturk for proposing this idea.

It's a new API introduced in concurrent-ruby 1.3.1, which works better
in the container environment.

ruby-concurrency/concurrent-ruby#1038

Since there are gems like sorbet-runtime that still use older versions
of concurrent-ruby, we can't directly bump concurrent-ruby's requirement,
but need to check if the method is available before calling it.
Copy link

codecov bot commented Jul 6, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 98.65%. Comparing base (8e62859) to head (adb69d2).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2339      +/-   ##
==========================================
- Coverage   98.66%   98.65%   -0.01%     
==========================================
  Files         205      205              
  Lines       13479    13483       +4     
==========================================
+ Hits        13299    13302       +3     
- Misses        180      181       +1     
Components Coverage Δ
sentry-ruby 99.02% <85.71%> (-0.01%) ⬇️
sentry-rails 97.41% <ø> (ø)
sentry-sidekiq 97.01% <ø> (ø)
sentry-resque 96.79% <ø> (ø)
sentry-delayed_job 98.92% <ø> (ø)
sentry-opentelemetry 100.00% <ø> (ø)
Files Coverage Δ
sentry-ruby/spec/sentry/configuration_spec.rb 100.00% <100.00%> (ø)
sentry-ruby/lib/sentry/configuration.rb 98.42% <80.00%> (-0.38%) ⬇️

@st0012 st0012 requested a review from sl0thentr0py July 6, 2024 13:40
@trevorturk
Copy link

Nice, thanks! I wonder if we should move that / 2.0 into the new helper method since we can (hopefully) trust the new container version to do the right thing? Either way looks good enough. Thank you!

@sl0thentr0py
Copy link
Member

ty both!

@sl0thentr0py sl0thentr0py merged commit 142ec2f into master Jul 9, 2024
124 of 125 checks passed
@sl0thentr0py sl0thentr0py deleted the adopt-usable-processor-count branch July 9, 2024 11:58
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.

3 participants